Merge branch '60449-reduce-gitaly-calls-when-rendering-commits-in-md' into 'master'

Lessen Gitaly N+1 errors in markdown processing

Closes #60449

See merge request gitlab-org/gitlab-ce!31037

(cherry picked from commit e17350be)

9d6fca6f Initial commit of WIP code for consideration
d82ac022 Make Rubocop happy, even though it is 100% wrong
978cf4af Cast the Gitaly commit object to a ::Commit
0e318279 If we pass in a commit object, don't attempt a lookup
ab01bedb Reload the commit object if it isn't a ::Commit
8360a202 Make #extract_valid_commit_references a private method
f98c77e8 Use scan instead of gsub
a5b87299 Extract finding of local matches to a new method
e5229999 Improve basic test of removing of N+1 situation
91d3d8c5 Add CHANGELOG entry for reduced Gitaly calls
a49d1d76 Extract #set_parent_path to reduce complexity
04d7b960 Batch load commit refs found in markdown
56b3d9ba Prepend internal ivar with _
582bec5c Fix typo in spec title
7e083b11 Track commits by their full SHA string
2d81a176 Combine and simplify guard clauses
83f3d461 Extract #records_per_parent to AbstractReferenceFilter

changelogs/unreleased/60449-reduce-gitaly-calls-when-rendering-commits-in-md.yml

+---
+title: Batch processing of commit refs in markdown processing
+merge_request: 31037
+author:
+type: performance

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)

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)
-        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
+        return unless project.is_a?(Project) && project.valid_repo?
+        _, 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

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)

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