diff options
author | Stan Hu <stanhu@gmail.com> | 2019-04-27 22:25:45 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-04-29 05:33:50 -0700 |
commit | b02458ef52ac3e61d3c8b924e092e956375c15f7 (patch) | |
tree | 38a90e680302da6557a51e89daf50c157cfec77c | |
parent | d232137aa7d857396060f9ab02d4e99cf8081285 (diff) | |
download | gitlab-ce-b02458ef52ac3e61d3c8b924e092e956375c15f7.tar.gz |
Fix slow performance with compiling HAML templates
In Rails 5, including `ActionView::Context` can have a significant and
hidden performance penalty because this module also includes
`ActionView::CompiledTemplates`. This means that any module that
includes ActionView::Context becomes a descendant of
`CompiledTemplates`.
When a partial is rendered for the first time, it runs
`ActionView::CompiledTemplates#module_eval`, which will evaluate a
string that defines a new method for that partial. For example, the
source of partial might be this string:
```
def _app_views_project_show_html_haml___12345(local_assigns, output)
"hello world"
end
```
When this string is evaluated, the Ruby interpreter will define the
method and clear the global method cache for all descendants of
`ActionView::CompiledTemplates`. Previous to this change, we
inadvertently made a number of modules fall into this category:
* GroupChildEntity
* NoteUserEntity
* Notify
* MergeRequestUserEntity
* AnalyticsCommitEntity
* CommitEntity
* UserEntity
* Kaminari::Helpers::Paginator
* CurrentUserEntity
* ActionView::Base
* ActionDispatch::DebugExceptions::DebugView
* MarkupHelper
* MergeRequestPresenter
After this change:
* Kaminari::Helpers::Paginator
* ActionView::Base
* ActionDispatch::DebugExceptions::DebugView
Each time a partial is rendered for the first time, all methods for
those modules will have to be redefined. This can exact a significant
performance penalty.
How bad is this penalty? Using the following benchmark script, we can
use DTrace to sample the Ruby interpreter:
```
Benchmark.bm do |x|
x.report do
1000.times do
ActionView::CompiledTemplates.module_eval("def testme\nend")
end
end
end
```
This revealed a 11x jump in the time spent in `core#define_method`
alone.
Rails 6 fixes this behavior by moving the `include CompiledTemplates`
into ActionView::Base so that including `ActionView::Context` doesn't
quietly affect other modules in this way.
Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/11198
-rw-r--r-- | app/helpers/markup_helper.rb | 2 | ||||
-rw-r--r-- | app/serializers/concerns/user_status_tooltip.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/sh-fix-slow-partial-rendering.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/action_view_output/context.rb | 41 |
4 files changed, 48 insertions, 2 deletions
diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index ad77f99fe44..dce4168ad7b 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -4,7 +4,7 @@ require 'nokogiri' module MarkupHelper include ActionView::Helpers::TagHelper - include ActionView::Context + include ::Gitlab::ActionViewOutput::Context def plain?(filename) Gitlab::MarkupHelper.plain?(filename) diff --git a/app/serializers/concerns/user_status_tooltip.rb b/app/serializers/concerns/user_status_tooltip.rb index 633b117d392..a81e377691e 100644 --- a/app/serializers/concerns/user_status_tooltip.rb +++ b/app/serializers/concerns/user_status_tooltip.rb @@ -3,7 +3,7 @@ module UserStatusTooltip extend ActiveSupport::Concern include ActionView::Helpers::TagHelper - include ActionView::Context + include ::Gitlab::ActionViewOutput::Context include EmojiHelper include UsersHelper diff --git a/changelogs/unreleased/sh-fix-slow-partial-rendering.yml b/changelogs/unreleased/sh-fix-slow-partial-rendering.yml new file mode 100644 index 00000000000..0f65a6f8d69 --- /dev/null +++ b/changelogs/unreleased/sh-fix-slow-partial-rendering.yml @@ -0,0 +1,5 @@ +--- +title: Fix slow performance with compiling HAML templates +merge_request: 27782 +author: +type: performance diff --git a/lib/gitlab/action_view_output/context.rb b/lib/gitlab/action_view_output/context.rb new file mode 100644 index 00000000000..9fbc9811636 --- /dev/null +++ b/lib/gitlab/action_view_output/context.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +# This file was simplified from https://raw.githubusercontent.com/rails/rails/195f39804a7a4a0034f25e8704220e03d95a752a/actionview/lib/action_view/context.rb. +# +# It is only needed by modules that need to call ActionView helper +# methods (e.g. those in +# https://github.com/rails/rails/tree/c4d3e202e10ae627b3b9c34498afb45450652421/actionview/lib/action_view/helpers) +# to generate tags outside of a Rails controller (e.g. API, Sidekiq, +# etc.). +# +# In Rails 5, ActionView::Context automatically includes CompiledTemplates. +# This means that any module that includes ActionView::Context is now a descendant +# of CompiledTemplates. +# +# When a partial is rendered for the first time, it runs +# Module#module_eval, which will evaluate a string source that defines a +# new method. For example: +# +# def _app_views_profiles_show_html_haml___1285955918103175884_70307801785400(local_assigns, output_buffer) +# "hello world" +# end +# +# When a new method is defined, the Ruby interpreter clears the method +# cache for all descendants, and all methods for those modules will have +# to be redefined. This can lead to a significant performance penalty. +# +# Rails 6 fixes this behavior by moving out the `include +# CompiledTemplates` into ActionView::Base so that including `ActionView::Context` +# doesn't quietly affect other modules in this way. + +if Rails::VERSION::STRING.start_with?('6') + raise 'This module is no longer needed in Rails 6. Use ActionView::Context instead.' +end + +module Gitlab + module ActionViewOutput + module Context + attr_accessor :output_buffer, :view_flow + end + end +end |