Skip to content

Commit

Permalink
Speed up commit loads by disabling BatchLoader replace_methods
Browse files Browse the repository at this point in the history
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
(exAspArk/batch-loader#45) appears to cut load
times by about 50% for MergeRequestsController#show.

Relates to https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/6941
  • Loading branch information
stanhu committed Jun 13, 2019
1 parent 89fa253 commit 9482ea3
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 0 deletions.
5 changes: 5 additions & 0 deletions changelogs/unreleased/sh-speed-up-commit-loading.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
title: Speed up commit loads by disabling BatchLoader replace_methods
merge_request:
author:
type: performance
8 changes: 8 additions & 0 deletions spec/models/commit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9482ea3

Please sign in to comment.