From fc90d38893ae81b83884c3f5c85d05d564f1de33 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 22 Nov 2016 22:59:54 +0100 Subject: [PATCH] Moving some counter queries out of subqueries in the API --- Gemfile | 5 +++++ Gemfile.lock | 9 +++++++++ .../components/middleware/errors.jsx | 2 ++ app/controllers/api/v1/accounts_controller.rb | 14 +++++++++++--- app/controllers/api/v1/statuses_controller.rb | 13 ++++++++++--- app/controllers/application_controller.rb | 5 +++++ app/models/status.rb | 4 ++-- app/services/search_service.rb | 2 +- config/initializers/mini_profiler.rb | 17 +++++++++++++++++ 9 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 config/initializers/mini_profiler.rb diff --git a/Gemfile b/Gemfile index 4abed33ffe3..327a17ee9fe 100644 --- a/Gemfile +++ b/Gemfile @@ -50,6 +50,11 @@ gem 'react-rails' gem 'browserify-rails' gem 'autoprefixer-rails' +gem 'rack-mini-profiler', require: false +gem 'flamegraph' +gem 'stackprof' +gem 'memory_profiler' + group :development, :test do gem 'rspec-rails' gem 'pry-rails' diff --git a/Gemfile.lock b/Gemfile.lock index b058ae940a0..28ad1abb66f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -136,6 +136,7 @@ GEM execjs (2.7.0) fabrication (2.15.2) fast_blank (1.0.0) + flamegraph (0.9.5) font-awesome-rails (4.6.3.1) railties (>= 3.2, < 5.1) fuubar (2.1.1) @@ -205,6 +206,7 @@ GEM nokogiri (>= 1.5.9) mail (2.6.4) mime-types (>= 1.16, < 4) + memory_profiler (0.9.7) method_source (0.8.2) mime-types (3.1) mime-types-data (~> 3.2015) @@ -262,6 +264,8 @@ GEM rack-attack (5.0.1) rack rack-cors (0.4.0) + rack-mini-profiler (0.10.1) + rack (>= 1.2.0) rack-protection (1.5.3) rack rack-test (0.6.3) @@ -372,6 +376,7 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) + stackprof (0.2.10) temple (0.7.7) term-ansicolor (1.4.0) tins (~> 1.0) @@ -420,6 +425,7 @@ DEPENDENCIES dotenv-rails fabrication fast_blank + flamegraph font-awesome-rails fuubar goldfinger @@ -435,6 +441,7 @@ DEPENDENCIES letter_opener link_header lograge + memory_profiler neography nokogiri oj @@ -449,6 +456,7 @@ DEPENDENCIES rabl rack-attack rack-cors + rack-mini-profiler rails! rails_12factor rails_autolink @@ -463,6 +471,7 @@ DEPENDENCIES sidekiq simple_form simplecov + stackprof uglifier (>= 1.3.0) webmock will_paginate diff --git a/app/assets/javascripts/components/middleware/errors.jsx b/app/assets/javascripts/components/middleware/errors.jsx index fb161fc4c4f..3a1473bc14c 100644 --- a/app/assets/javascripts/components/middleware/errors.jsx +++ b/app/assets/javascripts/components/middleware/errors.jsx @@ -1,11 +1,13 @@ import { showAlert } from '../actions/alerts'; +const defaultSuccessSuffix = 'SUCCESS'; const defaultFailSuffix = 'FAIL'; export default function errorsMiddleware() { return ({ dispatch }) => next => action => { if (action.type) { const isFail = new RegExp(`${defaultFailSuffix}$`, 'g'); + const isSuccess = new RegExp(`${defaultSuccessSuffix}$`, 'g'); if (action.type.match(isFail)) { if (action.error.response) { diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 2dfab08319d..97d626af281 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -18,9 +18,11 @@ class Api::V1::AccountsController < ApiController def following results = Follow.where(account: @account).paginate_by_max_id(DEFAULT_ACCOUNTS_LIMIT, params[:max_id], params[:since_id]) - accounts = Account.where(id: results.map(&:target_account_id)).with_counters.map { |a| [a.id, a] }.to_h + accounts = Account.where(id: results.map(&:target_account_id)).map { |a| [a.id, a] }.to_h @accounts = results.map { |f| accounts[f.target_account_id] } + set_account_counters_maps(@accounts) + next_path = following_api_v1_account_url(max_id: results.last.id) if results.size == DEFAULT_ACCOUNTS_LIMIT prev_path = following_api_v1_account_url(since_id: results.first.id) unless results.empty? @@ -31,9 +33,11 @@ class Api::V1::AccountsController < ApiController def followers results = Follow.where(target_account: @account).paginate_by_max_id(DEFAULT_ACCOUNTS_LIMIT, params[:max_id], params[:since_id]) - accounts = Account.where(id: results.map(&:account_id)).with_counters.map { |a| [a.id, a] }.to_h + accounts = Account.where(id: results.map(&:account_id)).map { |a| [a.id, a] }.to_h @accounts = results.map { |f| accounts[f.account_id] } + set_account_counters_maps(@accounts) + next_path = followers_api_v1_account_url(max_id: results.last.id) if results.size == DEFAULT_ACCOUNTS_LIMIT prev_path = followers_api_v1_account_url(since_id: results.first.id) unless results.empty? @@ -53,9 +57,10 @@ class Api::V1::AccountsController < ApiController end def statuses - @statuses = @account.statuses.with_includes.with_counters.paginate_by_max_id(DEFAULT_STATUSES_LIMIT, params[:max_id], params[:since_id]).to_a + @statuses = @account.statuses.with_includes.paginate_by_max_id(DEFAULT_STATUSES_LIMIT, params[:max_id], params[:since_id]).to_a set_maps(@statuses) + set_counters_maps(@statuses) next_path = statuses_api_v1_account_url(max_id: @statuses.last.id) if @statuses.size == DEFAULT_STATUSES_LIMIT prev_path = statuses_api_v1_account_url(since_id: @statuses.first.id) unless @statuses.empty? @@ -98,6 +103,9 @@ class Api::V1::AccountsController < ApiController def search limit = params[:limit] ? [DEFAULT_ACCOUNTS_LIMIT, params[:limit].to_i].min : DEFAULT_ACCOUNTS_LIMIT @accounts = SearchService.new.call(params[:q], limit, params[:resolve] == 'true') + + set_account_counters_maps(@accounts) + render action: :index end diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 604e2969d5d..b2b432a6b16 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -13,14 +13,19 @@ class Api::V1::StatusesController < ApiController def context @context = OpenStruct.new(ancestors: @status.ancestors(current_account), descendants: @status.descendants(current_account)) - set_maps([@status] + @context[:ancestors] + @context[:descendants]) + statuses = [@status] + @context[:ancestors] + @context[:descendants] + + set_maps(statuses) + set_counters_maps(statuses) end def reblogged_by results = @status.reblogs.paginate_by_max_id(DEFAULT_ACCOUNTS_LIMIT, params[:max_id], params[:since_id]) - accounts = Account.where(id: results.map(&:account_id)).with_counters.map { |a| [a.id, a] }.to_h + accounts = Account.where(id: results.map(&:account_id)).map { |a| [a.id, a] }.to_h @accounts = results.map { |r| accounts[r.account_id] } + set_account_counters_maps(@accounts) + next_path = reblogged_by_api_v1_status_url(max_id: results.last.id) if results.size == DEFAULT_ACCOUNTS_LIMIT prev_path = reblogged_by_api_v1_status_url(since_id: results.first.id) unless results.empty? @@ -31,9 +36,11 @@ class Api::V1::StatusesController < ApiController def favourited_by results = @status.favourites.paginate_by_max_id(DEFAULT_ACCOUNTS_LIMIT, params[:max_id], params[:since_id]) - accounts = Account.where(id: results.map(&:account_id)).with_counters.map { |a| [a.id, a] }.to_h + accounts = Account.where(id: results.map(&:account_id)).map { |a| [a.id, a] }.to_h @accounts = results.map { |f| accounts[f.account_id] } + set_account_counters_maps(@accounts) + next_path = favourited_by_api_v1_status_url(max_id: results.last.id) if results.size == DEFAULT_ACCOUNTS_LIMIT prev_path = favourited_by_api_v1_status_url(since_id: results.first.id) unless results.empty? diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3a4c95db47e..283f950ccb1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -14,6 +14,7 @@ class ApplicationController < ActionController::Base before_action :store_current_location, except: :raise_not_found, unless: :devise_controller? before_action :set_locale + before_action :check_rack_mini_profiler def raise_not_found raise ActionController::RoutingError, "No route matches #{params[:unmatched_route]}" @@ -31,6 +32,10 @@ class ApplicationController < ActionController::Base I18n.locale = I18n.default_locale end + def check_rack_mini_profiler + Rack::MiniProfiler.authorize_request if current_user && current_user.admin? + end + protected def not_found diff --git a/app/models/status.rb b/app/models/status.rb index 1bf4b49c907..53987c3ec7a 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -71,7 +71,7 @@ class Status < ApplicationRecord def ancestors(account = nil) ids = (Status.find_by_sql(['WITH RECURSIVE search_tree(id, in_reply_to_id, path) AS (SELECT id, in_reply_to_id, ARRAY[id] FROM statuses WHERE id = ? UNION ALL SELECT statuses.id, statuses.in_reply_to_id, path || statuses.id FROM search_tree JOIN statuses ON statuses.id = search_tree.in_reply_to_id WHERE NOT statuses.id = ANY(path)) SELECT id FROM search_tree ORDER BY path DESC', id]) - [self]).pluck(:id) - statuses = Status.where(id: ids).with_counters.with_includes.group_by(&:id) + statuses = Status.where(id: ids).with_includes.group_by(&:id) results = ids.map { |id| statuses[id].first } results = results.reject { |status| account.blocking?(status.account) } unless account.nil? @@ -80,7 +80,7 @@ class Status < ApplicationRecord def descendants(account = nil) ids = (Status.find_by_sql(['WITH RECURSIVE search_tree(id, path) AS (SELECT id, ARRAY[id] FROM statuses WHERE id = ? UNION ALL SELECT statuses.id, path || statuses.id FROM search_tree JOIN statuses ON statuses.in_reply_to_id = search_tree.id WHERE NOT statuses.id = ANY(path)) SELECT id FROM search_tree ORDER BY path', id]) - [self]).pluck(:id) - statuses = Status.where(id: ids).with_counters.with_includes.group_by(&:id) + statuses = Status.where(id: ids).with_includes.group_by(&:id) results = ids.map { |id| statuses[id].first } results = results.reject { |status| account.blocking?(status.account) } unless account.nil? diff --git a/app/services/search_service.rb b/app/services/search_service.rb index c4cffda13fb..598c7d02c4f 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -12,7 +12,7 @@ class SearchService < BaseService Account.search_for("#{username} #{domain}") end - results = results.limit(limit).with_counters + results = results.limit(limit) if resolve && results.empty? && !domain.nil? results = [FollowRemoteAccountService.new.call("#{username}@#{domain}")] diff --git a/config/initializers/mini_profiler.rb b/config/initializers/mini_profiler.rb new file mode 100644 index 00000000000..265783618d7 --- /dev/null +++ b/config/initializers/mini_profiler.rb @@ -0,0 +1,17 @@ +require 'rack-mini-profiler' + +Rack::MiniProfilerRails.initialize!(Rails.application) + +Rails.application.middleware.swap(Rack::Deflater, Rack::MiniProfiler) +Rails.application.middleware.swap(Rack::MiniProfiler, Rack::Deflater) + +Rack::MiniProfiler.config.storage = Rack::MiniProfiler::MemoryStore + +if Rails.env.production? + Rack::MiniProfiler.config.storage_options = { + host: ENV.fetch('REDIS_HOST') { 'localhost' }, + port: ENV.fetch('REDIS_PORT') { 6379 }, + } + + Rack::MiniProfiler.config.storage = Rack::MiniProfiler::RedisStore +end