diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index d4a56709edb5f1d88017106e730319d22d11c228..6a3fba93aea87bda18fd25a49f67d291ff0fa261 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -107,6 +107,7 @@ def merge_request_diff after_create :ensure_merge_request_diff after_update :clear_memoized_shas + after_update :clear_memoized_source_branch_exists after_update :reload_diff_if_branch_changed after_commit :ensure_metrics, on: [:create, :update], unless: :importing? after_commit :expire_etag_cache, unless: :importing? @@ -862,6 +863,10 @@ def clear_memoized_shas clear_memoization(:target_branch_head) end + def clear_memoized_source_branch_exists + clear_memoization(:source_branch_exists) + end + def reload_diff_if_branch_changed if (saved_change_to_source_branch? || saved_change_to_target_branch?) && (source_branch_head && target_branch_head) @@ -1109,9 +1114,17 @@ def target_project_namespace end def source_branch_exists? - return false unless self.source_project + if Feature.enabled?(:memoize_source_branch_merge_request, project) + strong_memoize(:source_branch_exists) do + next false unless self.source_project + + self.source_project.repository.branch_exists?(self.source_branch) + end + else + return false unless self.source_project - self.source_project.repository.branch_exists?(self.source_branch) + self.source_project.repository.branch_exists?(self.source_branch) + end end def target_branch_exists? diff --git a/changelogs/unreleased/209786-improve-performance-of-diffs_batch-diffs-metadata-source-branch-ex.yml b/changelogs/unreleased/209786-improve-performance-of-diffs_batch-diffs-metadata-source-branch-ex.yml new file mode 100644 index 0000000000000000000000000000000000000000..215d64bbacbcdd9c8e5ed48a7fd3fcfbf8e9e31e --- /dev/null +++ b/changelogs/unreleased/209786-improve-performance-of-diffs_batch-diffs-metadata-source-branch-ex.yml @@ -0,0 +1,5 @@ +--- +title: Further improve the performance for loading large diffs on a Merge request +merge_request: 34516 +author: +type: performance diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 332f5d62c62b758e89356d262e1b4c914ca69b29..4853fc2438e4333aac662aab61133b92a8318861 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1217,6 +1217,59 @@ def set_compare(merge_request) end end + describe "#source_branch_exists?" do + let(:merge_request) { subject } + let(:repository) { merge_request.source_project.repository } + + context 'when memoize_source_branch_merge_request feature is enabled' do + before do + stub_feature_flags(memoize_source_branch_merge_request: true) + end + + context 'when the source project is set' do + it 'memoizes the value and returns the result' do + expect(repository).to receive(:branch_exists?).once.with(merge_request.source_branch).and_return(true) + + 2.times { expect(merge_request.source_branch_exists?).to eq(true) } + end + end + + context 'when the source project is not set' do + before do + merge_request.source_project = nil + end + + it 'returns false' do + expect(merge_request.source_branch_exists?).to eq(false) + end + end + end + + context 'when memoize_source_branch_merge_request feature is disabled' do + before do + stub_feature_flags(memoize_source_branch_merge_request: false) + end + + context 'when the source project is set' do + it 'does not memoize the value and returns the result' do + expect(repository).to receive(:branch_exists?).twice.with(merge_request.source_branch).and_return(true) + + 2.times { expect(merge_request.source_branch_exists?).to eq(true) } + end + end + + context 'when the source project is not set' do + before do + merge_request.source_project = nil + end + + it 'returns false' do + expect(merge_request.source_branch_exists?).to eq(false) + end + end + end + end + describe '#default_merge_commit_message' do it 'includes merge information as the title' do request = build(:merge_request, source_branch: 'source', target_branch: 'target') diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index cd9eae2f2eed9ef46ac512b0d48ee11bbe311d89..14133731e37957f87070a0ac2573ce3123036b38 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -30,6 +30,7 @@ def conflicts_service(merge_request) it 'returns a falsey value when one of the MR branches is missing' do merge_request = create_merge_request('conflict-resolvable') merge_request.project.repository.rm_branch(merge_request.author, 'conflict-resolvable') + merge_request.clear_memoized_source_branch_exists expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey end