From 9482ea320c3c83f39053c7cca386d5171ad3bd1c Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 13 Jun 2019 13:50:27 -0700 Subject: [PATCH] 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 --- changelogs/unreleased/sh-speed-up-commit-loading.yml | 5 +++++ spec/models/commit_spec.rb | 8 ++++++++ 2 files changed, 13 insertions(+) create mode 100644 changelogs/unreleased/sh-speed-up-commit-loading.yml 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 0000000000000..600a0b03f6296 --- /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: +author: +type: performance diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 14f4b4d692f63..e76186fb280c1 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -44,6 +44,14 @@ 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