From 12dc511fea5489da0d083ed8ccc5de5e5daa3762 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 22 Jul 2016 09:48:26 +1000 Subject: [PATCH] PERF: make score calculator cheaper when site has long topics --- app/jobs/scheduled/periodical_updates.rb | 12 +++- lib/score_calculator.rb | 79 +++++++++++------------- spec/components/score_calculator_spec.rb | 13 ++++ 3 files changed, 61 insertions(+), 43 deletions(-) diff --git a/app/jobs/scheduled/periodical_updates.rb b/app/jobs/scheduled/periodical_updates.rb index b051b3f85..2d0485ba5 100644 --- a/app/jobs/scheduled/periodical_updates.rb +++ b/app/jobs/scheduled/periodical_updates.rb @@ -7,12 +7,22 @@ module Jobs class PeriodicalUpdates < Jobs::Scheduled every 15.minutes + def self.should_update_long_topics? + @call_count ||= 0 + @call_count += 1 + + # once every 6 hours + (@call_count % 24) == 1 + end + def execute(args) # Feature topics in categories CategoryFeaturedTopic.feature_topics # Update the scores of posts - ScoreCalculator.new.calculate(1.day.ago) + args = {min_topic_age: 1.day.ago} + args[:max_topic_length] = 500 unless self.class.should_update_long_topics? + ScoreCalculator.new.calculate(args) # Automatically close stuff that we missed Topic.auto_close diff --git a/lib/score_calculator.rb b/lib/score_calculator.rb index 681877bda..8901c12e5 100644 --- a/lib/score_calculator.rb +++ b/lib/score_calculator.rb @@ -16,48 +16,44 @@ class ScoreCalculator end # Calculate the score for all posts based on the weightings - def calculate(min_topic_age=nil) - - update_posts_score(min_topic_age) - - update_posts_rank(min_topic_age) - - update_topics_rank(min_topic_age) - - update_topics_percent_rank(min_topic_age) - + def calculate(opts=nil) + update_posts_score(opts) + update_posts_rank(opts) + update_topics_rank(opts) + update_topics_percent_rank(opts) end private - def update_posts_score(min_topic_age) + def update_posts_score(opts) limit = 20000 components = [] - @weightings.each_key { |k| components << "COALESCE(#{k}, 0) * :#{k}" } + @weightings.each_key { |k| components << "COALESCE(posts.#{k}, 0) * :#{k}" } components = components.join(" + ") builder = SqlBuilder.new < #{components}", @weightings) + builder.where("posts.score IS NULL OR posts.score <> #{components}", @weightings) - filter_topics(builder, min_topic_age) + filter_topics(builder, opts) while builder.exec.cmd_tuples == limit end end - def update_posts_rank(min_topic_age) + def update_posts_rank(opts) limit = 20000 builder = SqlBuilder.new < posts.percent_rank") - filter_topics(builder, min_topic_age) + filter_topics(builder, opts) while builder.exec.cmd_tuples == limit end end - def update_topics_rank(min_topic_age) - builder = SqlBuilder.new("UPDATE topics AS t - SET has_summary = (t.like_count >= :likes_required AND - t.posts_count >= :posts_required AND + def update_topics_rank(opts) + builder = SqlBuilder.new("UPDATE topics AS topics + SET has_summary = (topics.like_count >= :likes_required AND + topics.posts_count >= :posts_required AND x.max_score >= :score_required), score = x.avg_score FROM (SELECT p.topic_id, @@ -99,12 +96,12 @@ SQL GROUP BY p.topic_id) AS x /*where*/") - builder.where("x.topic_id = t.id AND + builder.where("x.topic_id = topics.id AND ( - (t.score <> x.avg_score OR t.score IS NULL) OR - (t.has_summary IS NULL OR t.has_summary <> ( - t.like_count >= :likes_required AND - t.posts_count >= :posts_required AND + (topics.score <> x.avg_score OR topics.score IS NULL) OR + (topics.has_summary IS NULL OR topics.has_summary <> ( + topics.like_count >= :likes_required AND + topics.posts_count >= :posts_required AND x.max_score >= :score_required )) ) @@ -113,15 +110,13 @@ SQL posts_required: SiteSetting.summary_posts_required, score_required: SiteSetting.summary_score_threshold) - if min_topic_age - builder.where("t.bumped_at > :bumped_at ", - bumped_at: min_topic_age) - end + + filter_topics(builder, opts) builder.exec end - def update_topics_percent_rank(min_topic_age) + def update_topics_percent_rank(opts) builder = SqlBuilder.new("UPDATE topics SET percent_rank = x.percent_rank FROM (SELECT id, percent_rank() @@ -131,22 +126,22 @@ SQL builder.where("x.id = topics.id AND (topics.percent_rank <> x.percent_rank OR topics.percent_rank IS NULL)") - - if min_topic_age - builder.where("topics.bumped_at > :bumped_at ", - bumped_at: min_topic_age) - end - + filter_topics(builder, opts) builder.exec end - def filter_topics(builder, min_topic_age) - if min_topic_age - builder.where('posts.topic_id IN - (SELECT id FROM topics WHERE bumped_at > :bumped_at)', - bumped_at: min_topic_age) + def filter_topics(builder, opts) + return builder unless opts + + if min_topic_age = opts[:min_topic_age] + builder.where("topics.bumped_at > :bumped_at ", + bumped_at: min_topic_age) + end + if max_topic_length = opts[:max_topic_length] + builder.where("topics.posts_count < :max_topic_length", + max_topic_length: max_topic_length) end builder diff --git a/spec/components/score_calculator_spec.rb b/spec/components/score_calculator_spec.rb index bcf41ebaa..d747b1433 100644 --- a/spec/components/score_calculator_spec.rb +++ b/spec/components/score_calculator_spec.rb @@ -49,6 +49,19 @@ describe ScoreCalculator do expect(topic.has_summary).to eq(false) end + it "respects the min_topic_age" do + topic.update_columns(has_summary: true, bumped_at: 1.month.ago) + ScoreCalculator.new(reads: 3).calculate(min_topic_age: 20.days.ago) + expect(topic.has_summary).to eq(true) + end + + it "respects the max_topic_length" do + Fabricate(:post, topic_id: topic.id) + topic.update_columns(has_summary: true) + ScoreCalculator.new(reads: 3).calculate(max_topic_length: 1) + expect(topic.has_summary).to eq(true) + end + it "won't update the site settings when the site settings don't match" do SiteSetting.expects(:summary_likes_required).returns(0) SiteSetting.expects(:summary_posts_required).returns(1)