diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-12-12 18:29:41 +0000 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2017-12-19 14:05:09 -0200 |
commit | 1b65f905c7ee39624a7c0ed7c80428b85cb370e4 (patch) | |
tree | d08be7d6dbd35b359522341dff3144bdd4ba2024 | |
parent | fff75870c70ee03890cdd6fa5ce6da51d5f08e57 (diff) | |
download | gitlab-ce-1b65f905c7ee39624a7c0ed7c80428b85cb370e4.tar.gz |
Merge branch 'zj-memoization-mr-commits' into 'master'
Use memoization for commits on diffs
See merge request gitlab-org/gitlab-ce!15857
-rw-r--r-- | app/models/merge_request.rb | 21 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/zj-memoization-mr-commits.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/conflict/file_collection.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/utils/strong_memoize.rb | 20 | ||||
-rw-r--r-- | spec/lib/gitlab/utils/strong_memoize_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 4 |
7 files changed, 57 insertions, 13 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index fc9018e65a1..993006760f7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -8,6 +8,7 @@ class MergeRequest < ActiveRecord::Base include ManualInverseAssociation include EachBatch include ThrottledTouch + include Gitlab::Utils::StrongMemoize ignore_column :locked_at, :ref_fetched @@ -53,6 +54,7 @@ class MergeRequest < ActiveRecord::Base after_create :ensure_merge_request_diff, unless: :importing? after_update :reload_diff_if_branch_changed + after_update :clear_memoized_shas # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests @@ -395,13 +397,17 @@ class MergeRequest < ActiveRecord::Base end def source_branch_head - return unless source_project - - source_project.repository.commit(source_branch_ref) if source_branch_ref + strong_memoize(:source_branch_head) do + if source_project && source_branch_ref + source_project.repository.commit(source_branch_ref) + end + end end def target_branch_head - target_project.repository.commit(target_branch_ref) + strong_memoize(:target_branch_head) do + target_project.repository.commit(target_branch_ref) + end end def branch_merge_base_commit @@ -533,6 +539,13 @@ class MergeRequest < ActiveRecord::Base end end + def clear_memoized_shas + @target_branch_sha = @source_branch_sha = nil + + clear_memoization(:source_branch_head) + clear_memoization(:target_branch_head) + end + def reload_diff_if_branch_changed if (source_branch_changed? || target_branch_changed?) && (source_branch_head && target_branch_head) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index c37aa0a594b..e35de9b97ee 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -104,19 +104,19 @@ class MergeRequestDiff < ActiveRecord::Base def base_commit return unless base_commit_sha - project.commit(base_commit_sha) + project.commit_by(oid: base_commit_sha) end def start_commit return unless start_commit_sha - project.commit(start_commit_sha) + project.commit_by(oid: start_commit_sha) end def head_commit return unless head_commit_sha - project.commit(head_commit_sha) + project.commit_by(oid: head_commit_sha) end def commit_shas diff --git a/changelogs/unreleased/zj-memoization-mr-commits.yml b/changelogs/unreleased/zj-memoization-mr-commits.yml new file mode 100644 index 00000000000..59dfc6d6049 --- /dev/null +++ b/changelogs/unreleased/zj-memoization-mr-commits.yml @@ -0,0 +1,5 @@ +--- +title: Cache commits for MergeRequest diffs +merge_request: +author: +type: performance diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index fb28e80ff73..b9099ce256a 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -19,6 +19,8 @@ module Gitlab commit_message: commit_message || default_commit_message } resolver.resolve_conflicts(user, files, args) + ensure + @merge_request.clear_memoized_shas end def files diff --git a/lib/gitlab/utils/strong_memoize.rb b/lib/gitlab/utils/strong_memoize.rb index a2ac9285b56..fe091f4611b 100644 --- a/lib/gitlab/utils/strong_memoize.rb +++ b/lib/gitlab/utils/strong_memoize.rb @@ -11,6 +11,8 @@ module Gitlab # # We could write it like: # + # include Gitlab::Utils::StrongMemoize + # # def trigger_from_token # strong_memoize(:trigger) do # Ci::Trigger.find_by_token(params[:token].to_s) @@ -18,14 +20,22 @@ module Gitlab # end # def strong_memoize(name) - ivar_name = "@#{name}" - - if instance_variable_defined?(ivar_name) - instance_variable_get(ivar_name) + if instance_variable_defined?(ivar(name)) + instance_variable_get(ivar(name)) else - instance_variable_set(ivar_name, yield) + instance_variable_set(ivar(name), yield) end end + + def clear_memoization(name) + remove_instance_variable(ivar(name)) if instance_variable_defined?(ivar(name)) + end + + private + + def ivar(name) + "@#{name}" + end end end end diff --git a/spec/lib/gitlab/utils/strong_memoize_spec.rb b/spec/lib/gitlab/utils/strong_memoize_spec.rb index 4a104ab6d97..473f8100771 100644 --- a/spec/lib/gitlab/utils/strong_memoize_spec.rb +++ b/spec/lib/gitlab/utils/strong_memoize_spec.rb @@ -49,4 +49,16 @@ describe Gitlab::Utils::StrongMemoize do end end end + + describe '#clear_memoization' do + let(:value) { 'mepmep' } + + it 'removes the instance variable' do + object.method_name + + object.clear_memoization(:method_name) + + expect(object.instance_variable_defined?(:@method_name)).to be(false) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index da5a5f6d0b7..bb63abd167b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -124,6 +124,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do project.repository.rm_branch(subject.author, subject.target_branch) + subject.clear_memoized_shas end it 'returns nil' do @@ -734,7 +735,7 @@ describe MergeRequest do before do project.repository.raw_repository.delete_branch(subject.target_branch) - subject.reload + subject.clear_memoized_shas end it 'does not crash' do @@ -1479,6 +1480,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do subject.project.repository.rm_branch(subject.author, subject.target_branch) + subject.clear_memoized_shas end it 'returns nil' do |