summaryrefslogtreecommitdiff
path: root/app/models
diff options
context:
space:
mode:
authorZeger-Jan van de Weg <git@zjvandeweg.nl>2017-12-11 16:38:16 +0100
committerZeger-Jan van de Weg <git@zjvandeweg.nl>2017-12-12 16:28:26 +0100
commit3ab026b7d69ec97796fe4bb409933d7a716eef19 (patch)
tree6711c10cf577250647330b1d2725e13ce2177a66 /app/models
parent806a68a81f1baeed07c146b1b5d9eb77796c46ba (diff)
downloadgitlab-ce-3ab026b7d69ec97796fe4bb409933d7a716eef19.tar.gz
Use memoization for commits on diffs
The Gitaly CommitService is being hammered by n + 1 calls, mostly when finding commits. This leads to this gRPC being turned of on production: https://gitlab.com/gitlab-org/gitaly/issues/514#note_48991378 Hunting down where it came from, most of them were due to MergeRequest#show. To prove this, I set a script to request the MergeRequest#show page 50 times. The GDK was being scraped by Prometheus, where we have metrics on controller#action and their Gitaly calls performed. On both occations I've restarted the full GDK so all caches had to be rebuild. Current master, 806a68a81f1baee, needed 435 requests After this commit, 154 requests
Diffstat (limited to 'app/models')
-rw-r--r--app/models/merge_request.rb21
-rw-r--r--app/models/merge_request_diff.rb6
2 files changed, 20 insertions, 7 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 422f138c4ea..26a3388602a 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -8,6 +8,7 @@ class MergeRequest < ActiveRecord::Base
include ManualInverseAssociation
include EachBatch
include ThrottledTouch
+ include Gitlab::Utils::StrongMemoize
ignore_column :locked_at,
:ref_fetched
@@ -53,6 +54,7 @@ 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
@@ -387,13 +389,17 @@ class MergeRequest < ActiveRecord::Base
end
def source_branch_head
- return unless source_project
-
- source_project.repository.commit(source_branch_ref) if source_branch_ref
+ strong_memoize(:source_branch_head) do
+ if source_project && source_branch_ref
+ source_project.repository.commit(source_branch_ref)
+ end
+ end
end
def target_branch_head
- target_project.repository.commit(target_branch_ref)
+ strong_memoize(:target_branch_head) do
+ target_project.repository.commit(target_branch_ref)
+ end
end
def branch_merge_base_commit
@@ -525,6 +531,13 @@ 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)
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index c37aa0a594b..e35de9b97ee 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -104,19 +104,19 @@ class MergeRequestDiff < ActiveRecord::Base
def base_commit
return unless base_commit_sha
- project.commit(base_commit_sha)
+ project.commit_by(oid: base_commit_sha)
end
def start_commit
return unless start_commit_sha
- project.commit(start_commit_sha)
+ project.commit_by(oid: start_commit_sha)
end
def head_commit
return unless head_commit_sha
- project.commit(head_commit_sha)
+ project.commit_by(oid: head_commit_sha)
end
def commit_shas