Commit 9a99d8e4 authored by Yorick Peterse's avatar Yorick Peterse

Cache various Repository Git operations

This caches the output of the following methods:

* Repository#empty?
* Repository#has_visible_content?
* Repository#root_ref

The cache for Repository#has_visible_content? is flushed whenever a
commit is pushed to a new branch or an existing branch is removed.
The cache for Repository#root_ref is only flushed whenever a user
changes the default branch of a project. The cache for Repository#empty?
is never explicitly flushed as there's no need for it.
parent 7322c5a0
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.5.0 (unreleased) v 8.5.0 (unreleased)
- Cache various Repository methods to improve performance (Yorick Peterse)
- Ensure rake tasks that don't need a DB connection can be run without one - Ensure rake tasks that don't need a DB connection can be run without one
- Update New Relic gem to 3.14.1.311 (Stan Hu) - Update New Relic gem to 3.14.1.311 (Stan Hu)
- Add "visibility" flag to GET /projects api endpoint - Add "visibility" flag to GET /projects api endpoint
......
...@@ -790,6 +790,8 @@ class Project < ActiveRecord::Base ...@@ -790,6 +790,8 @@ class Project < ActiveRecord::Base
def change_head(branch) def change_head(branch)
# Cached divergent commit counts are based on repository head # Cached divergent commit counts are based on repository head
repository.expire_branch_cache repository.expire_branch_cache
repository.expire_root_ref_cache
gitlab_shell.update_repository_head(self.path_with_namespace, branch) gitlab_shell.update_repository_head(self.path_with_namespace, branch)
reload_default_branch reload_default_branch
end end
......
...@@ -44,7 +44,9 @@ class Repository ...@@ -44,7 +44,9 @@ class Repository
end end
def empty? def empty?
raw_repository.empty? return @empty unless @empty.nil?
@empty = cache.fetch(:empty?) { raw_repository.empty? }
end end
# #
...@@ -57,8 +59,12 @@ class Repository ...@@ -57,8 +59,12 @@ class Repository
# This method return true if repository contains some content visible in project page. # This method return true if repository contains some content visible in project page.
# #
def has_visible_content? def has_visible_content?
return @has_visible_content unless @has_visible_content.nil?
@has_visible_content = cache.fetch(:has_visible_content?) do
raw_repository.branch_count > 0 raw_repository.branch_count > 0
end end
end
def commit(id = 'HEAD') def commit(id = 'HEAD')
return nil unless raw_repository return nil unless raw_repository
...@@ -243,6 +249,16 @@ class Repository ...@@ -243,6 +249,16 @@ class Repository
end end
end end
def expire_root_ref_cache
cache.expire(:root_ref)
@root_ref = nil
end
def expire_has_visible_content_cache
cache.expire(:has_visible_content?)
@has_visible_content = nil
end
def rebuild_cache def rebuild_cache
cache_keys.each do |key| cache_keys.each do |key|
cache.expire(key) cache.expire(key)
...@@ -480,7 +496,7 @@ class Repository ...@@ -480,7 +496,7 @@ class Repository
end end
def root_ref def root_ref
@root_ref ||= raw_repository.root_ref @root_ref ||= cache.fetch(:root_ref) { raw_repository.root_ref }
end end
def commit_dir(user, path, message, branch) def commit_dir(user, path, message, branch)
......
...@@ -21,8 +21,12 @@ class GitPushService ...@@ -21,8 +21,12 @@ class GitPushService
project.repository.expire_cache project.repository.expire_cache
if push_remove_branch?(ref, newrev) if push_remove_branch?(ref, newrev)
project.repository.expire_has_visible_content_cache
@push_commits = [] @push_commits = []
elsif push_to_new_branch?(ref, oldrev) elsif push_to_new_branch?(ref, oldrev)
project.repository.expire_has_visible_content_cache
# Re-find the pushed commits. # Re-find the pushed commits.
if is_default_branch?(ref) if is_default_branch?(ref)
# Initial push to the default branch. Take the full history of that branch as "newly pushed". # Initial push to the default branch. Take the full history of that branch as "newly pushed".
......
...@@ -232,11 +232,92 @@ describe Repository, models: true do ...@@ -232,11 +232,92 @@ describe Repository, models: true do
end end
describe 'when there are branches' do describe 'when there are branches' do
before do it 'returns true' do
allow(repository.raw_repository).to receive(:branch_count).and_return(3) expect(repository.raw_repository).to receive(:branch_count).and_return(3)
expect(subject).to eq(true)
end
it 'caches the output' do
expect(repository.raw_repository).to receive(:branch_count).
once.
and_return(3)
repository.has_visible_content?
repository.has_visible_content?
end
end
end
describe '#empty?' do
let(:empty_repository) { create(:project_empty_repo).repository }
it 'returns true for an empty repository' do
expect(empty_repository.empty?).to eq(true)
end
it 'returns false for a non-empty repository' do
expect(repository.empty?).to eq(false)
end
it 'caches the output' do
expect(repository.raw_repository).to receive(:empty?).
once.
and_return(false)
repository.empty?
repository.empty?
end
end
describe '#root_ref' do
it 'returns a branch name' do
expect(repository.root_ref).to be_an_instance_of(String)
end end
it { is_expected.to eq(true) } it 'caches the output' do
expect(repository.raw_repository).to receive(:root_ref).
once.
and_return('master')
repository.root_ref
repository.root_ref
end
end
describe '#expire_cache' do
it 'expires all caches' do
expect(repository).to receive(:expire_branch_cache)
repository.expire_cache
end
end
describe '#expire_root_ref_cache' do
it 'expires the root reference cache' do
repository.root_ref
expect(repository.raw_repository).to receive(:root_ref).
once.
and_return('foo')
repository.expire_root_ref_cache
expect(repository.root_ref).to eq('foo')
end
end
describe '#expire_has_visible_content_cache' do
it 'expires the visible content cache' do
repository.has_visible_content?
expect(repository.raw_repository).to receive(:branch_count).
once.
and_return(0)
repository.expire_has_visible_content_cache
expect(repository.has_visible_content?).to eq(false)
end end
end end
end end
...@@ -21,6 +21,18 @@ describe GitPushService, services: true do ...@@ -21,6 +21,18 @@ describe GitPushService, services: true do
end end
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
it 'flushes general cached data' do
expect(project.repository).to receive(:expire_cache)
subject
end
it 'flushes the visible content cache' do
expect(project.repository).to receive(:expire_has_visible_content_cache)
subject
end
end end
context 'existing branch' do context 'existing branch' do
...@@ -29,6 +41,12 @@ describe GitPushService, services: true do ...@@ -29,6 +41,12 @@ describe GitPushService, services: true do
end end
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
it 'flushes general cached data' do
expect(project.repository).to receive(:expire_cache)
subject
end
end end
context 'rm branch' do context 'rm branch' do
...@@ -37,6 +55,18 @@ describe GitPushService, services: true do ...@@ -37,6 +55,18 @@ describe GitPushService, services: true do
end end
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
it 'flushes the visible content cache' do
expect(project.repository).to receive(:expire_has_visible_content_cache)
subject
end
it 'flushes general cached data' do
expect(project.repository).to receive(:expire_cache)
subject
end
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