summaryrefslogtreecommitdiff
path: root/app/models
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2018-03-20 15:51:54 +0000
committerSean McGivern <sean@gitlab.com>2018-03-21 17:24:46 +0000
commit39c9928cfbf5b72042755c58bd5c484551c5f1ad (patch)
tree02d8de7ece39fa76039e4a640ada9fef147bc94d /app/models
parent398f13f39a0ce51c9ca3f223e87df88db822a396 (diff)
downloadgitlab-ce-39c9928cfbf5b72042755c58bd5c484551c5f1ad.tar.gz
Fix N+1 in `MergeRequest#merge_request_diff_for`reduce-query-count-for-mergerequestscontroller-show
Previously, this would issue a query for each unique `diff_refs_or_sha` passed. This was because we didn't want to load other MR diffs into memory, as they had some very large columns. Now they are actually very small, and it's more efficient to just load them all at once and do the finding in Ruby.
Diffstat (limited to 'app/models')
-rw-r--r--app/models/merge_request.rb27
1 files changed, 17 insertions, 10 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 7e6d89ec9c7..91d8be5559b 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -536,18 +536,25 @@ class MergeRequest < ActiveRecord::Base
merge_request_diff(true)
end
+ def viewable_diffs
+ @viewable_diffs ||= merge_request_diffs.viewable.to_a
+ end
+
def merge_request_diff_for(diff_refs_or_sha)
- @merge_request_diffs_by_diff_refs_or_sha ||= Hash.new do |h, diff_refs_or_sha|
- diffs = merge_request_diffs.viewable
- h[diff_refs_or_sha] =
- if diff_refs_or_sha.is_a?(Gitlab::Diff::DiffRefs)
- diffs.find_by_diff_refs(diff_refs_or_sha)
- else
- diffs.find_by(head_commit_sha: diff_refs_or_sha)
- end
- end
+ matcher =
+ if diff_refs_or_sha.is_a?(Gitlab::Diff::DiffRefs)
+ {
+ 'start_commit_sha' => diff_refs_or_sha.start_sha,
+ 'head_commit_sha' => diff_refs_or_sha.head_sha,
+ 'base_commit_sha' => diff_refs_or_sha.base_sha
+ }
+ else
+ { 'head_commit_sha' => diff_refs_or_sha }
+ end
- @merge_request_diffs_by_diff_refs_or_sha[diff_refs_or_sha]
+ viewable_diffs.find do |diff|
+ diff.attributes.slice(*matcher.keys) == matcher
+ end
end
def version_params_for(diff_refs)