From a05b7716616be311c93d288e2bc9c4e862ce5180 Mon Sep 17 00:00:00 2001 From: BatedUrGonnaDie Date: Tue, 19 Jun 2018 00:46:44 -0700 Subject: [PATCH 1/7] optimize db queries on home page Gather all rivals with category id, and search through that instead of searching for a rival every single run iteration, find all pbs in one query using unions, use any? instead of present to only load pb query once, get all user runs in one go, instead of checking every iteration, eager load run game and category, use pb_for and check if present to reduce another query if there is a pb --- Gemfile | 1 + Gemfile.lock | 3 +++ app/models/game.rb | 4 ++-- app/models/user.rb | 16 ++++++++++------ app/views/runs/index.slim | 4 ++-- app/views/shared/_follows.slim | 8 +++++--- app/views/shared/_run_table.slim | 8 ++++---- 7 files changed, 27 insertions(+), 17 deletions(-) diff --git a/Gemfile b/Gemfile index e4528fece..2a4aeb4a9 100644 --- a/Gemfile +++ b/Gemfile @@ -65,6 +65,7 @@ gem 'cancancan' gem 'doorkeeper' # db +gem 'active_record_union' gem 'activerecord-import' gem 'arel' gem 'aws-sdk-rails' diff --git a/Gemfile.lock b/Gemfile.lock index 728f45a7e..5e55e7cca 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -43,6 +43,8 @@ GEM activemodel (>= 4.1, < 6) case_transform (>= 0.2) jsonapi-renderer (>= 0.1.1.beta1, < 0.3) + active_record_union (1.3.0) + activerecord (>= 4.0) activejob (5.2.0) activesupport (= 5.2.0) globalid (>= 0.3.6) @@ -436,6 +438,7 @@ PLATFORMS DEPENDENCIES active_model_serializers + active_record_union activerecord-import administrate api-pagination (= 4.7) diff --git a/app/models/game.rb b/app/models/game.rb index e18eecfa0..99a6a281d 100644 --- a/app/models/game.rb +++ b/app/models/game.rb @@ -6,11 +6,11 @@ class Game < ApplicationRecord validates :name, presence: true - has_many :categories + has_many :categories, dependent: :destroy has_many :runs, through: :categories has_many :users, -> { distinct }, through: :runs has_many :runners, -> { distinct }, through: :runs, class_name: 'User' - has_many :aliases, class_name: 'GameAlias' + has_many :aliases, class_name: 'GameAlias', dependent: :destroy after_create :create_initial_alias diff --git a/app/models/user.rb b/app/models/user.rb index adf23f693..ae3e714c4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,7 +4,7 @@ class User < ApplicationRecord include RivalUser include TwitchUser - has_many :runs + has_many :runs, dependent: :destroy has_many :categories, -> { distinct }, through: :runs has_many :games, -> { distinct }, through: :runs @@ -39,9 +39,9 @@ def self.search(term) # avatar returns the user's avatar, or a default avatar if the user has not set one. It cannot return nil. def avatar - return "https://splits.io/logo.svg" if read_attribute(:avatar).nil? + return 'https://splits.io/logo.svg' if self[:avatar].nil? - URI.parse(read_attribute(:avatar) || '').tap do |uri| + URI.parse(self[:avatar] || '').tap do |uri| uri.scheme = 'https' end.to_s end @@ -63,9 +63,8 @@ def previous_upload_for(category) end def pbs - runs.where( - id: categories.map { |category| pb_for(category).id10 } | runs.where(category: nil).pluck(:id) - ) + Run.where(user: self).select('DISTINCT ON (category_id) *').order('category_id, realtime_duration_ms ASC') + .union(Run.where(user: self, category: nil)) end def runs?(category) @@ -76,6 +75,11 @@ def to_s twitch_display_name || name || 'somebody' end + def rivals + User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalries.from_user_id = 1') + .select('users.*, rivalries.category_id') + end + def twitch_followed_users User.joins(:twitch_user_followers).where(twitch_user_follows: {from_user_id: id}) end diff --git a/app/views/runs/index.slim b/app/views/runs/index.slim index ff1ed29ae..15fff97de 100644 --- a/app/views/runs/index.slim +++ b/app/views/runs/index.slim @@ -9,7 +9,7 @@ span Splits I/O - else h2 Splits I/O - - if logged_in? && current_user.runs.present? + - if logged_in? && current_user.runs.any? h5 Want some jelly with these PBs? - else h5 @@ -20,7 +20,7 @@ article .row .col-lg-12 - if logged_in? - - if current_user.runs.present? + - if current_user.runs.any? .card.shadow = render partial: 'shared/run_table', locals: { \ runs: current_user.pbs, \ diff --git a/app/views/shared/_follows.slim b/app/views/shared/_follows.slim index 8756db387..21a184a14 100644 --- a/app/views/shared/_follows.slim +++ b/app/views/shared/_follows.slim @@ -2,7 +2,9 @@ h4 Recent PBs By People You Follow .card.col-md-6.p-0 .list-group - - Run.includes(:game, :category, :user).where(user: current_user.twitch_followed_users).where.not(games: {name: [nil, ""]}).order('runs.created_at DESC').limit(10).each do |run| + - followed_runs = Run.includes(:game, :category, :user).where(user: current_user.twitch_followed_users).where.not(games: {name: [nil, ""]}).order('runs.created_at DESC').limit(10) + - user_runs = current_user.pbs.where(category_id: followed_runs.map(&:category_id)) + - followed_runs.each do |run| = link_to(run, class: 'list-group-item list-group-item-action flex-column align-items-start') .media = image_tag(run.user.avatar, size: '70x70', class: 'mr-3') @@ -14,8 +16,8 @@ = "#{run.game} #{run.category}" div = format_ms(run.duration_ms(Run::REAL)) - - if current_user.runs?(run.category) - - user_pb = current_user.pb_for(run.category) + - user_pb = user_runs.detect { |r| r.category_id == run.category_id } + - if user_pb.present? = " (" = pretty_difference(user_pb.duration_ms(Run::REAL), run.duration_ms(Run::REAL)) = ")" diff --git a/app/views/shared/_run_table.slim b/app/views/shared/_run_table.slim index 62eb26aa2..be46b72ae 100644 --- a/app/views/shared/_run_table.slim +++ b/app/views/shared/_run_table.slim @@ -1,7 +1,7 @@ - description ||= nil - cols ||= [:runner, :time, :name, :rival, :uploaded] - runs = order_runs(runs).page(params[:page]) -- if runs.empty? +- if runs.blank? center i No runs matched! - else @@ -24,7 +24,7 @@ - if cols.include?(:uploaded) th.align-right = th_sorter('Uploaded', 'created_at') tbody - - runs.includes(:user).each do |run| + - runs.includes(:user, :game, :category).each do |run| tr id="run_#{run.id36}" - if cols.include?(:owner_controls) td.align-right @@ -43,8 +43,8 @@ - if cols.include?(:rival) td.align-right small - - rival = run.user.rival_for(run.category) - - if rival.present? && rival.runs?(run.category) + - rival = run.user.rivals.detect { |r| r.category_id == run.category_id } + - if rival.present? && rival.pb_for(run.category) - rival_run = rival.pb_for(run.category) = link_to compare_path(run, rival_run), class: 'run-link stealth-link' do = pretty_difference(run.duration_ms(Run::REAL), rival_run.duration_ms(Run::REAL)) From ae4815fc2384107eb4abe397797d5b51d2054e96 Mon Sep 17 00:00:00 2001 From: BatedUrGonnaDie Date: Tue, 19 Jun 2018 00:59:38 -0700 Subject: [PATCH 2/7] add .present? check to better indicate what is happening --- app/views/shared/_run_table.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/_run_table.slim b/app/views/shared/_run_table.slim index be46b72ae..105b17d7d 100644 --- a/app/views/shared/_run_table.slim +++ b/app/views/shared/_run_table.slim @@ -44,7 +44,7 @@ td.align-right small - rival = run.user.rivals.detect { |r| r.category_id == run.category_id } - - if rival.present? && rival.pb_for(run.category) + - if rival.present? && rival.pb_for(run.category).present? - rival_run = rival.pb_for(run.category) = link_to compare_path(run, rival_run), class: 'run-link stealth-link' do = pretty_difference(run.duration_ms(Run::REAL), rival_run.duration_ms(Run::REAL)) From 5d16e0d4b3ab30ebda6cc884f7e1189720b36c92 Mon Sep 17 00:00:00 2001 From: BatedUrGonnaDie Date: Tue, 19 Jun 2018 01:17:01 -0700 Subject: [PATCH 3/7] don't always query first user rivals --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index ae3e714c4..cb16d154e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -76,7 +76,7 @@ def to_s end def rivals - User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalries.from_user_id = 1') + User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalres.from_user_id = ' + id) .select('users.*, rivalries.category_id') end From e17be3f315e18f54973477c1f93df99f738102bb Mon Sep 17 00:00:00 2001 From: BatedUrGonnaDie Date: Tue, 19 Jun 2018 01:52:13 -0700 Subject: [PATCH 4/7] somehow i removed the 'i' --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index cb16d154e..a9d6d5e64 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -76,7 +76,7 @@ def to_s end def rivals - User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalres.from_user_id = ' + id) + User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalries.from_user_id = ' + id.to_s) .select('users.*, rivalries.category_id') end From 0709c17c16500769b12cf293488545493c2ac54e Mon Sep 17 00:00:00 2001 From: BatedUrGonnaDie Date: Tue, 19 Jun 2018 18:56:23 -0700 Subject: [PATCH 5/7] use runs instead of Run.where use union_all now since we know there will not be any duplicates, change dependents as well --- app/models/game.rb | 2 +- app/models/user.rb | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/game.rb b/app/models/game.rb index 99a6a281d..b797e39ca 100644 --- a/app/models/game.rb +++ b/app/models/game.rb @@ -10,7 +10,7 @@ class Game < ApplicationRecord has_many :runs, through: :categories has_many :users, -> { distinct }, through: :runs has_many :runners, -> { distinct }, through: :runs, class_name: 'User' - has_many :aliases, class_name: 'GameAlias', dependent: :destroy + has_many :aliases, class_name: 'GameAlias', dependent: :restrict_with_exception after_create :create_initial_alias diff --git a/app/models/user.rb b/app/models/user.rb index a9d6d5e64..3a8f2e216 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -4,7 +4,7 @@ class User < ApplicationRecord include RivalUser include TwitchUser - has_many :runs, dependent: :destroy + has_many :runs has_many :categories, -> { distinct }, through: :runs has_many :games, -> { distinct }, through: :runs @@ -39,9 +39,9 @@ def self.search(term) # avatar returns the user's avatar, or a default avatar if the user has not set one. It cannot return nil. def avatar - return 'https://splits.io/logo.svg' if self[:avatar].nil? + return 'https://splits.io/logo.svg' if read_attribute(:avatar).nil? - URI.parse(self[:avatar] || '').tap do |uri| + URI.parse(read_attribute(:avatar) || '').tap do |uri| uri.scheme = 'https' end.to_s end @@ -63,8 +63,8 @@ def previous_upload_for(category) end def pbs - Run.where(user: self).select('DISTINCT ON (category_id) *').order('category_id, realtime_duration_ms ASC') - .union(Run.where(user: self, category: nil)) + runs.where.not(category: nil).select('DISTINCT ON (category_id) *').order('category_id, realtime_duration_ms ASC') + .union_all(runs.by_category(nil)) end def runs?(category) @@ -76,7 +76,7 @@ def to_s end def rivals - User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalries.from_user_id = ' + id.to_s) + User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalries.from_user_id = ', id.to_s) .select('users.*, rivalries.category_id') end From 7c3c69f7596ebf69567961d3d92333dea0833f2d Mon Sep 17 00:00:00 2001 From: BatedUrGonnaDie Date: Tue, 19 Jun 2018 19:06:06 -0700 Subject: [PATCH 6/7] Undo rubocop thing --- app/models/game.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/game.rb b/app/models/game.rb index b797e39ca..8bcadda7d 100644 --- a/app/models/game.rb +++ b/app/models/game.rb @@ -10,7 +10,7 @@ class Game < ApplicationRecord has_many :runs, through: :categories has_many :users, -> { distinct }, through: :runs has_many :runners, -> { distinct }, through: :runs, class_name: 'User' - has_many :aliases, class_name: 'GameAlias', dependent: :restrict_with_exception + has_many :aliases, class_name: 'GameAlias' after_create :create_initial_alias From 374782f2ff75beb4c190ede3a96ce1cebdd71d1e Mon Sep 17 00:00:00 2001 From: BatedUrGonnaDie Date: Wed, 20 Jun 2018 00:13:58 -0700 Subject: [PATCH 7/7] make rivalry code infinitely easier --- app/models/user.rb | 5 ----- app/views/shared/_run_table.slim | 8 ++++---- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 3a8f2e216..e43997f9e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -75,11 +75,6 @@ def to_s twitch_display_name || name || 'somebody' end - def rivals - User.joins('INNER JOIN rivalries ON rivalries.to_user_id = users.id AND rivalries.from_user_id = ', id.to_s) - .select('users.*, rivalries.category_id') - end - def twitch_followed_users User.joins(:twitch_user_followers).where(twitch_user_follows: {from_user_id: id}) end diff --git a/app/views/shared/_run_table.slim b/app/views/shared/_run_table.slim index 105b17d7d..c6cf7b89e 100644 --- a/app/views/shared/_run_table.slim +++ b/app/views/shared/_run_table.slim @@ -43,13 +43,13 @@ - if cols.include?(:rival) td.align-right small - - rival = run.user.rivals.detect { |r| r.category_id == run.category_id } - - if rival.present? && rival.pb_for(run.category).present? - - rival_run = rival.pb_for(run.category) + - rivalry = run.user.rivalries.includes(:to_user).detect { |r| r.category_id == run.category_id } + - if rivalry.present? && rivalry.to_user.pb_for(run.category).present? + - rival_run = rivalry.to_user.pb_for(run.category) = link_to compare_path(run, rival_run), class: 'run-link stealth-link' do = pretty_difference(run.duration_ms(Run::REAL), rival_run.duration_ms(Run::REAL)) ' against - = user_badge(rival) + = user_badge(rivalry.to_user) - elsif run.category.present? = link_to new_rivalry_path(category: run.category) .text-success = icon('fas', 'plus')