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
This commit is contained in:
Sam Saffron 2019-06-05 18:22:47 +10:00
parent ce79a71c5d
commit 19e3b3b1bc
2 changed files with 43 additions and 23 deletions

View File

@ -4,11 +4,23 @@
require_dependency 'topic_poster' require_dependency 'topic_poster'
class TopicPostersSummary 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 attr_reader :topic, :options
def initialize(topic, options = {}) def initialize(topic, options = {})
@topic = topic @topic = topic
@options = options @options = options
@translations = options[:translations] || TopicPostersSummary.translations
end end
def summary def summary
@ -18,23 +30,38 @@ class TopicPostersSummary
private private
def new_topic_poster_for(user) def new_topic_poster_for(user)
TopicPoster.new.tap do |topic_poster| topic_poster = TopicPoster.new
topic_poster.user = user topic_poster.user = user
topic_poster.description = descriptions_for(user) topic_poster.description = descriptions_for(user)
topic_poster.primary_group = primary_group_lookup[user.id] topic_poster.primary_group = primary_group_lookup[user.id]
if topic.last_post_user_id == user.id if topic.last_post_user_id == user.id
topic_poster.extras = +'latest' topic_poster.extras = +'latest'
topic_poster.extras << ' single' if user_ids.uniq.size == 1 topic_poster.extras << ' single' if user_ids.uniq.size == 1
end
end end
topic_poster
end end
def descriptions_by_id def descriptions_by_id
@descriptions_by_id ||= begin @descriptions_by_id ||= begin
user_ids_with_descriptions.each_with_object({}) do |(id, description), descriptions| result = {}
descriptions[id] ||= [] ids = user_ids
descriptions[id] << description
if id = ids.shift
result[id] ||= []
result[id] << @translations[:original_poster]
end 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
end end
@ -50,17 +77,6 @@ class TopicPostersSummary
summary summary
end 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? def last_poster_is_topic_creator?
topic.user_id == topic.last_post_user_id topic.user_id == topic.last_post_user_id
end end

View File

@ -446,10 +446,14 @@ class TopicQuery
avatar_lookup = AvatarLookup.new(user_ids) avatar_lookup = AvatarLookup.new(user_ids)
primary_group_lookup = PrimaryGroupLookup.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| topics.each do |t|
t.posters = t.posters_summary( t.posters = t.posters_summary(
avatar_lookup: avatar_lookup, avatar_lookup: avatar_lookup,
primary_group_lookup: primary_group_lookup primary_group_lookup: primary_group_lookup,
translations: translations
) )
end end
end end