Revert "Merge branch 'zj-memoization-mr-commits' into 'master'"

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)

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

changelogs/unreleased/zj-memoization-mr-commits.yml

----
-title: Cache commits for MergeRequest diffs
-merge_request:
-author:
-type: performance

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

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

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

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