Commit b02458ef authored by Stan Hu's avatar Stan Hu

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
parent d232137a
...@@ -4,7 +4,7 @@ require 'nokogiri' ...@@ -4,7 +4,7 @@ require 'nokogiri'
module MarkupHelper module MarkupHelper
include ActionView::Helpers::TagHelper include ActionView::Helpers::TagHelper
include ActionView::Context include ::Gitlab::ActionViewOutput::Context
def plain?(filename) def plain?(filename)
Gitlab::MarkupHelper.plain?(filename) Gitlab::MarkupHelper.plain?(filename)
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module UserStatusTooltip module UserStatusTooltip
extend ActiveSupport::Concern extend ActiveSupport::Concern
include ActionView::Helpers::TagHelper include ActionView::Helpers::TagHelper
include ActionView::Context include ::Gitlab::ActionViewOutput::Context
include EmojiHelper include EmojiHelper
include UsersHelper include UsersHelper
......
---
title: Fix slow performance with compiling HAML templates
merge_request: 27782
author:
type: performance
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment