diff options
| author | Stan Hu <stanhu@gmail.com> | 2019-06-20 21:49:14 -0700 |
|---|---|---|
| committer | Stan Hu <stanhu@gmail.com> | 2019-08-27 16:56:57 -0700 |
| commit | 76f4615aca2d9744590191aa5cdf0daf10ef62c8 (patch) | |
| tree | 44a1f8a151143e66f4157d573edb0f1d16b9fdde | |
| parent | 0ef49746ad4ab732c9645fb709676c32cb3ea389 (diff) | |
| download | gitlab-ce-sh-disable-replace-methods-batch-loader.tar.gz | |
Speed up MR widget by disabling BatchLoader replace_methodssh-disable-replace-methods-batch-loader
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 proudction, we've observed a small percentage of time used
in Module#define_methods as a result of the BatchLoader. Disabling this
for ActiveRecord models is generally a good thing to do since there are
so many methods.
| -rw-r--r-- | app/models/ci/pipeline.rb | 2 | ||||
| -rw-r--r-- | app/models/ci/stage.rb | 2 | ||||
| -rw-r--r-- | app/models/commit.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-disable-replace-methods-batch-loader.yml | 5 |
4 files changed, 8 insertions, 3 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 0a943a33bbb..3b3482499c0 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -530,7 +530,7 @@ module Ci end def number_of_warnings - BatchLoader.for(id).batch(default_value: 0) do |pipeline_ids, loader| + BatchLoader.for(id).batch(default_value: 0, replace_methods: false) do |pipeline_ids, loader| ::Ci::Build.where(commit_id: pipeline_ids) .latest .failed_but_allowed diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index d90339d90dc..b460fd58c75 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -105,7 +105,7 @@ module Ci end def number_of_warnings - BatchLoader.for(id).batch(default_value: 0) do |stage_ids, loader| + BatchLoader.for(id).batch(default_value: 0, replace_methods: false) do |stage_ids, loader| ::Ci::Build.where(stage_id: stage_ids) .latest .failed_but_allowed diff --git a/app/models/commit.rb b/app/models/commit.rb index 0889ce7e287..bf9bdaef19c 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -229,7 +229,7 @@ class Commit end def lazy_author - BatchLoader.for(author_email.downcase).batch do |emails, loader| + BatchLoader.for(author_email.downcase).batch(replace_methods: false) do |emails, loader| users = User.by_any_email(emails).includes(:emails) emails.each do |email| diff --git a/changelogs/unreleased/sh-disable-replace-methods-batch-loader.yml b/changelogs/unreleased/sh-disable-replace-methods-batch-loader.yml new file mode 100644 index 00000000000..d44913fbd81 --- /dev/null +++ b/changelogs/unreleased/sh-disable-replace-methods-batch-loader.yml @@ -0,0 +1,5 @@ +--- +title: Speed up MR widget by disabling BatchLoader replace_methods +merge_request: 29920 +author: +type: performance |