From 5b33f5e407fcee961ac8723b7ff2c1bd61bbb42f Mon Sep 17 00:00:00 2001 From: Eric Date: Sun, 8 Sep 2024 15:49:26 +0200 Subject: [PATCH 1/8] Only 1 to_hash to prepare_path --- lib/grape/endpoint.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 9072ba877..2684bf679 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] + inheritable_setting.route[:saved_validations].concat(namespace_stackable[:validations]) if namespace_stackable[: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,8 @@ def map_routes end def prepare_path(path) - path_settings = inheritable_setting.to_hash[:namespace_stackable].merge(inheritable_setting.to_hash[:namespace_inheritable]) + settings = inheritable_setting.to_hash + path_settings = settings[:namespace_stackable].merge!(settings[:namespace_inheritable]) Path.new(path, namespace, path_settings) end From 156e405ceb857227ece990b9b970f6312f268ad1 Mon Sep 17 00:00:00 2001 From: Eric Date: Sun, 8 Sep 2024 16:03:48 +0200 Subject: [PATCH 2/8] Use inheritable_setting.namespace_stackable and inheritable_setting.namespace_inheritable instead --- lib/grape/endpoint.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 2684bf679..ebec676e0 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -205,8 +205,9 @@ def map_routes end def prepare_path(path) - settings = inheritable_setting.to_hash - path_settings = settings[:namespace_stackable].merge!(settings[: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 From c4ed47991e43dbf0b116687f27b3ef384b778322 Mon Sep 17 00:00:00 2001 From: Eric Date: Sun, 8 Sep 2024 16:49:37 +0200 Subject: [PATCH 3/8] Use `include` instead of `send(:include)` --- lib/grape/middleware/error.rb | 6 +++--- spec/grape/extensions/param_builders/hash_spec.rb | 2 +- .../param_builders/hash_with_indifferent_access_spec.rb | 2 +- spec/integration/hashie/hashie_spec.rb | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 6e5304607..f201ee8ef 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 872330bb4..fdce95226 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 0f741193b..97b4e56cd 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 8c8314997..73c97ce1e 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 From fe655f6892c1fd0fbdc2d8d7c8045591408d8a3b Mon Sep 17 00:00:00 2001 From: Eric Date: Sun, 8 Sep 2024 16:49:58 +0200 Subject: [PATCH 4/8] Use `include` instead of `send(:include)` --- lib/grape/dsl/helpers.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/grape/dsl/helpers.rb b/lib/grape/dsl/helpers.rb index ef8118c0e..180712282 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 From 8a99cc6253495256b54a4ea10fa3487bc94d4b53 Mon Sep 17 00:00:00 2001 From: Eric Date: Sun, 8 Sep 2024 16:50:12 +0200 Subject: [PATCH 5/8] small refactor --- lib/grape/endpoint.rb | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index ebec676e0..b4c5d33cd 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -114,9 +114,9 @@ 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].concat(namespace_stackable[:validations]) if namespace_stackable[:validations].any? + 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.any? endpoints&.each { |e| e.inherit_settings(namespace_stackable) } @@ -290,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) @@ -317,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) @@ -330,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 unless helpers + + Module.new { helpers.each { |mod_to_include| include mod_to_include } } end private :build_stack, :build_helpers @@ -349,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 From 0a72aced60beaeae3531d1e79253fcc34ba5279d Mon Sep 17 00:00:00 2001 From: Eric Date: Sun, 8 Sep 2024 17:36:58 +0200 Subject: [PATCH 6/8] Add CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eda07b1b..5c55cd504 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 Hash allocation when compiling - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 2.1.3 (2024-07-13) From 438faf90630b9b4a9a2a4862ff9ed6daab6bc2fb Mon Sep 17 00:00:00 2001 From: Eric Date: Sun, 8 Sep 2024 17:44:15 +0200 Subject: [PATCH 7/8] Return if helpers.empty? instead. --- lib/grape/endpoint.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index b4c5d33cd..5fdb03567 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -333,7 +333,7 @@ def build_stack(helpers) def build_helpers helpers = namespace_stackable(:helpers) - return unless helpers + return if helpers.empty? Module.new { helpers.each { |mod_to_include| include mod_to_include } } end From d4fe9d89161290e2e4d1a0144dfb4ad9c2eeedf7 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 8 Sep 2024 17:48:59 +0200 Subject: [PATCH 8/8] Update CHANGELOG.md Object allocation instead of just hash --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c55cd504..01057484f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +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 Hash allocation when compiling - [@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)