From 758ad13c580241fa48f35409585ecf74698b4e9d Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 21 Jul 2020 18:48:01 -0700 Subject: [PATCH] Avoid creating helper modules until modified In applications which use :all helpers (the default), most controllers won't be making modifications to their _helpers module. In CRuby this created many ICLASS objects which could cause a large increase in memory usage in applications with many controllers and helpers. To avoid creating unnecessary modules this PR builds modules only when a modification is being made: ethier by calling `helper`, `helper_method`, or through having a default helper (one matching the controller's name) included onto it. --- actionpack/lib/abstract_controller/helpers.rb | 38 ++++++++++++++++--- actionview/lib/action_view/test_case.rb | 2 +- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/abstract_controller/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index 2ca4b78fdd..d05f154bf6 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -7,8 +7,19 @@ module AbstractController extend ActiveSupport::Concern included do - class_attribute :_helpers, default: define_helpers_module(self) class_attribute :_helper_methods, default: Array.new + + # This is here so that it is always higher in the inheritance chain than + # the definition in lib/action_view/rendering.rb + redefine_singleton_method(:_helpers) do + if @_helpers ||= nil + @_helpers + else + superclass._helpers + end + end + + self._helpers = define_helpers_module(self) end class MissingHelperError < LoadError @@ -25,17 +36,24 @@ module AbstractController end end + def _helpers + self.class._helpers + end + module ClassMethods # When a class is inherited, wrap its helper module in a new module. # This ensures that the parent class's module can be changed # independently of the child class's. def inherited(klass) - helpers = _helpers - klass._helpers = define_helpers_module(klass, helpers) + # Inherited from parent by default + klass._helpers = nil + klass.class_eval { default_helper_module! } unless klass.anonymous? super end + attr_writer :_helpers + # Declare a controller method as a helper. For example, the following # makes the +current_user+ and +logged_in?+ controller methods available # to the view: @@ -65,7 +83,7 @@ module AbstractController file, line = location.path, location.lineno methods.each do |method| - _helpers.class_eval <<-ruby_eval, file, line + _helpers_for_modification.class_eval <<-ruby_eval, file, line def #{method}(*args, &block) # def current_user(*args, &block) controller.send(:'#{method}', *args, &block) # controller.send(:'current_user', *args, &block) end # end @@ -127,10 +145,11 @@ module AbstractController # def helper(*args, &block) modules_for_helpers(args).each do |mod| - _helpers.include(mod) + next if _helpers.include?(mod) + _helpers_for_modification.include(mod) end - _helpers.module_eval(&block) if block_given? + _helpers_for_modification.module_eval(&block) if block_given? end # Clears up all existing helpers in this class, only keeping the helper @@ -161,6 +180,13 @@ module AbstractController end end + def _helpers_for_modification + unless @_helpers + self._helpers = define_helpers_module(self, superclass._helpers) + end + _helpers + end + private def define_helpers_module(klass, helpers = nil) # In some tests inherited is called explicitly. In that case, just diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index c93cf64d6e..93971111ef 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -74,7 +74,7 @@ module ActionView def helper_method(*methods) # Almost a duplicate from ActionController::Helpers methods.flatten.each do |method| - _helpers.module_eval <<-end_eval, __FILE__, __LINE__ + 1 + _helpers_for_modification.module_eval <<-end_eval, __FILE__, __LINE__ + 1 def #{method}(*args, &block) # def current_user(*args, &block) _test_case.send(:'#{method}', *args, &block) # _test_case.send(:'current_user', *args, &block) end # end