Commit c6273ec5 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Avoid re-fetching merge-base SHA from Gitaly unnecessarily

parent 7734e85b
class Compare class Compare
include Gitlab::Utils::StrongMemoize
delegate :same, :head, :base, to: :@compare delegate :same, :head, :base, to: :@compare
attr_reader :project attr_reader :project
...@@ -11,9 +13,10 @@ class Compare ...@@ -11,9 +13,10 @@ class Compare
end end
end end
def initialize(compare, project, straight: false) def initialize(compare, project, base_sha: nil, straight: false)
@compare = compare @compare = compare
@project = project @project = project
@base_sha = base_sha
@straight = straight @straight = straight
end end
...@@ -22,40 +25,41 @@ class Compare ...@@ -22,40 +25,41 @@ class Compare
end end
def start_commit def start_commit
return @start_commit if defined?(@start_commit) strong_memoize(:start_commit) do
commit = @compare.base
commit = @compare.base ::Commit.new(commit, project) if commit
@start_commit = commit ? ::Commit.new(commit, project) : nil end
end end
def head_commit def head_commit
return @head_commit if defined?(@head_commit) strong_memoize(:head_commit) do
commit = @compare.head
commit = @compare.head ::Commit.new(commit, project) if commit
@head_commit = commit ? ::Commit.new(commit, project) : nil end
end end
alias_method :commit, :head_commit alias_method :commit, :head_commit
def base_commit def base_commit
return @base_commit if defined?(@base_commit) strong_memoize(:base_commit) do
return unless start_commit && head_commit
return OpenStruct.new(sha: @base_sha) if @base_sha
@base_commit = if start_commit && head_commit project.merge_base_commit(start_commit.id, head_commit.id)
project.merge_base_commit(start_commit.id, head_commit.id) end
else
nil
end
end end
def start_commit_sha def start_commit_sha
start_commit.try(:sha) start_commit&.sha
end end
def base_commit_sha def base_commit_sha
base_commit.try(:sha) base_commit&.sha
end end
def head_commit_sha def head_commit_sha
commit.try(:sha) commit&.sha
end end
def raw_diffs(*args) def raw_diffs(*args)
......
...@@ -10,9 +10,14 @@ class CompareService ...@@ -10,9 +10,14 @@ class CompareService
@start_ref_name = new_start_ref_name @start_ref_name = new_start_ref_name
end end
def execute(target_project, target_ref, straight: false) def execute(target_project, target_ref, base_sha: nil, straight: false)
raw_compare = target_project.repository.compare_source_branch(target_ref, start_project.repository, start_ref_name, straight: straight) raw_compare = target_project.repository.compare_source_branch(target_ref, start_project.repository, start_ref_name, straight: straight)
Compare.new(raw_compare, target_project, straight: straight) if raw_compare return unless raw_compare
Compare.new(raw_compare,
target_project,
base_sha: base_sha,
straight: straight)
end end
end end
---
title: Avoid re-fetching merge-base SHA from Gitaly unnecessarily
merge_request:
author:
type: performance
...@@ -44,7 +44,11 @@ module Gitlab ...@@ -44,7 +44,11 @@ module Gitlab
project.commit(head_sha) project.commit(head_sha)
else else
straight = start_sha == base_sha straight = start_sha == base_sha
CompareService.new(project, head_sha).execute(project, start_sha, straight: straight)
CompareService.new(project, head_sha).execute(project,
start_sha,
base_sha: base_sha,
straight: straight)
end end
end end
end end
......
...@@ -59,6 +59,36 @@ describe Compare do ...@@ -59,6 +59,36 @@ describe Compare do
end end
end end
describe '#base_commit_sha' do
it 'returns @base_sha if it is present' do
expect(project).not_to receive(:merge_base_commit)
sha = double
service = described_class.new(raw_compare, project, base_sha: sha)
expect(service.base_commit_sha).to eq(sha)
end
it 'fetches merge base SHA from repo when @base_sha is nil' do
expect(project).to receive(:merge_base_commit)
.with(start_commit.id, head_commit.id)
.once
.and_call_original
expect(subject.base_commit_sha)
.to eq(project.repository.merge_base(start_commit.id, head_commit.id))
end
it 'is memoized on first call' do
expect(project).to receive(:merge_base_commit)
.with(start_commit.id, head_commit.id)
.once
.and_call_original
3.times { subject.base_commit_sha }
end
end
describe '#diff_refs' do describe '#diff_refs' do
it 'uses base_commit sha as base_sha' do it 'uses base_commit sha as base_sha' do
expect(subject).to receive(:base_commit).at_least(:once).and_call_original expect(subject).to receive(:base_commit).at_least(:once).and_call_original
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment