Merge branch 'perf.slow-issuable' into 'master'

app/models/repository.rb

@@ -60,6 +60,10 @@ class Repository
     @project = project
   end
 
+  def ==(other)
+    @disk_path == other.disk_path
+  end
+
   def raw_repository
     return nil unless full_path
 
@@ -75,6 +79,10 @@ class Repository
     )
   end
 
+  def inspect
+    "#<#{self.class.name}:#{@disk_path}>"
+  end
+
   #
   # Git repository can contains some hidden refs like:
   #   /refs/notes/*
@@ -992,25 +1000,22 @@ class Repository
   end
 
   def with_repo_branch_commit(start_repository, start_branch_name)
-    return yield(nil) if start_repository.empty_repo?
+    return yield nil if start_repository.empty_repo?
 
-    branch_name_or_sha =
-      if start_repository == self
-        start_branch_name
-      else
-        tmp_ref = fetch_ref(
-          start_repository.path_to_repo,
-          "#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
-          "refs/tmp/#{SecureRandom.hex}/head"
-        )
+    if start_repository == self
+      yield commit(start_branch_name)
+    else
+      sha = start_repository.commit(start_branch_name).sha
 
-        start_repository.commit(start_branch_name).sha
+      if branch_commit = commit(sha)
+        yield branch_commit
+      else
+        with_repo_tmp_commit(
+          start_repository, start_branch_name, sha) do |tmp_commit|
+          yield tmp_commit
+        end
       end
-
-    yield(commit(branch_name_or_sha))
-
-  ensure
-    rugged.references.delete(tmp_ref) if tmp_ref
+    end
   end
 
   def add_remote(name, url)
@@ -1219,4 +1224,16 @@ class Repository
       .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
       .map { |c| commit(c) }
   end
+
+  def with_repo_tmp_commit(start_repository, start_branch_name, sha)
+    tmp_ref = fetch_ref(
+      start_repository.path_to_repo,
+      "#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}",
+      "refs/tmp/#{SecureRandom.hex}/head"
+    )
+
+    yield commit(sha)
+  ensure
+    rugged.references.delete(tmp_ref) if tmp_ref
+  end
 end

changelogs/unreleased/perf-slow-issuable.yml

+---
+title: Fix repository equality check and avoid fetching ref if the commit is already
+  available. This affects merge request creation performance
+merge_request: 13685
+author:
+type: other

spec/models/repository_spec.rb

@@ -923,13 +923,16 @@ describe Repository, models: true do
   describe '#update_branch_with_hooks' do
     let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature
     let(:new_rev) { 'a74ae73c1ccde9b974a70e82b901588071dc142a' } # commit whose parent is old_rev
+    let(:updating_ref) { 'refs/heads/feature' }
+    let(:target_project) { project }
+    let(:target_repository) { target_project.repository }
 
     context 'when pre hooks were successful' do
       before do
         service = Gitlab::Git::HooksService.new
         expect(Gitlab::Git::HooksService).to receive(:new).and_return(service)
         expect(service).to receive(:execute)
-          .with(committer, repository, old_rev, new_rev, 'refs/heads/feature')
+          .with(committer, target_repository, old_rev, new_rev, updating_ref)
           .and_yield(service).and_return(true)
       end
 
@@ -960,6 +963,37 @@ describe Repository, models: true do
           expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev)
         end
       end
+
+      context 'when target project does not have the commit' do
+        let(:target_project) { create(:project, :empty_repo) }
+        let(:old_rev) { Gitlab::Git::BLANK_SHA }
+        let(:new_rev) { project.commit('feature').sha }
+        let(:updating_ref) { 'refs/heads/master' }
+
+        it 'fetch_ref and create the branch' do
+          expect(target_project.repository).to receive(:fetch_ref)
+            .and_call_original
+
+          GitOperationService.new(committer, target_repository)
+            .with_branch(
+              'master',
+              start_project: project,
+              start_branch_name: 'feature') { new_rev }
+
+          expect(target_repository.branch_names).to contain_exactly('master')
+        end
+      end
+
+      context 'when target project already has the commit' do
+        let(:target_project) { create(:project, :repository) }
+
+        it 'does not fetch_ref and just pass the commit' do
+          expect(target_repository).not_to receive(:fetch_ref)
+
+          GitOperationService.new(committer, target_repository)
+            .with_branch('feature', start_project: project) { new_rev }
+        end
+      end
     end
 
     context 'when temporary ref failed to be created from other project' do