Merge branch 'sh-fix-slow-partial-rendering' into 'master'

Fix slow performance with compiling HAML templates

Closes gitlab-ee#11198

See merge request gitlab-org/gitlab-ce!27782

7 files changed, 125 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
diff --git a/rubocop/cop/include_action_view_context.rb b/rubocop/cop/include_action_view_context.rb
new file mode 100644
index 00000000000..14662a33e95
--- /dev/null
+++ b/rubocop/cop/include_action_view_context.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require_relative '../spec_helpers'
+
+module RuboCop
+  module Cop
+    # Cop that makes sure workers include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`.
+    class IncludeActionViewContext < RuboCop::Cop::Cop
+      include SpecHelpers
+
+      MSG = 'Include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`, for Rails 5.'.freeze
+
+      def_node_matcher :includes_action_view_context?, <<~PATTERN
+        (send nil? :include (const (const nil? :ActionView) :Context))
+      PATTERN
+
+      def on_send(node)
+        return if in_spec?(node)
+        return unless includes_action_view_context?(node)
+
+        add_offense(node.arguments.first, location: :expression)
+      end
+
+      def autocorrect(node)
+        lambda do |corrector|
+          corrector.replace(node.source_range, '::Gitlab::ActionViewOutput::Context')
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index 50eab6f9270..ce6bdbf292c 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -4,6 +4,7 @@ require_relative 'cop/gitlab/predicate_memoization'
 require_relative 'cop/gitlab/httparty'
 require_relative 'cop/gitlab/finder_with_find_by'
 require_relative 'cop/gitlab/union'
+require_relative 'cop/include_action_view_context'
 require_relative 'cop/include_sidekiq_worker'
 require_relative 'cop/safe_params'
 require_relative 'cop/active_record_association_reload'
diff --git a/spec/rubocop/cop/include_action_view_context_spec.rb b/spec/rubocop/cop/include_action_view_context_spec.rb
new file mode 100644
index 00000000000..c888555b54f
--- /dev/null
+++ b/spec/rubocop/cop/include_action_view_context_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../rubocop/cop/include_action_view_context'
+
+describe RuboCop::Cop::IncludeActionViewContext do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'when `ActionView::Context` is included' do
+    let(:source) { 'include ActionView::Context' }
+    let(:correct_source) { 'include ::Gitlab::ActionViewOutput::Context' }
+
+    it 'registers an offense' do
+      inspect_source(source)
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([1])
+        expect(cop.highlights).to eq(['ActionView::Context'])
+      end
+    end
+
+    it 'autocorrects to the right version' do
+      autocorrected = autocorrect_source(source)
+
+      expect(autocorrected).to eq(correct_source)
+    end
+  end
+
+  context 'when `ActionView::Context` is not included' do
+    it 'registers no offense' do
+      inspect_source('include Context')
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(0)
+      end
+    end
+  end
+end