Merge branch 'sh-memoize-repository-empty' into 'master'

Memoize Git::Repository#has_visible_content?

See merge request gitlab-org/gitlab-ce!18383

5 files changed, 40 insertions, 6 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index fd1afafe4df..5bdaa7f0720 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -331,6 +331,7 @@ class Repository
     return unless empty?
 
     expire_method_caches(%i(has_visible_content?))
+    raw_repository.expire_has_local_branches_cache
   end
 
   def lookup_cache
diff --git a/changelogs/unreleased/sh-memoize-repository-empty.yml b/changelogs/unreleased/sh-memoize-repository-empty.yml
new file mode 100644
index 00000000000..64db3ca0371
--- /dev/null
+++ b/changelogs/unreleased/sh-memoize-repository-empty.yml
@@ -0,0 +1,5 @@
+---
+title: Memoize Git::Repository#has_visible_content?
+merge_request:
+author:
+type: performance
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 11a421cb430..294475be9c6 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -9,6 +9,7 @@ module Gitlab
       include Gitlab::Git::RepositoryMirroring
       include Gitlab::Git::Popen
       include Gitlab::EncodingHelper
+      include Gitlab::Utils::StrongMemoize
 
       ALLOWED_OBJECT_DIRECTORIES_VARIABLES = %w[
         GIT_OBJECT_DIRECTORY
@@ -231,13 +232,13 @@ module Gitlab
         end
       end
 
+      def expire_has_local_branches_cache
+        clear_memoization(:has_local_branches)
+      end
+
       def has_local_branches?
-        gitaly_migrate(:has_local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
-          if is_enabled
-            gitaly_repository_client.has_local_branches?
-          else
-            has_local_branches_rugged?
-          end
+        strong_memoize(:has_local_branches) do
+          uncached_has_local_branches?
         end
       end
 
@@ -1559,6 +1560,16 @@ module Gitlab
 
       private
 
+      def uncached_has_local_branches?
+        gitaly_migrate(:has_local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
+          if is_enabled
+            gitaly_repository_client.has_local_branches?
+          else
+            has_local_branches_rugged?
+          end
+        end
+      end
+
       def local_write_ref(ref_path, ref, old_ref: nil, shell: true)
         if shell
           shell_write_ref(ref_path, ref, old_ref)
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 1e00e8d2739..5acf40ea5ce 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -470,9 +470,20 @@ describe Gitlab::Git::Repository, seed_helper: true do
           FileUtils.rm_rf(heads_dir)
           FileUtils.mkdir_p(heads_dir)
 
+          repository.expire_has_local_branches_cache
           expect(repository.has_local_branches?).to eq(false)
         end
       end
+
+      context 'memoizes the value' do
+        it 'returns true' do
+          expect(repository).to receive(:uncached_has_local_branches?).once.and_call_original
+
+          2.times do
+            expect(repository.has_local_branches?).to eq(true)
+          end
+        end
+      end
     end
 
     context 'with gitaly' do
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 60ab52565cb..e45fe7db1e7 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1437,6 +1437,12 @@ describe Repository do
 
       repository.expire_emptiness_caches
     end
+
+    it 'expires the memoized repository cache' do
+      allow(repository.raw_repository).to receive(:expire_has_local_branches_cache).and_call_original
+
+      repository.expire_emptiness_caches
+    end
   end
 
   describe 'skip_merges option' do