diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2017-12-13 14:04:33 -0200 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2017-12-13 14:04:33 -0200 |
commit | c80c204b8b72fabd4fa8c4199bbba071468b9594 (patch) | |
tree | b8b6e19f544d4c61b83331a2f0f6a5aafa0a7168 | |
parent | f48e815c895792897a835a7cbbbc2f185a1cdebd (diff) | |
download | gitlab-ce-c80c204b8b72fabd4fa8c4199bbba071468b9594.tar.gz |
Revert "Merge branch 'zj-memoization-mr-commits' into 'master'"
This reverts commit 54f008126d83af1869572565336e01d3953b2b91.
-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, 13 insertions, 57 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c342820eb2d..d7faf22fc46 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -7,7 +7,6 @@ class MergeRequest < ActiveRecord::Base include TimeTrackable include ManualInverseAssociation include EachBatch - include Gitlab::Utils::StrongMemoize ignore_column :locked_at, :ref_fetched @@ -53,7 +52,6 @@ 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 @@ -388,17 +386,13 @@ class MergeRequest < ActiveRecord::Base end def source_branch_head - strong_memoize(:source_branch_head) do - if source_project && source_branch_ref - source_project.repository.commit(source_branch_ref) - end - end + return unless source_project + + source_project.repository.commit(source_branch_ref) if source_branch_ref end def target_branch_head - strong_memoize(:target_branch_head) do - target_project.repository.commit(target_branch_ref) - end + target_project.repository.commit(target_branch_ref) end def branch_merge_base_commit @@ -530,13 +524,6 @@ 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 e35de9b97ee..c37aa0a594b 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_by(oid: base_commit_sha) + project.commit(base_commit_sha) end def start_commit return unless start_commit_sha - project.commit_by(oid: start_commit_sha) + project.commit(start_commit_sha) end def head_commit return unless head_commit_sha - project.commit_by(oid: head_commit_sha) + project.commit(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 deleted file mode 100644 index 59dfc6d6049..00000000000 --- a/changelogs/unreleased/zj-memoization-mr-commits.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -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 b9099ce256a..fb28e80ff73 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -19,8 +19,6 @@ 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 fe091f4611b..a2ac9285b56 100644 --- a/lib/gitlab/utils/strong_memoize.rb +++ b/lib/gitlab/utils/strong_memoize.rb @@ -11,8 +11,6 @@ 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) @@ -20,22 +18,14 @@ module Gitlab # end # def strong_memoize(name) - if instance_variable_defined?(ivar(name)) - instance_variable_get(ivar(name)) + ivar_name = "@#{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 473f8100771..4a104ab6d97 100644 --- a/spec/lib/gitlab/utils/strong_memoize_spec.rb +++ b/spec/lib/gitlab/utils/strong_memoize_spec.rb @@ -49,16 +49,4 @@ 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 6caed4cc193..81eb62f1801 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -124,7 +124,6 @@ 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 +733,7 @@ describe MergeRequest do before do project.repository.raw_repository.delete_branch(subject.target_branch) - subject.clear_memoized_shas + subject.reload end it 'does not crash' do @@ -1469,7 +1468,6 @@ 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 |