Merge branch 'lru-object-caching-group-project-object-builder' into 'master'

LRU object caching for GroupProjectObjectBuilder

See merge request gitlab-org/gitlab!21823

Gemfile

@@ -484,3 +484,6 @@ gem 'countries', '~> 3.0'
 gem 'retriable', '~> 3.1.2'
 
 gem 'liquid', '~> 4.0'
+
+# LRU cache
+gem 'lru_redux'

Gemfile.lock

@@ -598,6 +598,7 @@ GEM
     loofah (2.4.0)
       crass (~> 1.0.2)
       nokogiri (>= 1.5.9)
+    lru_redux (1.1.0)
     lumberjack (1.0.13)
     mail (2.7.1)
       mini_mime (>= 0.1.1)
@@ -1263,6 +1264,7 @@ DEPENDENCIES
   liquid (~> 4.0)
   lograge (~> 0.5)
   loofah (~> 2.2)
+  lru_redux
   mail_room (~> 0.10.0)
   marginalia (~> 1.8.0)
   memory_profiler (~> 0.9)

changelogs/unreleased/lru-object-caching-group-project-object-builder.yml

+---
+title: LRU object caching for GroupProjectObjectBuilder
+merge_request: 21823
+author:
+type: performance

lib/gitlab/import_export/group_project_object_builder.rb

@@ -12,6 +12,13 @@ module Gitlab
     #
     # It also adds some logic around Group Labels/Milestones for edge cases.
     class GroupProjectObjectBuilder
+      # Cache keeps 1000 entries at most, 1000 is chosen based on:
+      #    - one cache entry uses around 0.5K memory, 1000 items uses around 500K.
+      #      (leave some buffer it should be less than 1M). It is afforable cost for project import.
+      #    - for projects in Gitlab.com, it seems 1000 entries for labels/milestones is enough.
+      #      For example, gitlab has ~970 labels and 26 milestones.
+      LRU_CACHE_SIZE = 1000
+
       def self.build(*args)
         Project.transaction do
           new(*args).find
@@ -23,17 +30,34 @@ module Gitlab
         @attributes = attributes
         @group = @attributes['group']
         @project = @attributes['project']
+
+        if Gitlab::SafeRequestStore.active?
+          @lru_cache = cache_from_request_store
+          @cache_key = [klass, attributes]
+        end
       end
 
       def find
         return if epic? && group.nil?
 
-        find_object || klass.create(project_attributes)
+        find_with_cache do
+          find_object || klass.create(project_attributes)
+        end
       end
 
       private
 
-      attr_reader :klass, :attributes, :group, :project
+      attr_reader :klass, :attributes, :group, :project, :lru_cache, :cache_key
+
+      def find_with_cache
+        return yield unless lru_cache && cache_key
+
+        lru_cache[cache_key] ||= yield
+      end
+
+      def cache_from_request_store
+        Gitlab::SafeRequestStore[:lru_cache] ||= LruRedux::Cache.new(LRU_CACHE_SIZE)
+      end
 
       def find_object
         klass.where(where_clause).first

spec/lib/gitlab/import_export/group_project_object_builder_spec.rb

@@ -12,6 +12,59 @@ describe Gitlab::ImportExport::GroupProjectObjectBuilder do
            group: create(:group))
   end
 
+  let(:lru_cache) { subject.send(:lru_cache) }
+  let(:cache_key) { subject.send(:cache_key) }
+
+  context 'request store is not active' do
+    subject do
+      described_class.new(Label,
+                          'title' => 'group label',
+                          'project' => project,
+                          'group' => project.group)
+    end
+
+    it 'ignore cache initialize' do
+      expect(lru_cache).to be_nil
+      expect(cache_key).to be_nil
+    end
+  end
+
+  context 'request store is active', :request_store do
+    subject do
+      described_class.new(Label,
+                          'title' => 'group label',
+                          'project' => project,
+                          'group' => project.group)
+    end
+
+    it 'initialize cache in memory' do
+      expect(lru_cache).not_to be_nil
+      expect(cache_key).not_to be_nil
+    end
+
+    it 'cache object when first time find the object' do
+      group_label = create(:group_label, name: 'group label', group: project.group)
+
+      expect(subject).to receive(:find_object).and_call_original
+      expect { subject.find }
+        .to change { lru_cache[cache_key] }
+        .from(nil).to(group_label)
+
+      expect(subject.find).to eq(group_label)
+    end
+
+    it 'read from cache when object has been cached' do
+      group_label = create(:group_label, name: 'group label', group: project.group)
+
+      subject.find
+
+      expect(subject).not_to receive(:find_object)
+      expect { subject.find }.not_to change { lru_cache[cache_key] }
+
+      expect(subject.find).to eq(group_label)
+    end
+  end
+
   context 'labels' do
     it 'finds the existing group label' do
       group_label = create(:group_label, name: 'group label', group: project.group)