Commit 15e3d337 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'zj-commit-cache' into 'master'

Cache commits on the repository model

See merge request gitlab-org/gitlab-ce!14863
parents aee7f0d0 3411fef1
...@@ -249,9 +249,7 @@ module Ci ...@@ -249,9 +249,7 @@ module Ci
end end
def commit def commit
@commit ||= project.commit(sha) @commit ||= project.commit_by(oid: sha)
rescue
nil
end end
def branch? def branch?
......
...@@ -540,6 +540,10 @@ class Project < ActiveRecord::Base ...@@ -540,6 +540,10 @@ class Project < ActiveRecord::Base
repository.commit(ref) repository.commit(ref)
end end
def commit_by(oid:)
repository.commit_by(oid: oid)
end
# ref can't be HEAD, can only be branch/tag name or SHA # ref can't be HEAD, can only be branch/tag name or SHA
def latest_successful_builds_for(ref = default_branch) def latest_successful_builds_for(ref = default_branch)
latest_pipeline = pipelines.latest_successful_for(ref) latest_pipeline = pipelines.latest_successful_for(ref)
...@@ -553,7 +557,7 @@ class Project < ActiveRecord::Base ...@@ -553,7 +557,7 @@ class Project < ActiveRecord::Base
def merge_base_commit(first_commit_id, second_commit_id) def merge_base_commit(first_commit_id, second_commit_id)
sha = repository.merge_base(first_commit_id, second_commit_id) sha = repository.merge_base(first_commit_id, second_commit_id)
repository.commit(sha) if sha commit_by(oid: sha) if sha
end end
def saved? def saved?
......
...@@ -76,6 +76,7 @@ class Repository ...@@ -76,6 +76,7 @@ class Repository
@full_path = full_path @full_path = full_path
@disk_path = disk_path || full_path @disk_path = disk_path || full_path
@project = project @project = project
@commit_cache = {}
end end
def ==(other) def ==(other)
...@@ -103,18 +104,17 @@ class Repository ...@@ -103,18 +104,17 @@ class Repository
def commit(ref = 'HEAD') def commit(ref = 'HEAD')
return nil unless exists? return nil unless exists?
return ref if ref.is_a?(::Commit)
commit = find_commit(ref)
if ref.is_a?(Gitlab::Git::Commit) end
ref
else
Gitlab::Git::Commit.find(raw_repository, ref)
end
commit = ::Commit.new(commit, @project) if commit # Finding a commit by the passed SHA
commit # Also takes care of caching, based on the SHA
rescue Rugged::OdbError, Rugged::TreeError def commit_by(oid:)
nil return @commit_cache[oid] if @commit_cache.key?(oid)
@commit_cache[oid] = find_commit(oid)
end end
def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil) def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil)
...@@ -231,7 +231,7 @@ class Repository ...@@ -231,7 +231,7 @@ class Repository
# branches or tags, but we want to keep some of these commits around, for # branches or tags, but we want to keep some of these commits around, for
# example if they have comments or CI builds. # example if they have comments or CI builds.
def keep_around(sha) def keep_around(sha)
return unless sha && commit(sha) return unless sha && commit_by(oid: sha)
return if kept_around?(sha) return if kept_around?(sha)
...@@ -1064,6 +1064,18 @@ class Repository ...@@ -1064,6 +1064,18 @@ class Repository
private private
# TODO Generice finder, later split this on finders by Ref or Oid
# gitlab-org/gitlab-ce#39239
def find_commit(oid_or_ref)
commit = if oid_or_ref.is_a?(Gitlab::Git::Commit)
oid_or_ref
else
Gitlab::Git::Commit.find(raw_repository, oid_or_ref)
end
::Commit.new(commit, @project) if commit
end
def blob_data_at(sha, path) def blob_data_at(sha, path)
blob = blob_at(sha, path) blob = blob_at(sha, path)
return unless blob return unless blob
...@@ -1102,12 +1114,12 @@ class Repository ...@@ -1102,12 +1114,12 @@ class Repository
def last_commit_for_path_by_gitaly(sha, path) def last_commit_for_path_by_gitaly(sha, path)
c = raw_repository.gitaly_commit_client.last_commit_for_path(sha, path) c = raw_repository.gitaly_commit_client.last_commit_for_path(sha, path)
commit(c) commit_by(oid: c)
end end
def last_commit_for_path_by_rugged(sha, path) def last_commit_for_path_by_rugged(sha, path)
sha = last_commit_id_for_path_by_shelling_out(sha, path) sha = last_commit_id_for_path_by_shelling_out(sha, path)
commit(sha) commit_by(oid: sha)
end end
def last_commit_id_for_path_by_shelling_out(sha, path) def last_commit_id_for_path_by_shelling_out(sha, path)
......
---
title: Cache commits fetched from the repository
merge_request:
author:
type: performance
...@@ -72,7 +72,8 @@ module Gitlab ...@@ -72,7 +72,8 @@ module Gitlab
decorate(repo, commit) if commit decorate(repo, commit) if commit
rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError,
Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository,
Rugged::OdbError, Rugged::TreeError
nil nil
end end
......
...@@ -46,7 +46,7 @@ describe Projects::PipelinesController do ...@@ -46,7 +46,7 @@ describe Projects::PipelinesController do
context 'when performing gitaly calls', :request_store do context 'when performing gitaly calls', :request_store do
it 'limits the Gitaly requests' do it 'limits the Gitaly requests' do
expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(10) expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(8)
end end
end end
end end
......
...@@ -86,7 +86,7 @@ describe MergeRequest do ...@@ -86,7 +86,7 @@ describe MergeRequest do
context 'when the target branch does not exist' do context 'when the target branch does not exist' do
before do before do
project.repository.raw_repository.delete_branch(subject.target_branch) project.repository.rm_branch(subject.author, subject.target_branch)
end end
it 'returns nil' do it 'returns nil' do
...@@ -1388,7 +1388,7 @@ describe MergeRequest do ...@@ -1388,7 +1388,7 @@ describe MergeRequest do
context 'when the target branch does not exist' do context 'when the target branch does not exist' do
before do before do
subject.project.repository.raw_repository.delete_branch(subject.target_branch) subject.project.repository.rm_branch(subject.author, subject.target_branch)
end end
it 'returns nil' do it 'returns nil' do
......
...@@ -2260,4 +2260,24 @@ describe Repository do ...@@ -2260,4 +2260,24 @@ describe Repository do
end end
end end
end end
describe 'commit cache' do
set(:project) { create(:project, :repository) }
it 'caches based on SHA' do
# Gets the commit oid, and warms the cache
oid = project.commit.id
expect(Gitlab::Git::Commit).not_to receive(:find).once
project.commit_by(oid: oid)
end
it 'caches nil values' do
expect(Gitlab::Git::Commit).to receive(:find).once
project.commit_by(oid: '1' * 40)
project.commit_by(oid: '1' * 40)
end
end
end end
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