Speedup DiffNote#active? on discussions, preloading noteables and avoid touching git repository to return diff_refs when possible

- Preloading noteable we share the same noteable instance when more than one 
discussion refers to the same noteable.
- Any other call to that object that is cached in that object will be for any 
discussion.
- In those cases where merge_request_diff has all the sha stored to build a diff_refs get that 
diff_refs using directly those sha instead accessing to the git repository to first get the 
commits and later the sha.

6 files changed, 40 insertions, 3 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 03166294ddd..116e7904a4e 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -378,6 +378,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       fresh.
       discussions
 
+    preload_noteable_for_regular_notes(@discussions.flat_map(&:notes))
+
     # This is not executed lazily
     @notes = Banzai::NoteRenderer.render(
       @discussions.flat_map(&:notes),
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 0c47abe0fba..26bde2230a9 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -92,6 +92,10 @@ module NotesHelper
     project.team.max_member_access_for_user_ids(user_ids)
   end
 
+  def preload_noteable_for_regular_notes(notes)
+    ActiveRecord::Associations::Preloader.new.preload(notes.select { |note| !note.for_commit? }, :noteable)
+  end
+
   def note_max_access_for_user(note)
     note.project.team.human_max_access(note.author_id)
   end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 9671955db36..c816deb4e0c 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -67,7 +67,7 @@ class DiffNote < Note
     return false unless supported?
     return true if for_commit?
 
-    diff_refs ||= self.noteable.diff_refs
+    diff_refs ||= noteable_diff_refs
 
     self.position.diff_refs == diff_refs
   end
@@ -78,6 +78,14 @@ class DiffNote < Note
     !self.for_merge_request? || self.noteable.support_new_diff_notes?
   end
 
+  def noteable_diff_refs
+    if noteable.respond_to?(:diff_sha_refs)
+      noteable.diff_sha_refs
+    else
+      noteable.diff_refs
+    end
+  end
+
   def set_original_position
     self.original_position = self.position.dup
   end
@@ -96,7 +104,7 @@ class DiffNote < Note
       self.project,
       nil,
       old_diff_refs: self.position.diff_refs,
-      new_diff_refs: self.noteable.diff_refs,
+      new_diff_refs: noteable_diff_refs,
       paths: self.position.paths
     ).execute(self)
   end
diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 74facfd1c9c..e2218a5f02b 100644
--- a/app/models/discussion.rb
+++ b/app/models/discussion.rb
@@ -49,6 +49,12 @@ class Discussion
     self.noteable == target && !diff_discussion?
   end
 
+  def active?
+    return @active if defined?(@active)
+
+    @active = first_note.active?
+  end
+
   def expanded?
     !diff_discussion? || active?
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index f1b9c1d6feb..a99c4ba52a4 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -255,6 +255,19 @@ class MergeRequest < ActiveRecord::Base
     )
   end
 
+  # Return diff_refs instance trying to not touch the git repository
+  def diff_sha_refs
+    if merge_request_diff && merge_request_diff.diff_refs_by_sha?
+      return Gitlab::Diff::DiffRefs.new(
+        base_sha:  merge_request_diff.base_commit_sha,
+        start_sha: merge_request_diff.start_commit_sha,
+        head_sha:  merge_request_diff.head_commit_sha
+      )
+    else
+      diff_refs
+    end
+  end
+
   def validate_branches
     if target_project == source_project && target_branch == source_branch
       errors.add :branch_conflict, "You can not use same project/branch for source and target"
@@ -659,7 +672,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def support_new_diff_notes?
-    diff_refs && diff_refs.complete?
+    diff_sha_refs && diff_sha_refs.complete?
   end
 
   def update_diff_notes_positions(old_diff_refs:, new_diff_refs:)
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 3f520c8f3ff..119266f2d2c 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -82,6 +82,10 @@ class MergeRequestDiff < ActiveRecord::Base
     project.commit(self.head_commit_sha)
   end
 
+  def diff_refs_by_sha?
+    base_commit_sha? && head_commit_sha? && start_commit_sha?
+  end
+
   def compare
     @compare ||=
       begin