From 708722ad132bab9a38f610dbed32639eed105343 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 11 Oct 2024 10:59:20 +0800 Subject: [PATCH] PERF: Speed up `PostSerializer#reactions` by avoiding nested loop take 2 (#313) This is a follow-up to 7467349d6cd428f4e566cbbbfe3c5b8f37efdf99. 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. --- .../topic_view_serializer_extension.rb | 8 ++++++-- plugin.rb | 12 ++++-------- spec/serializers/post_serializer_spec.rb | 8 +++++--- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/discourse_reactions/topic_view_serializer_extension.rb b/lib/discourse_reactions/topic_view_serializer_extension.rb index 900a02f..8508816 100644 --- a/lib/discourse_reactions/topic_view_serializer_extension.rb +++ b/lib/discourse_reactions/topic_view_serializer_extension.rb @@ -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 @@ -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) diff --git a/plugin.rb b/plugin.rb index e010237..662e90a 100644 --- a/plugin.rb +++ b/plugin.rb @@ -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 diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb index 0684d7c..3f453f2 100644 --- a/spec/serializers/post_serializer_spec.rb +++ b/spec/serializers/post_serializer_spec.rb @@ -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