From 04071073e5c73cbb6407e3c9366c9cef4cf6f19d Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Fri, 29 Sep 2023 13:09:54 +0200 Subject: [PATCH 1/2] refactor(instructeurs): faster procedures listing --- .../instructeurs/procedures_controller.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 4c2b4d121..686f07f10 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -10,22 +10,25 @@ module Instructeurs all_procedures = current_instructeur .procedures .kept + + all_procedures_for_listing = all_procedures .with_attached_logo - .includes(:defaut_groupe_instructeur) - .includes(:dossiers) dossiers = current_instructeur.dossiers .joins(groupe_instructeur: :procedure) .where(procedures: { hidden_at: nil }) + # .uniq is much more faster than a distinct on a joint column + procedures_dossiers_en_cours = dossiers.joins(:revision).en_cours.pluck(ProcedureRevision.arel_table[:procedure_id]).uniq + @procedures = all_procedures.order(closed_at: :desc, unpublished_at: :desc, published_at: :desc, created_at: :desc) - publiees_or_closes_with_dossiers_en_cours = all_procedures.publiees.or(all_procedures.closes.where(dossiers: { id: dossiers.en_cours.ids })) + publiees_or_closes_with_dossiers_en_cours = all_procedures_for_listing.publiees.or(all_procedures.closes.where(id: procedures_dossiers_en_cours)) @procedures_en_cours = publiees_or_closes_with_dossiers_en_cours.order(published_at: :desc).page(params[:page]).per(ITEMS_PER_PAGE) - closes_with_no_dossier_en_cours = all_procedures.closes.excluding(all_procedures.closes.where(dossiers: { id: dossiers.en_cours.ids })) + closes_with_no_dossier_en_cours = all_procedures.closes.excluding(all_procedures.closes.where(id: procedures_dossiers_en_cours)) @procedures_closes = closes_with_no_dossier_en_cours.order(created_at: :desc).page(params[:page]).per(ITEMS_PER_PAGE) - @procedures_draft = all_procedures.brouillons.order(created_at: :desc).page(params[:page]).per(ITEMS_PER_PAGE) + @procedures_draft = all_procedures_for_listing.brouillons.order(created_at: :desc).page(params[:page]).per(ITEMS_PER_PAGE) @procedures_en_cours_count = publiees_or_closes_with_dossiers_en_cours.count - @procedures_draft_count = all_procedures.brouillons.count + @procedures_draft_count = all_procedures_for_listing.brouillons.count @procedures_closes_count = closes_with_no_dossier_en_cours.count @dossiers_count_per_procedure = dossiers.by_statut('tous').group('groupe_instructeurs.procedure_id').reorder(nil).count From 8678986808856932f2b6b64ab85106647e008611 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Fri, 29 Sep 2023 14:18:01 +0200 Subject: [PATCH 2/2] chore(benchmark): tooling for benchmarking controller action --- Gemfile | 1 + Gemfile.lock | 2 + lib/tasks/benchmarks.rake | 81 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/Gemfile b/Gemfile index 25829f8ee..f280c3160 100644 --- a/Gemfile +++ b/Gemfile @@ -116,6 +116,7 @@ group :test do end group :development do + gem 'benchmark-ips', require: false gem 'brakeman', require: false gem 'haml-lint' gem 'letter_opener_web' diff --git a/Gemfile.lock b/Gemfile.lock index eaa0e86bb..b64652702 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -121,6 +121,7 @@ GEM activesupport (>= 3.1) caxlsx (>= 2.0.2) bcrypt (3.1.18) + benchmark-ips (2.12.0) better_html (1.0.16) actionview (>= 4.0) activesupport (>= 4.0) @@ -813,6 +814,7 @@ DEPENDENCIES anchored axe-core-rspec bcrypt + benchmark-ips bootsnap (>= 1.4.4) brakeman browser diff --git a/lib/tasks/benchmarks.rake b/lib/tasks/benchmarks.rake index b33904134..4dc88bd5a 100644 --- a/lib/tasks/benchmarks.rake +++ b/lib/tasks/benchmarks.rake @@ -38,4 +38,85 @@ namespace :benchmarks do x.report("Démarche 68139") { TagsSubstitutionConcern::TagsParser.parse(procedure.attestation_template.body) } end end + + # Benchmark une action Rails spécifique, y compris le temps de génération des views. + # Optionnellement, compare avec une autre implémentation de l'action + # sur la même branche ou sur deux branches git. + # + # Le benchmark se fait en deux temps (2 processes ruby séparés à lancer l'un après l'autre) + # en stockant les résultats intermédiaires dans un fichier. + # + # === Prérequis === + # + # 1. On fake l'authentification. Commenter le `before_action :authenticate_*` concerné + # + # 2. Override dans le controller le(s) `current_*` concernés + # + # Par exemple : + # + # def current_instructeur + # @current_instructeur ||= Instructeur.find(12_345) + # end + # + # 3. Pour se rapprocher un peu plus de l'environnement de prod, activer le cache: + # `rails dev:cache` puis redémarrer le serveur (à refaire à la fin pour l'enlever) + # + # 4a. Pour comparer l'implémentation de deux actions sur la même branche (par ex `index` et `index_main`): + # la seconde implémentation doit : + # - avoir une route définie (ie. `get :index_main`) + # - adapter les `before_action` + # - la conclure par `render :index` si la vue est identique, ou adapter les vues. + # + # Exécuter 2 foix: + # + # rake benchmarks:action[Instructeurs::ProceduresController,index,index_main] + # + # 4b. Pour comparer l'implémentation d'une même action sur 2 branches, + # exécuter sur chacune d'entre elle : + # + # rake benchmarks:action[Instructeurs::ProceduresController,index,index] + # + # Attention : penser à refaire les étapes 1 et 2 sur la seconde branche ! + # + desc "Benchmark a Rails action" + task :action, [:controller, :action1, :action2] => :environment do |_, args| + require 'benchmark/ips' + + controller_class = args[:controller].constantize + action1_name = args[:action1] + action2_name = args[:action2] + + warden_mock = Struct.new(:user) do + def authenticate(*_args); end + end + + warden = warden_mock.new(nil) + + setup_controller_instance = Proc.new do |controller_class, warden| + controller = controller_class.new + controller.request = ActionDispatch::TestRequest.create + controller.response = ActionDispatch::TestResponse.new + controller.request.env['warden'] = warden + controller + end + + Benchmark.ips do |x| + x.config(time: 30, warmup: 10) + x.hold! 'tmp/benchmarks_action_hold' # run in different processes so we can switch branches + + x.report("#{controller_class}##{action1_name}1") do + controller = setup_controller_instance.call(controller_class, warden) + controller.process(action1_name) + end + + if action2_name + x.report("#{controller_class}##{action2_name}2") do + controller = setup_controller_instance.call(controller_class, warden) + controller.process(action2_name) + end + end + + x.compare! + end + end end