Build the cookie jar only on use and drop its deferral Proc#2757
Open
ericproulx wants to merge 1 commit into
Open
Build the cookie jar only on use and drop its deferral Proc#2757ericproulx wants to merge 1 commit into
ericproulx wants to merge 1 commit into
Conversation
Danger ReportNo issues found. |
a60c995 to
8075e9b
Compare
Two related steps that make the response-cookie path lazy at the request
boundary:
- `build_response_cookies` runs on every request and went through
`cookies` -> `request.cookies`, materializing the `Grape::Cookies` jar
even when the handler never touched a cookie. Gate it on a new
`Grape::Request#cookies?` predicate (true only once the jar exists),
so a cookie-free request allocates no jar at all.
- With the jar now built only when a cookie is actually read or written,
its `-> { rack_cookies }` deferral Proc no longer earns its keep: any
access immediately forces the parse, and a write (`[]=`) always did.
Parse `rack_cookies` eagerly in the constructor and replace the
`is_a?(Proc)` lazy reader with a plain `attr_reader`, dropping the
closure allocation and the per-access branch.
The `ActiveSupport::HashWithIndifferentAccess` wrapping is unchanged --
it backs Grape's string/symbol-indifferent cookie access.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8075e9b to
c2b0cf5
Compare
dblock
approved these changes
Jun 2, 2026
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
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.
Makes the response-cookie path lazy at the request boundary, in two related steps.
1. Build the jar only when a cookie is touched
Grape::Endpoint#build_response_cookiesruns on every request and delegatedresponse_cookies→cookies→request.cookies, materializing theGrape::Cookiesjar even when the handler never touched a cookie (the jar'sresponse_cookiesthen immediately no-ops). Gate it on a newGrape::Request#cookies?predicate, true only once the jar exists:A
Set-Cookieis only ever emitted after acookies[...] =write, which always materializes the jar first — socookies?is true exactly when there could be something to flush. Behaviour is unchanged; a cookie-free request now allocates no jar.2. Drop the jar's deferral
ProcThe
-> { rack_cookies }Proc existed (since #2549) to defer the request-cookie parse, because the jar used to be built on every request. Now that step 1 only builds it when a cookie is actually read or written — and any access forces the parse immediately (a write via[]=always did) — the Proc no longer earns its keep. Parserack_cookieseagerly in the constructor and replace theis_a?(Proc)lazy reader with a plainattr_reader, dropping the closure allocation and a per-access branch.The
ActiveSupport::HashWithIndifferentAccesswrapping is unchanged — it backs Grape's string/symbol-indifferent cookie access (set_cookie username=…is read back ascookies[:username]).Result
A request that never touches a cookie now allocates zero
Grape::Cookies/Procobjects (was ~10 on the flush path); a cookie-using request loses the extraProcclosure and the per-accessis_a?(Proc)check.Tests
spec/grape/endpoint_spec.rb#cookies— 5 examples, 0 failures (set / update / delete / symbol-keyed read / "does not set response cookies"). Rubocop clean.🤖 Generated with Claude Code