Optimize memory alloc and retained (#2441)

* Fix Cache in namespace and path
Compile! skip non defined method
Pattern, optimize capture_default
Use delete_if instead of -

* Revert root_prefix and to_regexp

* Refactor path

* Fix prefix, api string allocation

* Drop AttributeTranslator in favor of OrderedOptions
Manage Route regexp in Route class

* Add cache for capture_index

* Drop attribute_translator
Remove useless alias

* Fix all Rubocop Lint/MissingSuper
Add changelog

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-05-10 16:10:58 UTC using RuboCop version 1.63.2.
+# on 2024-05-20 14:55:33 UTC using RuboCop version 1.63.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -40,16 +40,6 @@ Lint/EmptyClass:
   Exclude:
     - 'lib/grape/dsl/parameters.rb'
 
-# Offense count: 5
-# Configuration parameters: AllowedParentClasses.
-Lint/MissingSuper:
-  Exclude:
-    - 'lib/grape/api/instance.rb'
-    - 'lib/grape/exceptions/validation_array_errors.rb'
-    - 'lib/grape/namespace.rb'
-    - 'lib/grape/path.rb'
-    - 'lib/grape/router/pattern.rb'
-
 # Offense count: 1
 # Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:

CHANGELOG.md

@@ -44,6 +44,7 @@
 * [#2414](https://github.com/ruby-grape/grape/pull/2414): Fix Rack::Lint missing content-type - [@ericproulx](https://github.com/ericproulx).
 * [#2378](https://github.com/ruby-grape/grape/pull/2378): Do not overwrite `route_param` with a regular one if they share same name - [@arg](https://github.com/arg).
 * [#2444](https://github.com/ruby-grape/grape/pull/2444): Replace method_missing in endpoint - [@ericproulx](https://github.com/ericproulx).
+* [#2441](https://github.com/ruby-grape/grape/pull/2441): Optimize memory alloc and retained - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 ### 2.0.0 (2023/11/11)

lib/grape.rb

@@ -16,13 +16,15 @@
 require 'active_support/core_ext/hash/keys'
 require 'active_support/core_ext/hash/reverse_merge'
 require 'active_support/core_ext/hash/slice'
+require 'active_support/core_ext/module/delegation'
 require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/object/deep_dup'
 require 'active_support/core_ext/object/duplicable'
 require 'active_support/core_ext/string/output_safety'
 require 'active_support/core_ext/string/exclude'
 require 'active_support/deprecation'
 require 'active_support/inflector'
+require 'active_support/ordered_options'
 require 'active_support/notifications'
 
 require 'English'

lib/grape/api/instance.rb

@@ -125,6 +125,7 @@ def evaluate_as_instance_with_configuration(block, lazy: false)
         end
 
         def inherited(subclass)
+          super
           subclass.reset!
           subclass.logger = logger.clone
         end
@@ -220,7 +221,7 @@ def add_head_not_allowed_methods_and_options_methods
 
       def collect_route_config_per_pattern
         all_routes       = self.class.endpoints.map(&:routes).flatten
-        routes_by_regexp = all_routes.group_by { |route| route.pattern.to_regexp }
+        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|

lib/grape/dsl/routing.rb

@@ -30,7 +30,7 @@ def version(*args, &block)
           if args.any?
             options = args.extract_options!
             options = options.reverse_merge(using: :path)
-            requested_versions = args.flatten
+            requested_versions = args.flatten.map(&:to_s)
 
             raise Grape::Exceptions::MissingVendorOption.new if options[:using] == :header && !options.key?(:vendor)
 
@@ -54,7 +54,7 @@ def version(*args, &block)
 
         # Define a root URL prefix for your entire API.
         def prefix(prefix = nil)
-          namespace_inheritable(:root_prefix, prefix)
+          namespace_inheritable(:root_prefix, prefix&.to_s)
         end
 
         # Create a scope without affecting the URL.

lib/grape/endpoint.rb

@@ -190,10 +190,11 @@ def prepare_default_route_attributes
     end
 
     def prepare_version
-      version = namespace_inheritable(:version) || []
+      version = namespace_inheritable(:version)
+      return unless version
       return if version.empty?
 
-      version.length == 1 ? version.first.to_s : version
+      version.length == 1 ? version.first : version
     end
 
     def merge_route_options(**default)
@@ -206,7 +207,7 @@ def map_routes
 
     def prepare_path(path)
       path_settings = inheritable_setting.to_hash[:namespace_stackable].merge(inheritable_setting.to_hash[:namespace_inheritable])
-      Path.prepare(path, namespace, path_settings)
+      Path.new(path, namespace, path_settings)
     end
 
     def namespace

lib/grape/exceptions/validation_array_errors.rb

@@ -6,6 +6,7 @@ class ValidationArrayErrors < Base
       attr_reader :errors
 
       def initialize(errors)
+        super()
         @errors = errors
       end
     end

lib/grape/namespace.rb

@@ -31,13 +31,14 @@ def self.joined_space(settings)
     # Join the namespaces from a list of settings to create a path prefix.
     # @param settings [Array] list of Grape::Util::InheritableSettings.
     def self.joined_space_path(settings)
-      Grape::Router.normalize_path(JoinedSpaceCache[joined_space(settings)])
+      JoinedSpaceCache[joined_space(settings)]
     end
 
     class JoinedSpaceCache < Grape::Util::Cache
       def initialize
+        super
         @cache = Hash.new do |h, joined_space|
-          h[joined_space] = -joined_space.join('/')
+          h[joined_space] = Grape::Router.normalize_path(joined_space.join('/'))
         end
       end
     end

lib/grape/path.rb

@@ -3,10 +3,6 @@
 module Grape
   # Represents a path to an endpoint.
   class Path
-    def self.prepare(raw_path, namespace, settings)
-      Path.new(raw_path, namespace, settings)
-    end
-
     attr_reader :raw_path, :namespace, :settings
 
     def initialize(raw_path, namespace, settings)
@@ -20,31 +16,27 @@ def mount_path
     end
 
     def root_prefix
-      split_setting(:root_prefix)
+      settings[:root_prefix]
     end
 
     def uses_specific_format?
-      if settings.key?(:format) && settings.key?(:content_types)
-        settings[:format] && Array(settings[:content_types]).size == 1
-      else
-        false
-      end
+      return false unless settings.key?(:format) && settings.key?(:content_types)
+
+      settings[:format] && Array(settings[:content_types]).size == 1
     end
 
     def uses_path_versioning?
-      if settings.key?(:version) && settings[:version_options] && settings[:version_options].key?(:using)
-        settings[:version] && settings[:version_options][:using] == :path
-      else
-        false
-      end
+      return false unless settings.key?(:version) && settings[:version_options]&.key?(:using)
+
+      settings[:version] && settings[:version_options][:using] == :path
     end
 
     def namespace?
-      namespace&.match?(/^\S/) && namespace != '/'
+      namespace&.match?(/^\S/) && not_slash?(namespace)
     end
 
     def path?
-      raw_path&.match?(/^\S/) && raw_path != '/'
+      raw_path&.match?(/^\S/) && not_slash?(raw_path)
     end
 
     def suffix
@@ -58,7 +50,7 @@ def suffix
     end
 
     def path
-      Grape::Router.normalize_path(PartsCache[parts])
+      PartsCache[parts]
     end
 
     def path_with_suffix
@@ -73,24 +65,29 @@ def to_s
 
     class PartsCache < Grape::Util::Cache
       def initialize
+        super
         @cache = Hash.new do |h, parts|
-          h[parts] = -parts.join('/')
+          h[parts] = Grape::Router.normalize_path(parts.join('/'))
         end
       end
     end
 
     def parts
-      parts = [mount_path, root_prefix].compact
-      parts << ':version' if uses_path_versioning?
-      parts << namespace.to_s
-      parts << raw_path.to_s
-      parts.flatten.reject { |part| part == '/' }
+      [].tap do |parts|
+        add_part(parts, mount_path)
+        add_part(parts, root_prefix)
+        parts << ':version' if uses_path_versioning?
+        add_part(parts, namespace)
+        add_part(parts, raw_path)
+      end
     end
 
-    def split_setting(key)
-      return if settings[key].nil?
+    def add_part(parts, value)
+      parts << value if value && not_slash?(value)
+    end
 
-      settings[key].to_s.split('/')
+    def not_slash?(value)
+      value != '/'
     end
   end
 end

lib/grape/router.rb

@@ -12,10 +12,6 @@ def self.normalize_path(path)
       path
     end
 
-    def self.supported_methods
-      @supported_methods ||= Grape::Http::Headers::SUPPORTED_METHODS + ['*']
-    end
-
     def initialize
       @neutral_map = []
       @neutral_regexes = []
@@ -28,13 +24,12 @@ def compile!
 
       @union = Regexp.union(@neutral_regexes)
       @neutral_regexes = nil
-      self.class.supported_methods.each do |method|
+      (Grape::Http::Headers::SUPPORTED_METHODS + ['*']).each do |method|
+        next unless map.key?(method)
+
         routes = map[method]
-        @optimized_map[method] = routes.map.with_index do |route, index|
-          route.index = index
-          Regexp.new("(?<_#{index}>#{route.pattern.to_regexp})")
-        end
-        @optimized_map[method] = Regexp.union(@optimized_map[method])
+        optimized_map = routes.map.with_index { |route, index| route.to_regexp(index) }
+        @optimized_map[method] = Regexp.union(optimized_map)
       end
       @compiled = true
     end
@@ -44,8 +39,10 @@ def append(route)
     end
 
     def associate_routes(pattern, **options)
-      @neutral_regexes << Regexp.new("(?<_#{@neutral_map.length}>)#{pattern.to_regexp}")
-      @neutral_map << Grape::Router::GreedyRoute.new(pattern: pattern, index: @neutral_map.length, **options)
+      Grape::Router::GreedyRoute.new(pattern: pattern, **options).then do |greedy_route|
+        @neutral_regexes << greedy_route.to_regexp(@neutral_map.length)
+        @neutral_map << greedy_route
+      end
     end
 
     def call(env)
@@ -145,11 +142,11 @@ def default_response
     end
 
     def match?(input, method)
-      @optimized_map[method].match(input) { |m| @map[method].detect { |route| m["_#{route.index}"] } }
+      @optimized_map[method].match(input) { |m| @map[method].detect { |route| m[route.regexp_capture_index] } }
     end
 
     def greedy_match?(input)
-      @union.match(input) { |m| @neutral_map.detect { |route| m["_#{route.index}"] } }
+      @union.match(input) { |m| @neutral_map.detect { |route| m[route.regexp_capture_index] } }
     end
 
     def call_with_allow_headers(env, route)

lib/grape/router/base_route.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module Grape
+  class Router
+    class BaseRoute
+      delegate_missing_to :@options
+
+      attr_reader :index, :pattern, :options
+
+      def initialize(**options)
+        @options = ActiveSupport::OrderedOptions.new.update(options)
+      end
+
+      alias attributes options
+
+      def regexp_capture_index
+        CaptureIndexCache[index]
+      end
+
+      def pattern_regexp
+        pattern.to_regexp
+      end
+
+      def to_regexp(index)
+        @index = index
+        Regexp.new("(?<#{regexp_capture_index}>#{pattern_regexp})")
+      end
+
+      class CaptureIndexCache < Grape::Util::Cache
+        def initialize
+          super
+          @cache = Hash.new do |h, index|
+            h[index] = "_#{index}"
+          end
+        end
+      end
+    end
+  end
+end