diff --git a/changelogs/unreleased/60449-reduce-gitaly-calls-when-rendering-commits-in-md.yml b/changelogs/unreleased/60449-reduce-gitaly-calls-when-rendering-commits-in-md.yml new file mode 100644 index 0000000000000000000000000000000000000000..ef11e8743f62ed2ea3e6587d9d6223dbc87dcf3f --- /dev/null +++ b/changelogs/unreleased/60449-reduce-gitaly-calls-when-rendering-commits-in-md.yml @@ -0,0 +1,5 @@ +--- +title: Batch processing of commit refs in markdown processing +merge_request: 31037 +author: +type: performance diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index 0224dd8fcd1aab94ef5e6aaa949b7f60c1888a89..52af28ce8ecb8345c99dfc5e0bb85e9d1a9dd015 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -337,6 +337,24 @@ module Banzai @current_project_namespace_path ||= project&.namespace&.full_path end + def records_per_parent + @_records_per_project ||= {} + + @_records_per_project[object_class.to_s.underscore] ||= begin + hash = Hash.new { |h, k| h[k] = {} } + + parent_per_reference.each do |path, parent| + record_ids = references_per_parent[path] + + parent_records(parent, record_ids).each do |record| + hash[parent][record_identifier(record)] = record + end + end + + hash + end + end + private def full_project_path(namespace, project_ref) diff --git a/lib/banzai/filter/commit_reference_filter.rb b/lib/banzai/filter/commit_reference_filter.rb index c3e5ac41cb8c783c74ee981e97be7f9407b89c27..e1d7b36b9a2a904a3ce1786cf1487d1958ed95bb 100644 --- a/lib/banzai/filter/commit_reference_filter.rb +++ b/lib/banzai/filter/commit_reference_filter.rb @@ -19,12 +19,11 @@ module Banzai end def find_object(project, id) - return unless project.is_a?(Project) + return unless project.is_a?(Project) && project.valid_repo? - if project && project.valid_repo? - # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/43894 - Gitlab::GitalyClient.allow_n_plus_1_calls { project.commit(id) } - end + _, record = records_per_parent[project].detect { |k, _v| Gitlab::Git.shas_eql?(k, id) } + + record end def referenced_merge_request_commit_shas @@ -66,6 +65,14 @@ module Banzai private + def record_identifier(record) + record.id + end + + def parent_records(parent, ids) + parent.commits_by(oids: ids.to_a) + end + def noteable context[:noteable] end diff --git a/lib/banzai/filter/issuable_reference_filter.rb b/lib/banzai/filter/issuable_reference_filter.rb index 2963cba91e872f470ebaf07b031c703beaa4e310..b91ba9f7256db2accac8515fc789b90966ac12d4 100644 --- a/lib/banzai/filter/issuable_reference_filter.rb +++ b/lib/banzai/filter/issuable_reference_filter.rb @@ -3,22 +3,8 @@ module Banzai module Filter class IssuableReferenceFilter < AbstractReferenceFilter - def records_per_parent - @records_per_project ||= {} - - @records_per_project[object_class.to_s.underscore] ||= begin - hash = Hash.new { |h, k| h[k] = {} } - - parent_per_reference.each do |path, parent| - record_ids = references_per_parent[path] - - parent_records(parent, record_ids).each do |record| - hash[parent][record.iid.to_i] = record - end - end - - hash - end + def record_identifier(record) + record.iid.to_i end def find_object(parent, iid) diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index 1bc0335cfc0deb494149dce5c7385c93e2fd4f19..326703eea05175d1e8f276fe3bc4498386ce57b3 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -105,6 +105,17 @@ describe Banzai::Filter::CommitReferenceFilter do expect(doc.css('a').first[:href]).to eq(url) end + + context "a doc with many (29) strings that could be SHAs" do + let!(:oids) { noteable.commits.collect(&:id) } + + it 'makes only a single request to Gitaly' do + expect(Gitlab::GitalyClient).to receive(:allow_n_plus_1_calls).exactly(0).times + expect(Gitlab::Git::Commit).to receive(:batch_by_oid).once.and_call_original + + reference_filter("A big list of SHAs #{oids.join(", ")}", noteable: noteable) + end + end end end