Skip to content

Commit

Permalink
PERF: Speed up PostSerializer#reactions by avoiding nested loop tak…
Browse files Browse the repository at this point in the history
…e 2 (#313)

This is a follow-up to 7467349.

Within the `object.post_actions.reject` loop, we are running through
another loop given by `object.post_actions_with_reaction_users&.find`.
When the `object.post_actions` and
`object.post_actions_with_reaction_users` array is large, we end up
spending alot of time executing the loops.

This commit resolves the problem by reducing the number of records we
loop in `object.post_actions_with_reaction_users` by prefiltering the
records by `post_action.id` to remove alot of unnecessary iterations in
the loop.

Local benchmarks for the method for 8000 items in `object.post_actions` records and 2000
items in `post_actions_with_reaction_users` shows the runtime of the
method decreasing from roughly 7seconds to about 5ms.
  • Loading branch information
tgxworld authored Oct 11, 2024
1 parent 7467349 commit 708722a
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 13 deletions.
8 changes: 6 additions & 2 deletions lib/discourse_reactions/topic_view_serializer_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ def self.load_post_action_reaction_users_for_posts(post_ids)
"post_actions.post_id IN (#{DiscourseReactions::PostActionExtension.post_action_with_reaction_user_sql})",
valid_reactions: DiscourseReactions::Reaction.reactions_counting_as_like,
)
.reduce({}) do |hash, post_action|
hash[post_action.post_id] ||= {}
hash[post_action.post_id][post_action.id] = post_action
hash
end
end

def posts
Expand All @@ -29,8 +34,7 @@ def posts
)

posts.each do |post|
post.post_actions_with_reaction_users =
post_actions_with_reaction_users.select { |post_action| post_action.post_id == post.id }
post.post_actions_with_reaction_users = post_actions_with_reaction_users[post.id] || {}
end

object.instance_variable_set(:@posts, posts)
Expand Down
12 changes: 4 additions & 8 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,10 @@ class Engine < ::Rails::Engine

# Also get rid of any PostAction records that match up to a ReactionUser
# that is now the main_reaction_id and has historical data.
object
.post_actions_with_reaction_users
&.find do |pa|
pa.id == post_action.id &&
pa.reaction_user&.reaction&.reaction_value ==
DiscourseReactions::Reaction.main_reaction_id
end
.present?
object.post_actions_with_reaction_users[post_action.id]
&.reaction_user
&.reaction
&.reaction_value == DiscourseReactions::Reaction.main_reaction_id
end

# Likes will only be blank if there are only reactions where the reaction is in
Expand Down
8 changes: 5 additions & 3 deletions spec/serializers/post_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@
reaction_user_1 && reaction_user_2 && reaction_user_3 && like

post_1.post_actions_with_reaction_users =
DiscourseReactions::TopicViewSerializerExtension
.load_post_action_reaction_users_for_posts([post_1.id])
.select { |pa| pa.post_id == post_1.id }
DiscourseReactions::TopicViewSerializerExtension.load_post_action_reaction_users_for_posts(
[post_1.id],
)[
post_1.id
]
end

it "renders custom reactions which should be sorted by count" do
Expand Down

0 comments on commit 708722a

Please sign in to comment.