Skip to content

Commit d956e44

Browse files
authored
Reduce array alloc on mount (#2543)
* Use << instead of += Some refactor Remove wrongful spec * Fix refresh_mount_step * Add CHANGELOG.md + missing `your contribution here`
1 parent 45abe0d commit d956e44

File tree

3 files changed

+32
-34
lines changed

3 files changed

+32
-34
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#### Fixes
1313

1414
* [#2538](https://github.com/ruby-grape/grape/pull/2538): Fix validating nested json array params - [@mohammednasser-32](https://github.com/mohammednasser-32).
15+
* [#2543](https://github.com/ruby-grape/grape/pull/2543): Fix array allocation on mount - [@ericproulx](https://github.com/ericproulx).
16+
* Your contribution here.
1517

1618
### 2.3.0 (2025-02-08)
1719

lib/grape/api.rb

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ def initial_setup(base_instance_parent)
5555
def override_all_methods!
5656
(base_instance.methods - Class.methods - NON_OVERRIDABLE).each do |method_override|
5757
define_singleton_method(method_override) do |*args, &block|
58-
add_setup(method_override, *args, &block)
58+
add_setup(method: method_override, args: args, block: block)
5959
end
6060
end
6161
end
@@ -79,24 +79,24 @@ def configure
7979
# For instance, a description could be done using: `desc configuration[:description]` if it may vary
8080
# depending on where the endpoint is mounted. Use with care, if you find yourself using configuration
8181
# too much, you may actually want to provide a new API rather than remount it.
82-
def mount_instance(opts = {})
83-
instance = Class.new(@base_parent)
84-
instance.configuration = Grape::Util::EndpointConfiguration.new(opts[:configuration] || {})
85-
instance.base = self
86-
replay_setup_on(instance)
87-
instance
82+
def mount_instance(configuration: nil)
83+
Class.new(@base_parent).tap do |instance|
84+
instance.configuration = Grape::Util::EndpointConfiguration.new(configuration || {})
85+
instance.base = self
86+
replay_setup_on(instance)
87+
end
8888
end
8989

90+
private
91+
9092
# Replays the set up to produce an API as defined in this class, can be called
9193
# on classes that inherit from Grape::API
9294
def replay_setup_on(instance)
9395
@setup.each do |setup_step|
94-
replay_step_on(instance, setup_step)
96+
replay_step_on(instance, **setup_step)
9597
end
9698
end
9799

98-
private
99-
100100
def instance_for_rack
101101
if never_mounted?
102102
base_instance
@@ -106,34 +106,35 @@ def instance_for_rack
106106
end
107107

108108
# Adds a new stage to the set up require to get a Grape::API up and running
109-
def add_setup(method, *args, &block)
110-
setup_step = { method: method, args: args, block: block }
111-
@setup += [setup_step]
109+
def add_setup(step)
110+
@setup << step
112111
last_response = nil
113112
@instances.each do |instance|
114-
last_response = replay_step_on(instance, setup_step)
113+
last_response = replay_step_on(instance, **step)
115114
end
116115

117-
# Updating all previously mounted classes in the case that new methods have been executed.
118-
if method != :mount && @setup.any?
119-
previous_mount_steps = @setup.select { |step| step[:method] == :mount }
120-
previous_mount_steps.each do |mount_step|
121-
refresh_mount_step = mount_step.merge(method: :refresh_mounted_api)
122-
@setup += [refresh_mount_step]
123-
@instances.each do |instance|
124-
replay_step_on(instance, refresh_mount_step)
125-
end
116+
refresh_mount_step if step[:method] != :mount
117+
last_response
118+
end
119+
120+
# Updating all previously mounted classes in the case that new methods have been executed.
121+
def refresh_mount_step
122+
@setup.each do |setup_step|
123+
next if setup_step[:method] != :mount
124+
125+
refresh_mount_step = setup_step.merge(method: :refresh_mounted_api)
126+
@setup << refresh_mount_step
127+
@instances.each do |instance|
128+
replay_step_on(instance, **refresh_mount_step)
126129
end
127130
end
128-
129-
last_response
130131
end
131132

132-
def replay_step_on(instance, setup_step)
133-
return if skip_immediate_run?(instance, setup_step[:args])
133+
def replay_step_on(instance, method:, args:, block:)
134+
return if skip_immediate_run?(instance, args)
134135

135-
args = evaluate_arguments(instance.configuration, *setup_step[:args])
136-
response = instance.send(setup_step[:method], *args, &setup_step[:block])
136+
eval_args = evaluate_arguments(instance.configuration, *args)
137+
response = instance.send(method, *eval_args, &block)
137138
if skip_immediate_run?(instance, [response])
138139
response
139140
else

spec/grape/api_spec.rb

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,11 +1699,6 @@ def self.io
16991699
expect(subject.io.string).to include(message)
17001700
end
17011701
end
1702-
1703-
it 'does not unnecessarily retain duplicate setup blocks' do
1704-
subject.logger
1705-
expect { subject.logger }.not_to change(subject.instance_variable_get(:@setup), :size)
1706-
end
17071702
end
17081703

17091704
describe '.helpers' do

0 commit comments

Comments
 (0)