Optimize hash alloc (#2512)

* Remove double splat operators when not needed including specs
Grape::Validations::Validators::Base last arguments is now just a simple param
Grape::Exceptions::Validation and Grape::Exceptions::ValidationErrors does not end with **args.
Grape::Request has a named param build_params_with: nil instead of **options
Remove @namespace_description in routing.rb

* Revert compile_many_routes.rb

* Optimize Head Route

* Use dup instead of new for Head Route

* Use opts = {} for validator instead of just opts
rubocop

* Optimize add_head_not_allowed_methods_and_options_methods
Renamed to add_head_and_options_methods

* Remove requirements and path from greedy route (not used)
allow header is joined

* Remove {} for opts since its internal.

* Fix rubocop

* Remove frozen allow_header. Now Dynamic

* Update CHANGELOG.md

Author: ericproulx

CHANGELOG.md

@@ -6,6 +6,7 @@
 * [#2500](https://github.com/ruby-grape/grape/pull/2500): Remove deprecated `file` method - [@ericproulx](https://github.com/ericproulx).
 * [#2501](https://github.com/ruby-grape/grape/pull/2501): Remove deprecated `except` and `proc` options in values validator - [@ericproulx](https://github.com/ericproulx).
 * [#2502](https://github.com/ruby-grape/grape/pull/2502): Remove deprecation `options` in `desc` - [@ericproulx](https://github.com/ericproulx).
+* [#2512](https://github.com/ruby-grape/grape/pull/2512): Optimize hash alloc - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/api.rb

@@ -91,7 +91,7 @@ def const_missing(*args)
       # For instance, a description could be done using: `desc configuration[:description]` if it may vary
       # depending on where the endpoint is mounted. Use with care, if you find yourself using configuration
       # too much, you may actually want to provide a new API rather than remount it.
-      def mount_instance(**opts)
+      def mount_instance(opts = {})
         instance = Class.new(@base_parent)
         instance.configuration = Grape::Util::EndpointConfiguration.new(opts[:configuration] || {})
         instance.base = self

lib/grape/api/instance.rb

@@ -194,88 +194,52 @@ def cascade?
       # will return an HTTP 405 response for any HTTP method that the resource
       # cannot handle.
       def add_head_not_allowed_methods_and_options_methods
-        versioned_route_configs = collect_route_config_per_pattern
         # The paths we collected are prepared (cf. Path#prepare), so they
         # contain already versioning information when using path versioning.
+        all_routes = self.class.endpoints.map(&:routes).flatten
+
         # Disable versioning so adding a route won't prepend versioning
         # informations again.
-        without_root_prefix do
-          without_versioning do
-            versioned_route_configs.each do |config|
-              next if config[:options][:matching_wildchar]
-
-              allowed_methods = config[:methods].dup
-
-              allowed_methods |= [Rack::HEAD] if !self.class.namespace_inheritable(:do_not_route_head) && allowed_methods.include?(Rack::GET)
-
-              allow_header = (self.class.namespace_inheritable(:do_not_route_options) ? allowed_methods : [Rack::OPTIONS] | allowed_methods)
-
-              config[:endpoint].options[:options_route_enabled] = true unless self.class.namespace_inheritable(:do_not_route_options) || allowed_methods.include?(Rack::OPTIONS)
-
-              attributes = config.merge(allowed_methods: allowed_methods, allow_header: allow_header)
-              generate_not_allowed_method(config[:pattern], **attributes)
-            end
-          end
-        end
+        without_root_prefix_and_versioning { collect_route_config_per_pattern(all_routes) }
       end
 
-      def collect_route_config_per_pattern
-        all_routes       = self.class.endpoints.map(&:routes).flatten
+      def collect_route_config_per_pattern(all_routes)
         routes_by_regexp = all_routes.group_by(&:pattern_regexp)
 
         # Build the configuration based on the first endpoint and the collection of methods supported.
-        routes_by_regexp.values.map do |routes|
-          last_route        = routes.last # Most of the configuration is taken from the last endpoint
-          matching_wildchar = routes.any? { |route| route.request_method == '*' }
-          {
-            options: { matching_wildchar: matching_wildchar },
-            pattern: last_route.pattern,
-            requirements: last_route.requirements,
-            path: last_route.origin,
-            endpoint: last_route.app,
-            methods: matching_wildchar ? Grape::Http::Headers::SUPPORTED_METHODS : routes.map(&:request_method)
-          }
-        end
-      end
+        routes_by_regexp.each_value do |routes|
+          last_route = routes.last # Most of the configuration is taken from the last endpoint
+          next if routes.any? { |route| route.request_method == '*' }
 
-      # Generate a route that returns an HTTP 405 response for a user defined
-      # path on methods not specified
-      def generate_not_allowed_method(pattern, allowed_methods: [], **attributes)
-        supported_methods =
-          if self.class.namespace_inheritable(:do_not_route_options)
-            Grape::Http::Headers::SUPPORTED_METHODS
-          else
-            Grape::Http::Headers::SUPPORTED_METHODS_WITHOUT_OPTIONS
-          end
-        not_allowed_methods = supported_methods - allowed_methods
-        @router.associate_routes(pattern, not_allowed_methods: not_allowed_methods, **attributes)
+          allowed_methods = routes.map(&:request_method)
+          allowed_methods |= [Rack::HEAD] if !self.class.namespace_inheritable(:do_not_route_head) && allowed_methods.include?(Rack::GET)
+
+          allow_header = self.class.namespace_inheritable(:do_not_route_options) ? allowed_methods : [Rack::OPTIONS] | allowed_methods
+          last_route.app.options[:options_route_enabled] = true unless self.class.namespace_inheritable(:do_not_route_options) || allowed_methods.include?(Rack::OPTIONS)
+
+          @router.associate_routes(last_route.pattern, {
+                                     endpoint: last_route.app,
+                                     allow_header: allow_header
+                                   })
+        end
       end
 
       # Allows definition of endpoints that ignore the versioning configuration
       # used by the rest of your API.
-      def without_versioning(&_block)
+      def without_root_prefix_and_versioning
         old_version = self.class.namespace_inheritable(:version)
         old_version_options = self.class.namespace_inheritable(:version_options)
+        old_root_prefix = self.class.namespace_inheritable(:root_prefix)
 
         self.class.namespace_inheritable_to_nil(:version)
         self.class.namespace_inheritable_to_nil(:version_options)
+        self.class.namespace_inheritable_to_nil(:root_prefix)
 
         yield
 
         self.class.namespace_inheritable(:version, old_version)
         self.class.namespace_inheritable(:version_options, old_version_options)
-      end
-
-      # Allows definition of endpoints that ignore the root prefix used by the
-      # rest of your API.
-      def without_root_prefix(&_block)
-        old_prefix = self.class.namespace_inheritable(:root_prefix)
-
-        self.class.namespace_inheritable_to_nil(:root_prefix)
-
-        yield
-
-        self.class.namespace_inheritable(:root_prefix, old_prefix)
+        self.class.namespace_inheritable(:root_prefix, old_root_prefix)
       end
     end
   end

lib/grape/dsl/inside_route.rb

@@ -26,8 +26,8 @@ def self.post_filter_methods(type)
       # has completed
       module PostBeforeFilter
         def declared(passed_params, options = {}, declared_params = nil, params_nested_path = [])
-          options = options.reverse_merge(include_missing: true, include_parent_namespaces: true, evaluate_given: false)
-          declared_params ||= optioned_declared_params(**options)
+          options.reverse_merge!(include_missing: true, include_parent_namespaces: true, evaluate_given: false)
+          declared_params ||= optioned_declared_params(options[:include_parent_namespaces])
 
           res = if passed_params.is_a?(Array)
                   declared_array(passed_params, options, declared_params, params_nested_path)
@@ -120,8 +120,8 @@ def optioned_param_key(declared_param, options)
           options[:stringify] ? declared_param.to_s : declared_param.to_sym
         end
 
-        def optioned_declared_params(**options)
-          declared_params = if options[:include_parent_namespaces]
+        def optioned_declared_params(include_parent_namespaces)
+          declared_params = if include_parent_namespaces
                               # Declared params including parent namespaces
                               route_setting(:declared_params)
                             else
@@ -199,10 +199,9 @@ def rack_response(message, status = 200, headers = { Rack::CONTENT_TYPE => conte
       # Redirect to a new url.
       #
       # @param url [String] The url to be redirect.
-      # @param options [Hash] The options used when redirect.
-      #                       :permanent, default false.
-      #                       :body, default a short message including the URL.
-      def redirect(url, permanent: false, body: nil, **_options)
+      # @param permanent [Boolean] default false.
+      # @param body default a short message including the URL.
+      def redirect(url, permanent: false, body: nil)
         body_message = body
         if permanent
           status 301

lib/grape/dsl/parameters.rb

@@ -136,7 +136,7 @@ def requires(*attrs, &block)
           require_required_and_optional_fields(attrs.first, opts)
         else
           validate_attributes(attrs, opts, &block)
-          block ? new_scope(orig_attrs, &block) : push_declared_params(attrs, **opts.slice(:as))
+          block ? new_scope(orig_attrs, &block) : push_declared_params(attrs, opts.slice(:as))
         end
       end
 
@@ -162,7 +162,7 @@ def optional(*attrs, &block)
         else
           validate_attributes(attrs, opts, &block)
 
-          block ? new_scope(orig_attrs, true, &block) : push_declared_params(attrs, **opts.slice(:as))
+          block ? new_scope(orig_attrs, true, &block) : push_declared_params(attrs, opts.slice(:as))
         end
       end
 

lib/grape/dsl/routing.rb

@@ -175,19 +175,12 @@ def route(methods, paths = ['/'], route_options = {}, &block)
         #       end
         #     end
         def namespace(space = nil, options = {}, &block)
-          @namespace_description = nil unless instance_variable_defined?(:@namespace_description) && @namespace_description
-
-          if space || block
-            within_namespace do
-              previous_namespace_description = @namespace_description
-              @namespace_description = (@namespace_description || {}).deep_merge(namespace_setting(:description) || {})
-              nest(block) do
-                namespace_stackable(:namespace, Namespace.new(space, **options)) if space
-              end
-              @namespace_description = previous_namespace_description
+          return Namespace.joined_space_path(namespace_stackable(:namespace)) unless space || block
+
+          within_namespace do
+            nest(block) do
+              namespace_stackable(:namespace, Namespace.new(space, options)) if space
             end
-          else
-            Namespace.joined_space_path(namespace_stackable(:namespace))
           end
         end
 

lib/grape/endpoint.rb

@@ -145,17 +145,16 @@ def reset_routes!
     end
 
     def mount_in(router)
-      if endpoints
-        endpoints.each { |e| e.mount_in(router) }
-      else
-        reset_routes!
-        routes.each do |route|
-          methods = [route.request_method]
-          methods << Rack::HEAD if !namespace_inheritable(:do_not_route_head) && route.request_method == Rack::GET
-          methods.each do |method|
-            route = Grape::Router::Route.new(method, route.origin, **route.attributes.to_h) unless route.request_method == method
-            router.append(route.apply(self))
-          end
+      return endpoints.each { |e| e.mount_in(router) } if endpoints
+
+      reset_routes!
+      routes.each do |route|
+        router.append(route.apply(self))
+        next unless !namespace_inheritable(:do_not_route_head) && route.request_method == Rack::GET
+
+        route.dup.then do |head_route|
+          head_route.convert_to_head_request!
+          router.append(head_route.apply(self))
         end
       end
     end
@@ -164,8 +163,9 @@ def to_routes
       route_options = prepare_default_route_attributes
       map_routes do |method, path|
         path = prepare_path(path)
-        params = merge_route_options(**route_options.merge(suffix: path.suffix))
-        route = Router::Route.new(method, path.path, **params)
+        route_options[:suffix] = path.suffix
+        params = options[:route_options].merge(route_options)
+        route = Grape::Router::Route.new(method, path.path, params)
         route.apply(self)
       end.flatten
     end
@@ -196,10 +196,6 @@ def prepare_version
       version.length == 1 ? version.first : version
     end
 
-    def merge_route_options(**default)
-      options[:route_options].clone.merge!(**default)
-    end
-
     def map_routes
       options[:method].map { |method| options[:path].map { |path| yield method, path } }
     end
@@ -259,9 +255,10 @@ def run
           run_filters befores, :before
 
           if (allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS])
-            raise Grape::Exceptions::MethodNotAllowed.new(header.merge('Allow' => allowed_methods)) unless options?
+            allow_header_value = allowed_methods.join(', ')
+            raise Grape::Exceptions::MethodNotAllowed.new(header.merge('Allow' => allow_header_value)) unless options?
 
-            header Grape::Http::Headers::ALLOW, allowed_methods
+            header Grape::Http::Headers::ALLOW, allow_header_value
             response_object = ''
             status 204
           else
@@ -287,59 +284,6 @@ def run
       end
     end
 
-    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: 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),
-                all_rescue_handler: namespace_inheritable(:all_rescue_handler),
-                grape_exceptions_rescue_handler: namespace_inheritable(:grape_exceptions_rescue_handler)
-
-      stack.concat namespace_stackable(:middleware)
-
-      if namespace_inheritable(:version).present?
-        stack.use Grape::Middleware::Versioner.using(namespace_inheritable(:version_options)[:using]),
-                  versions: namespace_inheritable(:version).flatten,
-                  version_options: namespace_inheritable(:version_options),
-                  prefix: namespace_inheritable(:root_prefix),
-                  mount_path: namespace_stackable(:mount_path).first
-      end
-
-      stack.use Grape::Middleware::Formatter,
-                format: format,
-                default_format: namespace_inheritable(:default_format) || :txt,
-                content_types: content_types,
-                formatters: namespace_stackable_with_hash(:formatters),
-                parsers: namespace_stackable_with_hash(:parsers)
-
-      builder = stack.build
-      builder.run ->(env) { env[Grape::Env::API_ENDPOINT].run }
-      builder.to_app
-    end
-
-    def build_helpers
-      helpers = namespace_stackable(:helpers)
-      return if helpers.empty?
-
-      Module.new { helpers.each { |mod_to_include| include mod_to_include } }
-    end
-
-    private :build_stack, :build_helpers
-
     def execute
       @block&.call(self)
     end
@@ -411,13 +355,66 @@ def validations
       return enum_for(:validations) unless block_given?
 
       route_setting(:saved_validations)&.each do |saved_validation|
-        yield Grape::Validations::ValidatorFactory.create_validator(**saved_validation)
+        yield Grape::Validations::ValidatorFactory.create_validator(saved_validation)
       end
     end
 
     def options?
       options[:options_route_enabled] &&
         env[Rack::REQUEST_METHOD] == Rack::OPTIONS
     end
+
+    private
+
+    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: 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),
+                all_rescue_handler: namespace_inheritable(:all_rescue_handler),
+                grape_exceptions_rescue_handler: namespace_inheritable(:grape_exceptions_rescue_handler)
+
+      stack.concat namespace_stackable(:middleware)
+
+      if namespace_inheritable(:version).present?
+        stack.use Grape::Middleware::Versioner.using(namespace_inheritable(:version_options)[:using]),
+                  versions: namespace_inheritable(:version).flatten,
+                  version_options: namespace_inheritable(:version_options),
+                  prefix: namespace_inheritable(:root_prefix),
+                  mount_path: namespace_stackable(:mount_path).first
+      end
+
+      stack.use Grape::Middleware::Formatter,
+                format: format,
+                default_format: namespace_inheritable(:default_format) || :txt,
+                content_types: content_types,
+                formatters: namespace_stackable_with_hash(:formatters),
+                parsers: namespace_stackable_with_hash(:parsers)
+
+      builder = stack.build
+      builder.run ->(env) { env[Grape::Env::API_ENDPOINT].run }
+      builder.to_app
+    end
+
+    def build_helpers
+      helpers = namespace_stackable(:helpers)
+      return if helpers.empty?
+
+      Module.new { helpers.each { |mod_to_include| include mod_to_include } }
+    end
   end
 end