From b02458ef52ac3e61d3c8b924e092e956375c15f7 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 27 Apr 2019 22:25:45 -0700
Subject: [PATCH] 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
---
 app/helpers/markup_helper.rb                  |  2 +-
 .../concerns/user_status_tooltip.rb           |  2 +-
 .../sh-fix-slow-partial-rendering.yml         |  5 +++
 lib/gitlab/action_view_output/context.rb      | 41 +++++++++++++++++++
 4 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/sh-fix-slow-partial-rendering.yml
 create mode 100644 lib/gitlab/action_view_output/context.rb

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