From 0844ba04b518b4cd0a326a2ee0e8431eeb26950d Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Wed, 31 Oct 2018 12:51:11 -0700
Subject: [PATCH] Significantly cut memory usage and SQL queries when reloading
 diffs

By preloading certain models with the diff, we can eliminate many N+1
queries. For a push to the staging GitLab.com www-gitlab-com repository,
this eliminates over 3000 SQL queries and appears to bring down the RSS
usage by several gigabytes.

Relates to https://gitlab.com/gitlab-org/gitlab-ce/issues/49703
---
 app/services/merge_requests/reload_diffs_service.rb   |  5 ++++-
 .../unreleased/sh-optimize-reload-diffs-service.yml   |  5 +++++
 .../merge_requests/reload_diffs_service_spec.rb       | 11 +++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/sh-optimize-reload-diffs-service.yml

diff --git a/app/services/merge_requests/reload_diffs_service.rb b/app/services/merge_requests/reload_diffs_service.rb
index b4d48fe92ad..44997cf012b 100644
--- a/app/services/merge_requests/reload_diffs_service.rb
+++ b/app/services/merge_requests/reload_diffs_service.rb
@@ -36,7 +36,10 @@ module MergeRequests
       # Remove cache for all diffs on this MR. Do not use the association on the
       # model, as that will interfere with other actions happening when
       # reloading the diff.
-      MergeRequestDiff.where(merge_request: merge_request).each do |merge_request_diff|
+      MergeRequestDiff
+        .where(merge_request: merge_request)
+        .preload(:merge_request_diff_files, merge_request: :target_project)
+        .find_each do |merge_request_diff|
         next if merge_request_diff == new_diff
 
         cacheable_collection(merge_request_diff).clear_cache
diff --git a/changelogs/unreleased/sh-optimize-reload-diffs-service.yml b/changelogs/unreleased/sh-optimize-reload-diffs-service.yml
new file mode 100644
index 00000000000..422102560ed
--- /dev/null
+++ b/changelogs/unreleased/sh-optimize-reload-diffs-service.yml
@@ -0,0 +1,5 @@
+---
+title: Significantly cut memory usage and SQL queries when reloading diffs
+merge_request: 22725
+author:
+type: performance
diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb
index 21f369a3818..546c9f277c5 100644
--- a/spec/services/merge_requests/reload_diffs_service_spec.rb
+++ b/spec/services/merge_requests/reload_diffs_service_spec.rb
@@ -60,6 +60,17 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin
 
         subject.execute
       end
+
+      it 'avoids N+1 queries', :request_store do
+        current_user
+        merge_request
+
+        control_count = ActiveRecord::QueryRecorder.new do
+          subject.execute
+        end.count
+
+        expect { subject.execute }.not_to exceed_query_limit(control_count)
+      end
     end
   end
 end