Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize memory alloc and retained #2441

Merged
merged 9 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
3 changes: 2 additions & 1 deletion lib/grape/api/instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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|
Expand Down
4 changes: 2 additions & 2 deletions lib/grape/dsl/routing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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.
Expand Down
7 changes: 4 additions & 3 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/grape/exceptions/validation_array_errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class ValidationArrayErrors < Base
attr_reader :errors

def initialize(errors)
super()
@errors = errors
end
end
Expand Down
5 changes: 3 additions & 2 deletions lib/grape/namespace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 24 additions & 27 deletions lib/grape/path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -58,7 +50,7 @@ def suffix
end

def path
Grape::Router.normalize_path(PartsCache[parts])
PartsCache[parts]
end

def path_with_suffix
Expand All @@ -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
25 changes: 11 additions & 14 deletions lib/grape/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
63 changes: 0 additions & 63 deletions lib/grape/router/attribute_translator.rb

This file was deleted.

39 changes: 39 additions & 0 deletions lib/grape/router/base_route.rb
Original file line number Diff line number Diff line change
@@ -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
Loading