summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-04-27 22:25:45 -0700
committerStan Hu <stanhu@gmail.com>2019-04-29 05:33:50 -0700
commitb02458ef52ac3e61d3c8b924e092e956375c15f7 (patch)
tree38a90e680302da6557a51e89daf50c157cfec77c
parentd232137aa7d857396060f9ab02d4e99cf8081285 (diff)
downloadgitlab-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.rb2
-rw-r--r--app/serializers/concerns/user_status_tooltip.rb2
-rw-r--r--changelogs/unreleased/sh-fix-slow-partial-rendering.yml5
-rw-r--r--lib/gitlab/action_view_output/context.rb41
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