diff options
author | Stan Hu <stanhu@gmail.com> | 2019-06-13 13:50:27 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-06-13 14:21:23 -0700 |
commit | 18020f00f880fd849800624e08e87d0a5aa5111f (patch) | |
tree | e447832453524d6c0e086742c69535447357dd09 | |
parent | 89fa253864442096b7cf5bb5803f6b103aa0d92e (diff) | |
download | gitlab-ce-18020f00f880fd849800624e08e87d0a5aa5111f.tar.gz |
Speed up commit loads by disabling BatchLoader replace_methods
We've seen a significant performance penalty when using
`BatchLoader#__replace_with!`. This defines methods on the batch loader
that proxy to the 'real' object using send. The alternative is
`method_missing`, which is slower. However, we've noticed that
`method_missing` can be faster if:
1. The objects being loaded have a large interface.
2. We don't call too many methods on the loaded object.
In production, we've seen the rendering times of the merge request
widget increase as a result of loading commit data. BatchLoader attempts
to call replace_methods on the lazy object, but this has a significant
performance penalty. Disabling this mode
(https://github.com/exAspArk/batch-loader/pull/45) appears to cut load
times by about 50% for MergeRequestsController#show.
Relates to https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/6941
-rw-r--r-- | changelogs/unreleased/sh-speed-up-commit-loading.yml | 5 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 8 |
2 files changed, 13 insertions, 0 deletions
diff --git a/changelogs/unreleased/sh-speed-up-commit-loading.yml b/changelogs/unreleased/sh-speed-up-commit-loading.yml new file mode 100644 index 00000000000..db408708385 --- /dev/null +++ b/changelogs/unreleased/sh-speed-up-commit-loading.yml @@ -0,0 +1,5 @@ +--- +title: Speed up commit loads by disabling BatchLoader replace_methods +merge_request: 29633 +author: +type: performance diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 14f4b4d692f..e76186fb280 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -44,6 +44,14 @@ describe Commit do expect(commit.id).to eq(oids[i]) end end + + it 'does not attempt to replace methods via BatchLoader' do + subject.each do |commit| + expect(commit).to receive(:method_missing).and_call_original + + commit.id + end + end end context 'when not found' do |