Avoid empty-hash merges on request hot paths#2689
Merged
Merged
Conversation
ericproulx
force-pushed
the
perf/avoid-empty-hash-merges
branch
from
4949eeb to
1695305
Compare
Danger ReportNo issues found. |
ericproulx
force-pushed
the
perf/avoid-empty-hash-merges
branch
5 times, most recently
from
7d4c209 to
97810df
Compare
dblock
approved these changes
Apr 21, 2026
Three `|| {}` patterns on the per-request path allocated an empty Hash
and handed it to a merge that produced a shallow copy of the left-hand
side — pure waste. Guard the merge when the right-hand side is absent
or empty instead.
When a route has no path placeholders (or none captured), `route.params`
returns an empty Hash (or nil). The old code did
`args.merge(route_params || {})`, allocating `{}` and then a new hash
identical to `args` on every matched static route.
`grape_routing_args` falls back to `{}` when env has no routing args.
`deep_merge!` on `{}` is a walking no-op. Skip the call entirely when
routing_args is empty.
`(body || {}).merge(key => representation)` — when no body is set
(the common case), this allocated `{}` only to merge one key into it.
Build the one-key hash directly.
process_route, no route_params (static path, common case):
old: 4.48 M i/s, 480 objects allocated / 1k calls
new: 8.31 M i/s, 160 objects allocated / 1k calls (1.85x faster, 3x fewer)
process_route, empty {} route_params:
old: 5.16 M i/s, 320 objects / 1k calls
new: 8.02 M i/s, 160 objects / 1k calls (1.55x faster, 2x fewer)
process_route, real route_params: within noise (unchanged path).
No behavior change; all 2,236 specs pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ericproulx
force-pushed
the
perf/avoid-empty-hash-merges
branch
from
97810df to
e91b191
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three
|| {}sites on the per-request path allocated an empty Hash and handed it to a merge that produced a shallow copy of the left-hand side — pure waste. Guard the merge when the right-hand side is absent or empty.Router#process_route—args.merge(route_params || {})→route_params.blank? ? args : args.merge(route_params). On every static route (no path placeholders), this saves both the{}allocation and the identical-copy hash frommerge.Request#make_params—deep_merge!on an empty routing-args hash walked nothing but still paid the call cost; skip it entirely when empty.DSL::Entity#present(keyed form) —(body || {}).merge(key => representation)→ ternary that builds the one-key hash directly whenbodyis nil.No behavior change.
Perf / Benchmarks
Microbenchmark of the
process_routemerge shape, 1,000 calls per run:route_paramsnil (common)route_paramsempty{}route_paramsrealTest plan
bundle exec rspec— 2,236 examples, 0 failures.bundle exec rubocop lib/grape/router.rb lib/grape/request.rb lib/grape/dsl/entity.rb— clean.🤖 Generated with Claude Code