From 19e3b3b1bce96787e27dd59c424605a43c7bf5db Mon Sep 17 00:00:00 2001
From: Sam Saffron <sam.saffron@gmail.com>
Date: Wed, 5 Jun 2019 18:22:47 +1000
Subject: [PATCH] PERF: speed up topic poster lookups

During profiling looking up topic users popped up as a hot path, this
change more than halved the amount of work it does

It reduces object allocations and method calls and avoids repeate translation
of common terms
---
 app/models/topic_posters_summary.rb | 60 ++++++++++++++++++-----------
 lib/topic_query.rb                  |  6 ++-
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/app/models/topic_posters_summary.rb b/app/models/topic_posters_summary.rb
index a8bc292c802..b054401617d 100644
--- a/app/models/topic_posters_summary.rb
+++ b/app/models/topic_posters_summary.rb
@@ -4,11 +4,23 @@
 require_dependency 'topic_poster'
 
 class TopicPostersSummary
+
+  # localization is fast, but this allows us to avoid
+  # calling it in a loop which adds up
+  def self.translations
+    {
+      original_poster: I18n.t(:original_poster),
+      most_recent_poster: I18n.t(:most_recent_poster),
+      frequent_poster: I18n.t(:frequent_poster)
+    }
+  end
+
   attr_reader :topic, :options
 
   def initialize(topic, options = {})
     @topic = topic
     @options = options
+    @translations = options[:translations] || TopicPostersSummary.translations
   end
 
   def summary
@@ -18,23 +30,38 @@ class TopicPostersSummary
   private
 
   def new_topic_poster_for(user)
-    TopicPoster.new.tap do |topic_poster|
-      topic_poster.user = user
-      topic_poster.description = descriptions_for(user)
-      topic_poster.primary_group = primary_group_lookup[user.id]
-      if topic.last_post_user_id == user.id
-        topic_poster.extras = +'latest'
-        topic_poster.extras << ' single' if user_ids.uniq.size == 1
-      end
+    topic_poster = TopicPoster.new
+    topic_poster.user = user
+    topic_poster.description = descriptions_for(user)
+    topic_poster.primary_group = primary_group_lookup[user.id]
+    if topic.last_post_user_id == user.id
+      topic_poster.extras = +'latest'
+      topic_poster.extras << ' single' if user_ids.uniq.size == 1
     end
+    topic_poster
   end
 
   def descriptions_by_id
     @descriptions_by_id ||= begin
-      user_ids_with_descriptions.each_with_object({}) do |(id, description), descriptions|
-        descriptions[id] ||= []
-        descriptions[id] << description
+      result = {}
+      ids = user_ids
+
+      if id = ids.shift
+        result[id] ||= []
+        result[id] << @translations[:original_poster]
       end
+
+      if id = ids.shift
+        result[id] ||= []
+        result[id] << @translations[:most_recent_poster]
+      end
+
+      while id = ids.shift
+        result[id] ||= []
+        result[id] << @translations[:frequent_poster]
+      end
+
+      result
     end
   end
 
@@ -50,17 +77,6 @@ class TopicPostersSummary
     summary
   end
 
-  def user_ids_with_descriptions
-    user_ids.zip([
-      :original_poster,
-      :most_recent_poster,
-      :frequent_poster,
-      :frequent_poster,
-      :frequent_poster,
-      :frequent_poster
-      ].map { |description| I18n.t(description) })
-  end
-
   def last_poster_is_topic_creator?
     topic.user_id == topic.last_post_user_id
   end
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index 33547952805..1b90719454f 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -446,10 +446,14 @@ class TopicQuery
       avatar_lookup = AvatarLookup.new(user_ids)
       primary_group_lookup = PrimaryGroupLookup.new(user_ids)
 
+      # memoize for loop so we don't keep looking these up
+      translations = TopicPostersSummary.translations
+
       topics.each do |t|
         t.posters = t.posters_summary(
           avatar_lookup: avatar_lookup,
-          primary_group_lookup: primary_group_lookup
+          primary_group_lookup: primary_group_lookup,
+          translations: translations
         )
       end
     end