From b6868ce9eab34aa47fdc7e28ae73365eb7a04e0d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 13 Jun 2022 21:53:40 +0100 Subject: [PATCH 1/2] perf(dossier): add dossier export benchmarks --- lib/tasks/benchmarks.rake | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 lib/tasks/benchmarks.rake diff --git a/lib/tasks/benchmarks.rake b/lib/tasks/benchmarks.rake new file mode 100644 index 000000000..f363e17e7 --- /dev/null +++ b/lib/tasks/benchmarks.rake @@ -0,0 +1,11 @@ +namespace :benchmarks do + desc 'Benchmark exports' + task exports: :environment do + p_45964 = Procedure.find(45964) + p_55824 = Procedure.find(55824) + Benchmark.bm do |x| + x.report("Démarche 45964") { ProcedureExportService.new(p_45964, p_45964.dossiers).to_xlsx } + x.report("Démarche 55824") { ProcedureExportService.new(p_55824, p_55824.dossiers).to_xlsx } + end + end +end From 564daeffe81e03fbe6a27d74529455cc26d873f5 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 13 Jun 2022 21:54:57 +0100 Subject: [PATCH 2/2] perf(dossier): improuve dossier preloading perf --- app/models/champ.rb | 5 + app/models/dossier.rb | 124 ++++++++++++++----- app/models/type_de_champ.rb | 4 + app/services/procedure_export_service.rb | 9 +- spec/system/instructeurs/instruction_spec.rb | 2 +- 5 files changed, 109 insertions(+), 35 deletions(-) diff --git a/app/models/champ.rb b/app/models/champ.rb index 14a0992f7..19f9a8a60 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -49,6 +49,7 @@ class Champ < ApplicationRecord :dossier_link?, :titre_identite?, :header_section?, + :siret?, :stable_id, to: :type_de_champ @@ -76,6 +77,10 @@ class Champ < ApplicationRecord !private? end + def child? + parent_id.present? + end + def sections @sections ||= dossier&.sections_for(self) end diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 650b8de68..89a129494 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -250,6 +250,7 @@ class Dossier < ApplicationRecord :followers_instructeurs, :traitement, :groupe_instructeur, + :etablissement, procedure: [ :groupe_instructeurs, :draft_types_de_champ, @@ -257,8 +258,7 @@ class Dossier < ApplicationRecord :published_types_de_champ, :published_types_de_champ_private ], - avis: [:claimant, :expert], - etablissement: :champ + avis: [:claimant, :expert] ).order(depose_at: 'asc') } scope :en_cours, -> { not_archived.state_en_construction_ou_instruction } @@ -435,39 +435,101 @@ class Dossier < ApplicationRecord validates :individual, presence: true, if: -> { revision.procedure.for_individual? } validates :groupe_instructeur, presence: true, if: -> { !brouillon? } - EXPORT_BATCH_SIZE = 5000 + EXPORT_BATCH_SIZE = 2000 def self.downloadable_sorted_batch - dossiers = downloadable_sorted.to_a - (dossiers.size.to_f / EXPORT_BATCH_SIZE).ceil.times do |i| - start_index = i * EXPORT_BATCH_SIZE - end_index = start_index + EXPORT_BATCH_SIZE - 1 - load_champs(dossiers[start_index..end_index]) - end - dossiers + ExportPreloader.new(self).in_batches end - def self.load_champs(dossiers) - ::ActiveRecord::Associations::Preloader.new.preload(dossiers, { - champs: { - type_de_champ: [], - etablissement: :champ, - piece_justificative_file_attachment: :blob, - champs: [ - type_de_champ: [], - piece_justificative_file_attachment: :blob - ] - }, - champs_private: { - type_de_champ: [], - etablissement: :champ, - piece_justificative_file_attachment: :blob, - champs: [ - type_de_champ: [], - piece_justificative_file_attachment: :blob - ] - } - }) + class ExportPreloader + def initialize(dossiers) + @dossiers = dossiers + end + + def in_batches + dossiers = @dossiers.downloadable_sorted.to_a + dossiers.each_slice(EXPORT_BATCH_SIZE) { |slice| load_dossiers(slice) } + dossiers + end + + private + + # returns: { revision_id : { type_de_champ_id : position } } + def positions + @positions ||= ProcedureRevisionTypeDeChamp + .where(revision_id: @dossiers.distinct.pluck(:revision_id)) + .select(:revision_id, :type_de_champ_id, :position) + .group_by(&:revision_id) + .transform_values do |coordinates| + coordinates.index_by(&:type_de_champ_id).transform_values(&:position) + end + end + + def load_dossiers(dossiers) + all_champs = Champ + .includes(:type_de_champ, piece_justificative_file_attachment: :blob) + .where(dossier_id: dossiers) + .to_a + + load_etablissements(all_champs) + + children_champs, root_champs = all_champs.partition(&:child?) + champs_by_dossier = root_champs.group_by(&:dossier_id) + champs_by_dossier_by_parent = children_champs + .group_by(&:dossier_id) + .transform_values do |champs| + champs.group_by(&:parent_id) + end + + dossiers.each do |dossier| + load_dossier(dossier, champs_by_dossier[dossier.id], champs_by_dossier_by_parent[dossier.id]) + end + end + + def load_etablissements(champs) + champs_siret = champs.filter(&:siret?) + etablissements_by_id = Etablissement.where(id: champs_siret.map(&:etablissement_id).compact).index_by(&:id) + champs_siret.each do |champ| + etablissement = etablissements_by_id[champ.etablissement_id] + champ.association(:etablissement).target = etablissement + if etablissement + etablissement.association(:champ).target = champ + end + end + end + + def load_dossier(dossier, champs, children_by_parent = {}) + champs_public, champs_private = champs.partition(&:public?) + + load_champs(dossier, :champs, champs_public, dossier) + load_champs(dossier, :champs_private, champs_private, dossier) + + # Load repetition children champs + champs.filter(&:repetition?).each do |parent_champ| + champs = children_by_parent[parent_champ.id] || [] + parent_champ.association(:dossier).target = dossier + + load_champs(parent_champ, :champs, champs, dossier) + parent_champ.association(:champs).set_inverse_instance(parent_champ) + end + + # We need to do this because of the check on `Etablissement#champ` in + # `Etablissement#libelle_for_export`. By assigning `nil` to `target` we mark association + # as loaded and so the check on `Etablissement#champ` will not trigger n+1 query. + if dossier.etablissement + dossier.etablissement.association(:champ).target = nil + end + end + + def load_champs(parent, name, champs, dossier) + champs.each do |champ| + champ.association(:dossier).target = dossier + end + + parent.association(name).target = champs.sort_by do |champ| + positions[dossier.revision_id][champ.type_de_champ_id] + end + end end def user_deleted? diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index 86bc5ac47..135d10c91 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -203,6 +203,10 @@ class TypeDeChamp < ApplicationRecord type_champ == TypeDeChamp.type_champs.fetch(:dossier_link) end + def siret? + type_champ == TypeDeChamp.type_champs.fetch(:siret) + end + def piece_justificative? type_champ == TypeDeChamp.type_champs.fetch(:piece_justificative) || type_champ == TypeDeChamp.type_champs.fetch(:titre_identite) end diff --git a/app/services/procedure_export_service.rb b/app/services/procedure_export_service.rb index d08833bdc..9d496b3ac 100644 --- a/app/services/procedure_export_service.rb +++ b/app/services/procedure_export_service.rb @@ -4,7 +4,6 @@ class ProcedureExportService def initialize(procedure, dossiers) @procedure = procedure @dossiers = dossiers - @tables = [:dossiers, :etablissements, :avis] + champs_repetables_options end def to_csv @@ -15,8 +14,10 @@ class ProcedureExportService def to_xlsx @dossiers = @dossiers.downloadable_sorted_batch + tables = [:dossiers, :etablissements, :avis] + champs_repetables_options + # We recursively build multi page spreadsheet - io = @tables.reduce(nil) do |package, table| + io = tables.reduce(nil) do |package, table| SpreadsheetArchitect.to_axlsx_package(options_for(table, :xlsx), package) end.to_stream create_blob(io, :xlsx) @@ -24,8 +25,10 @@ class ProcedureExportService def to_ods @dossiers = @dossiers.downloadable_sorted_batch + tables = [:dossiers, :etablissements, :avis] + champs_repetables_options + # We recursively build multi page spreadsheet - io = StringIO.new(@tables.reduce(nil) do |spreadsheet, table| + io = StringIO.new(tables.reduce(nil) do |spreadsheet, table| SpreadsheetArchitect.to_rodf_spreadsheet(options_for(table, :ods), spreadsheet) end.bytes) create_blob(io, :ods) diff --git a/spec/system/instructeurs/instruction_spec.rb b/spec/system/instructeurs/instruction_spec.rb index fcc541a97..d10675e5d 100644 --- a/spec/system/instructeurs/instruction_spec.rb +++ b/spec/system/instructeurs/instruction_spec.rb @@ -4,7 +4,7 @@ describe 'Instructing a dossier:', js: true do let(:password) { 'my-s3cure-p4ssword' } let!(:instructeur) { create(:instructeur, password: password) } - let!(:procedure) { create(:procedure, :published, instructeurs: [instructeur]) } + let!(:procedure) { create(:procedure, :with_type_de_champ, :published, instructeurs: [instructeur]) } let!(:dossier) { create(:dossier, :en_construction, :with_entreprise, procedure: procedure) } context 'the instructeur is also a user' do scenario 'a instructeur can fill a dossier' do