diff options
author | Stan Hu <stanhu@gmail.com> | 2018-10-31 12:51:11 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-11-01 07:19:54 -0700 |
commit | 0844ba04b518b4cd0a326a2ee0e8431eeb26950d (patch) | |
tree | e38a47efb50d4e701559ccb628cb009c3344f35d | |
parent | 98a504ecbb45907517ae465018906af7ef4573de (diff) | |
download | gitlab-ce-0844ba04b518b4cd0a326a2ee0e8431eeb26950d.tar.gz |
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
3 files changed, 20 insertions, 1 deletions
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 |