Reduce object allocation while compiling (#2496)

* Only 1 to_hash to prepare_path * Use inheritable_setting.namespace_stackable and inheritable_setting.namespace_inheritable instead * Use `include` instead of `send(:include)` * Use `include` instead of `send(:include)` * small refactor * Add CHANGELOG entry * Return if helpers.empty? instead. * Update CHANGELOG.md Object allocation instead of just hash
ruby-grape · Sep 8, 2024 · 2d94dd8 · 2d94dd8
1 parent 1cf4a80
commit 2d94dd8
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@
 * [#2464](https://github.com/ruby-grape/grape/pull/2464): The `length` validator only takes effect for parameters with types that support `#length` method - [@OuYangJinTing](https://github.com/OuYangJinTing).
 * [#2485](https://github.com/ruby-grape/grape/pull/2485): Add `is:` param to length validator - [@dakad](https://github.com/dakad).
 * [#2492](https://github.com/ruby-grape/grape/pull/2492): Fix `Grape::Endpoint#inspect` method - [@ericproulx](https://github.com/ericproulx).
+* [#2496](https://github.com/ruby-grape/grape/pull/2496): Reduce object allocation when compiling - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 ### 2.1.3 (2024-07-13)

diff --git a/lib/grape/dsl/helpers.rb b/lib/grape/dsl/helpers.rb
@@ -33,18 +33,22 @@ module ClassMethods
         #     end
         #
         def helpers(*new_modules, &block)
-          include_new_modules(new_modules) if new_modules.any?
-          include_block(block) if block
+          include_new_modules(new_modules)
+          include_block(block)
           include_all_in_scope if !block && new_modules.empty?
         end
 
         protected
 
         def include_new_modules(modules)
+          return if modules.empty?
+
           modules.each { |mod| make_inclusion(mod) }
         end
 
         def include_block(block)
+          return unless block
+
           Module.new.tap do |mod|
             make_inclusion(mod) { mod.class_eval(&block) }
           end
@@ -58,7 +62,7 @@ def make_inclusion(mod, &block)
 
         def include_all_in_scope
           Module.new.tap do |mod|
-            namespace_stackable(:helpers).each { |mod_to_include| mod.send :include, mod_to_include }
+            namespace_stackable(:helpers).each { |mod_to_include| mod.include mod_to_include }
             change!
           end
         end

diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb
@@ -114,10 +114,10 @@ def initialize(new_settings, options = {}, &block)
     # Update our settings from a given set of stackable parameters. Used when
     # the endpoint's API is mounted under another one.
     def inherit_settings(namespace_stackable)
-      inheritable_setting.route[:saved_validations] += namespace_stackable[:validations]
+      parent_validations = namespace_stackable[:validations]
+      inheritable_setting.route[:saved_validations].concat(parent_validations) if parent_validations.any?
       parent_declared_params = namespace_stackable[:declared_params]
-
-      inheritable_setting.route[:declared_params].concat(parent_declared_params.flatten) if parent_declared_params
+      inheritable_setting.route[:declared_params].concat(parent_declared_params.flatten) if parent_declared_params.any?
 
       endpoints&.each { |e| e.inherit_settings(namespace_stackable) }
     end
@@ -205,7 +205,9 @@ def map_routes
     end
 
     def prepare_path(path)
-      path_settings = inheritable_setting.to_hash[:namespace_stackable].merge(inheritable_setting.to_hash[:namespace_inheritable])
+      namespace_stackable_hash = inheritable_setting.namespace_stackable.to_hash
+      namespace_inheritable_hash = inheritable_setting.namespace_inheritable.to_hash
+      path_settings = namespace_stackable_hash.merge!(namespace_inheritable_hash)
       Path.new(path, namespace, path_settings)
     end
 
@@ -288,19 +290,22 @@ def run
     def build_stack(helpers)
       stack = Grape::Middleware::Stack.new
 
+      content_types = namespace_stackable_with_hash(:content_types)
+      format = namespace_inheritable(:format)
+
       stack.use Rack::Head
       stack.use Class.new(Grape::Middleware::Error),
                 helpers: helpers,
-                format: namespace_inheritable(:format),
-                content_types: namespace_stackable_with_hash(:content_types),
+                format: format,
+                content_types: content_types,
                 default_status: namespace_inheritable(:default_error_status),
                 rescue_all: namespace_inheritable(:rescue_all),
                 rescue_grape_exceptions: namespace_inheritable(:rescue_grape_exceptions),
                 default_error_formatter: namespace_inheritable(:default_error_formatter),
                 error_formatters: namespace_stackable_with_hash(:error_formatters),
-                rescue_options: namespace_stackable_with_hash(:rescue_options) || {},
-                rescue_handlers: namespace_reverse_stackable_with_hash(:rescue_handlers) || {},
-                base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers) || {},
+                rescue_options: namespace_stackable_with_hash(:rescue_options),
+                rescue_handlers: namespace_reverse_stackable_with_hash(:rescue_handlers),
+                base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers),
                 all_rescue_handler: namespace_inheritable(:all_rescue_handler),
                 grape_exceptions_rescue_handler: namespace_inheritable(:grape_exceptions_rescue_handler)
 
@@ -315,9 +320,9 @@ def build_stack(helpers)
       end
 
       stack.use Grape::Middleware::Formatter,
-                format: namespace_inheritable(:format),
+                format: format,
                 default_format: namespace_inheritable(:default_format) || :txt,
-                content_types: namespace_stackable_with_hash(:content_types),
+                content_types: content_types,
                 formatters: namespace_stackable_with_hash(:formatters),
                 parsers: namespace_stackable_with_hash(:parsers)
 
@@ -328,7 +333,9 @@ def build_stack(helpers)
 
     def build_helpers
       helpers = namespace_stackable(:helpers)
-      Module.new { helpers&.each { |mod_to_include| include mod_to_include } }
+      return if helpers.empty?
+
+      Module.new { helpers.each { |mod_to_include| include mod_to_include } }
     end
 
     private :build_stack, :build_helpers
@@ -347,7 +354,7 @@ def lazy_initialize!
       @lazy_initialize_lock.synchronize do
         return true if @lazy_initialized
 
-        @helpers = build_helpers.tap { |mod| self.class.send(:include, mod) }
+        @helpers = build_helpers&.tap { |mod| self.class.include mod }
         @app = options[:app] || build_stack(@helpers)
 
         @lazy_initialized = true

diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb
@@ -26,7 +26,7 @@ def default_options
 
       def initialize(app, *options)
         super
-        self.class.send(:include, @options[:helpers]) if @options[:helpers]
+        self.class.include(@options[:helpers]) if @options[:helpers]
       end
 
       def call!(env)
@@ -79,15 +79,15 @@ def default_rescue_handler(exception)
       end
 
       def rescue_handler_for_base_only_class(klass)
-        error, handler = options[:base_only_rescue_handlers].find { |err, _handler| klass == err }
+        error, handler = options[:base_only_rescue_handlers]&.find { |err, _handler| klass == err }
 
         return unless error
 
         handler || method(:default_rescue_handler)
       end
 
       def rescue_handler_for_class_or_its_ancestor(klass)
-        error, handler = options[:rescue_handlers].find { |err, _handler| klass <= err }
+        error, handler = options[:rescue_handlers]&.find { |err, _handler| klass <= err }
 
         return unless error
 

diff --git a/spec/grape/extensions/param_builders/hash_spec.rb b/spec/grape/extensions/param_builders/hash_spec.rb
@@ -29,7 +29,7 @@ def app
 
   describe 'in an api' do
     before do
-      subject.send(:include, Grape::Extensions::Hash::ParamBuilder) # rubocop:disable RSpec/DescribedClass
+      subject.include Grape::Extensions::Hash::ParamBuilder # rubocop:disable RSpec/DescribedClass
     end
 
     describe '#params' do

diff --git a/spec/grape/extensions/param_builders/hash_with_indifferent_access_spec.rb b/spec/grape/extensions/param_builders/hash_with_indifferent_access_spec.rb
@@ -29,7 +29,7 @@ def app
 
   describe 'in an api' do
     before do
-      subject.send(:include, Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder) # rubocop:disable RSpec/DescribedClass
+      subject.include Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder # rubocop:disable RSpec/DescribedClass
     end
 
     describe '#params' do

diff --git a/spec/integration/hashie/hashie_spec.rb b/spec/integration/hashie/hashie_spec.rb
@@ -28,7 +28,7 @@
 
     describe 'in an api' do
       before do
-        subject.send(:include, Grape::Extensions::Hashie::Mash::ParamBuilder)
+        subject.include Grape::Extensions::Hashie::Mash::ParamBuilder
       end
 
       describe '#params' do