Skip to content
Snippets Groups Projects
Commit 606ddbea authored by Stan Hu's avatar Stan Hu
Browse files

Optimize MergeRequestDiff#last_commit_sha

The previous implementation would load all the MergeRequestDiffCommit
entries into memory and retrieve the SHA from the latest entry. We can
save a bit of database query time and memory by using the database to
retrieve the first item from the database.

Seen while investigating Todo API performance.
(https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25711)
parent 615c14b2
No related merge requests found
Pipeline #50133712 failed
......@@ -118,7 +118,7 @@ def commits
end
def last_commit_sha
commit_shas.first
merge_request_diff_commits.first&.sha
end
def first_commit
......
require 'spec_helper'
describe MergeRequestDiff do
let(:diff_with_commits) { create(:merge_request).merge_request_diff }
let(:merge_request) { create(:merge_request) }
let(:diff_with_commits) { merge_request.merge_request_diff }
describe 'create new record' do
subject { diff_with_commits }
......@@ -224,6 +225,18 @@
end
end
describe '#last_commit_sha' do
let(:commit_shas) { diff_with_commits.commit_shas }
let(:sorted_commits) { merge_request.merge_request_diff.merge_request_diff_commits.sort_by { |x| x.committed_date } }
let(:last_commit_sha) { diff_with_commits.last_commit_sha }
it 'returns the most recent commit SHA' do
expect(last_commit_sha).to eq(commit_shas.first)
# Sanity check the sort order
expect(last_commit_sha).to eq(sorted_commits.last.sha)
end
end
describe '#commits_by_shas' do
let(:commit_shas) { diff_with_commits.commit_shas }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment