From 2d94dd8b705e2f5dfaa97cc6f706e1ceb0b9e87c Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 8 Sep 2024 18:03:05 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + lib/grape/dsl/helpers.rb | 10 ++++-- lib/grape/endpoint.rb | 33 +++++++++++-------- lib/grape/middleware/error.rb | 6 ++-- .../extensions/param_builders/hash_spec.rb | 2 +- .../hash_with_indifferent_access_spec.rb | 2 +- spec/integration/hashie/hashie_spec.rb | 2 +- 7 files changed, 34 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eda07b1b5..01057484f0 100644 --- 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 index ef8118c0ef..180712282b 100644 --- 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 index 9072ba877c..5fdb03567a 100644 --- 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 index 6e5304607b..f201ee8ef8 100644 --- 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,7 +79,7 @@ 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 @@ -87,7 +87,7 @@ def rescue_handler_for_base_only_class(klass) 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 index 872330bb45..fdce952264 100644 --- 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 index 0f741193b2..97b4e56cd8 100644 --- 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 index 8c8314997f..73c97ce1e1 100644 --- 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