From 1867442fbc3174c80b74b95112abfedc0e0527ee Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 24 Nov 2016 10:11:39 +0800 Subject: [PATCH 1/4] PERF: Add score indexes for top topics. --- .../20161124020918_add_scores_indexes_to_top_topics.rb | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 db/migrate/20161124020918_add_scores_indexes_to_top_topics.rb diff --git a/db/migrate/20161124020918_add_scores_indexes_to_top_topics.rb b/db/migrate/20161124020918_add_scores_indexes_to_top_topics.rb new file mode 100644 index 00000000000..0e79373d230 --- /dev/null +++ b/db/migrate/20161124020918_add_scores_indexes_to_top_topics.rb @@ -0,0 +1,9 @@ +class AddScoresIndexesToTopTopics < ActiveRecord::Migration + def change + add_index :top_topics, :daily_score + add_index :top_topics, :weekly_score + add_index :top_topics, :monthly_score + add_index :top_topics, :yearly_score + add_index :top_topics, :all_score + end +end From f812415c526f422e8944797efec442e4330d94fa Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 24 Nov 2016 10:13:03 +0800 Subject: [PATCH 2/4] Update annotations. --- app/models/category.rb | 8 +++++--- app/models/post_upload.rb | 4 +++- app/models/top_topic.rb | 5 +++++ app/models/user.rb | 15 ++++++++------- app/models/user_avatar.rb | 4 +++- app/models/user_profile.rb | 2 ++ 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index e0ac08ac877..2a044088c88 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -547,7 +547,9 @@ end # # Indexes # -# index_categories_on_email_in (email_in) UNIQUE -# index_categories_on_topic_count (topic_count) -# unique_index_categories_on_name (name) UNIQUE +# index_categories_on_background_url (background_url) +# index_categories_on_email_in (email_in) UNIQUE +# index_categories_on_logo_url (logo_url) +# index_categories_on_topic_count (topic_count) +# unique_index_categories_on_name (name) UNIQUE # diff --git a/app/models/post_upload.rb b/app/models/post_upload.rb index f341877ff7b..ea7ece7327f 100644 --- a/app/models/post_upload.rb +++ b/app/models/post_upload.rb @@ -13,5 +13,7 @@ end # # Indexes # -# idx_unique_post_uploads (post_id,upload_id) UNIQUE +# idx_unique_post_uploads (post_id,upload_id) UNIQUE +# index_post_uploads_on_post_id (post_id) +# index_post_uploads_on_upload_id (upload_id) # diff --git a/app/models/top_topic.rb b/app/models/top_topic.rb index 29f206ea302..3a5328723a2 100644 --- a/app/models/top_topic.rb +++ b/app/models/top_topic.rb @@ -265,13 +265,16 @@ end # # Indexes # +# index_top_topics_on_all_score (all_score) # index_top_topics_on_daily_likes_count (daily_likes_count) # index_top_topics_on_daily_op_likes_count (daily_op_likes_count) # index_top_topics_on_daily_posts_count (daily_posts_count) +# index_top_topics_on_daily_score (daily_score) # index_top_topics_on_daily_views_count (daily_views_count) # index_top_topics_on_monthly_likes_count (monthly_likes_count) # index_top_topics_on_monthly_op_likes_count (monthly_op_likes_count) # index_top_topics_on_monthly_posts_count (monthly_posts_count) +# index_top_topics_on_monthly_score (monthly_score) # index_top_topics_on_monthly_views_count (monthly_views_count) # index_top_topics_on_quarterly_likes_count (quarterly_likes_count) # index_top_topics_on_quarterly_op_likes_count (quarterly_op_likes_count) @@ -281,9 +284,11 @@ end # index_top_topics_on_weekly_likes_count (weekly_likes_count) # index_top_topics_on_weekly_op_likes_count (weekly_op_likes_count) # index_top_topics_on_weekly_posts_count (weekly_posts_count) +# index_top_topics_on_weekly_score (weekly_score) # index_top_topics_on_weekly_views_count (weekly_views_count) # index_top_topics_on_yearly_likes_count (yearly_likes_count) # index_top_topics_on_yearly_op_likes_count (yearly_op_likes_count) # index_top_topics_on_yearly_posts_count (yearly_posts_count) +# index_top_topics_on_yearly_score (yearly_score) # index_top_topics_on_yearly_views_count (yearly_views_count) # diff --git a/app/models/user.rb b/app/models/user.rb index 62f7835a595..d292d4917b4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1096,11 +1096,12 @@ end # # Indexes # -# idx_users_admin (id) -# idx_users_moderator (id) -# index_users_on_auth_token (auth_token) -# index_users_on_last_posted_at (last_posted_at) -# index_users_on_last_seen_at (last_seen_at) -# index_users_on_username (username) UNIQUE -# index_users_on_username_lower (username_lower) UNIQUE +# idx_users_admin (id) +# idx_users_moderator (id) +# index_users_on_auth_token (auth_token) +# index_users_on_last_posted_at (last_posted_at) +# index_users_on_last_seen_at (last_seen_at) +# index_users_on_uploaded_avatar_id (uploaded_avatar_id) +# index_users_on_username (username) UNIQUE +# index_users_on_username_lower (username_lower) UNIQUE # diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index 391936c79e4..6b24debf582 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -105,5 +105,7 @@ end # # Indexes # -# index_user_avatars_on_user_id (user_id) +# index_user_avatars_on_custom_upload_id (custom_upload_id) +# index_user_avatars_on_gravatar_upload_id (gravatar_upload_id) +# index_user_avatars_on_user_id (user_id) # diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 092276dc250..c05caea3b20 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -124,4 +124,6 @@ end # Indexes # # index_user_profiles_on_bio_cooked_version (bio_cooked_version) +# index_user_profiles_on_card_background (card_background) +# index_user_profiles_on_profile_background (profile_background) # From 857955dd043c17605584f7f43ec1c383b3b47b1c Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 24 Nov 2016 10:26:39 +0800 Subject: [PATCH 3/4] Follow our convention of declaring private methods. --- app/models/top_topic.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/top_topic.rb b/app/models/top_topic.rb index 3a5328723a2..d0564c9f7e0 100644 --- a/app/models/top_topic.rb +++ b/app/models/top_topic.rb @@ -52,6 +52,8 @@ class TopTopic < ActiveRecord::Base all: 6) end + private + def self.sort_orders @@sort_orders ||= [:posts, :views, :likes, :op_likes].freeze end @@ -224,10 +226,6 @@ class TopTopic < ActiveRecord::Base AND tt.#{period}_#{sort}_count <> c.count", from: start_of(period)) end - - private_class_method :sort_orders, :update_counts_and_compute_scores_for, :remove_invisible_topics, - :add_new_visible_topics, :update_posts_count_for, :update_views_count_for, :update_likes_count_for, - :compute_top_score_for, :start_of, :update_top_topics end # == Schema Information From b889bfefbb60469d039525e4874aef772d26bfe7 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 24 Nov 2016 10:29:44 +0800 Subject: [PATCH 4/4] PERF: Don't calculate the same query twice. --- app/controllers/list_controller.rb | 14 ++++++++++---- app/models/site_setting.rb | 4 ++-- app/models/top_topic.rb | 6 ------ spec/models/site_setting_spec.rb | 7 +++++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index e680ce8d683..080c9029e43 100644 --- a/app/controllers/list_controller.rb +++ b/app/controllers/list_controller.rb @@ -327,14 +327,20 @@ class ListController < ApplicationController exclude_category_ids.pluck(:id) end - def self.best_period_for(previous_visit_at, category_id=nil) + def self.best_period_with_topics_for(previous_visit_at, category_id=nil) best_periods_for(previous_visit_at).each do |period| top_topics = TopTopic.where("#{period}_score > 0") top_topics = top_topics.joins(:topic).where("topics.category_id = ?", category_id) if category_id - return period if top_topics.count >= SiteSetting.topics_per_period_in_top_page + top_topics = top_topics.limit(SiteSetting.topics_per_period_in_top_page) + return period if top_topics.count == SiteSetting.topics_per_period_in_top_page end - # default period is yearly - SiteSetting.top_page_default_timeframe.to_sym + + false + end + + def self.best_period_for(previous_visit_at, category_id=nil) + best_period_with_topics_for(previous_visit_at, category_id) || + SiteSetting.top_page_default_timeframe.to_sym end def self.best_periods_for(date) diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 70b62602eec..ef88e14516d 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -99,8 +99,8 @@ class SiteSetting < ActiveRecord::Base end def self.min_redirected_to_top_period(duration) - period = ListController.best_period_for(duration) - return period if TopTopic.topics_per_period(period) >= SiteSetting.topics_per_period_in_top_page + period = ListController.best_period_with_topics_for(duration) + return period if period # not enough topics nil diff --git a/app/models/top_topic.rb b/app/models/top_topic.rb index d0564c9f7e0..6172f7cf0a0 100644 --- a/app/models/top_topic.rb +++ b/app/models/top_topic.rb @@ -4,12 +4,6 @@ class TopTopic < ActiveRecord::Base belongs_to :topic - def self.topics_per_period(period) - DistributedMemoizer.memoize("#{Discourse.current_hostname}_topics_per_period_#{period}", 1.day) do - TopTopic.where("#{period}_score > 0").count - end.to_i - end - # The top topics we want to refresh often def self.refresh_daily! transaction do diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb index 78589dc4ade..d1905b2c085 100644 --- a/spec/models/site_setting_spec.rb +++ b/spec/models/site_setting_spec.rb @@ -76,7 +76,11 @@ describe SiteSetting do before do SiteSetting.topics_per_period_in_top_page = 2 SiteSetting.top_page_default_timeframe = 'daily' - TopTopic.stubs(:topics_per_period).with(:daily).returns(3) + + 2.times do + TopTopic.create!(daily_score: 2.5) + end + TopTopic.refresh! end @@ -91,7 +95,6 @@ describe SiteSetting do before do SiteSetting.topics_per_period_in_top_page = 20 SiteSetting.top_page_default_timeframe = 'daily' - TopTopic.stubs(:topics_per_period).with(:daily).returns(1) TopTopic.refresh! end