Commit fe45f544 authored by Sean McGivern's avatar Sean McGivern

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
parents d232137a fad99d93
......@@ -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)
......
......@@ -3,7 +3,7 @@
module UserStatusTooltip
extend ActiveSupport::Concern
include ActionView::Helpers::TagHelper
include ActionView::Context
include ::Gitlab::ActionViewOutput::Context
include EmojiHelper
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
# 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
......@@ -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'
......
# 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
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