cookie: improved allocations - #19864
Closed
bagder wants to merge 3 commits into
Closed
Conversation
This delays the allocating of the cookie struct until after all the checks have been done, as many cookies are received and discarded instead of accepted and this then saves one allocation for every discarded cookie.
Member
Author
|
augment review |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR optimizes cookie handling in curl by reducing memory allocations through two key strategies: (1) delaying heap allocation of cookie structs until after validation succeeds, and (2) storing only the normalized/canonical path instead of maintaining both the original and sanitized versions. The optimization results in 7-38% reduction in allocations across various cookie-handling test cases, with the most significant improvements in tests with heavy cookie processing.
Key Changes
- Introduced stack-based cookie struct initialization that's only cloned to heap after all validation passes, eliminating wasted allocations for rejected cookies
- Consolidated the dual
pathandspathfields into a singlepathfield that stores the canonical (normalized) path directly - Modified
sanitize_cookie_path()to work with length parameters and integrated it directly into the parsing flow
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/cookie.h | Removed spath field from Cookie struct; path now stores canonical path only |
| lib/cookie.c | Implemented delayed heap allocation using stack struct; refactored path sanitization to be length-based; added storecookie() helper; updated strstore() signature and freecookie() to support dual-mode operation |
| tests/data/test31 | Updated expected cookie paths to canonical form (trailing slashes removed) |
| tests/data/test46 | Updated expected cookie path from /want/ to /want |
| tests/data/test61 | Updated expected cookie paths to canonical form |
| tests/data/test420 | Updated expected cookie paths and cookie ordering in protocol output |
| tests/data/test444 | Updated 50 cookie entries to use canonical paths without trailing slashes |
| tests/data/test1105 | Updated expected cookie path from "/silly/" to /silly (quotes and trailing slash removed) |
| tests/data/test1561 | Updated expected cookie path for login path |
| tests/data/test1903 | Updated expected cookie paths to canonical form |
| tests/data/test1905 | Updated expected cookie paths to canonical form |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
To avoid wasting time allocating data for incoming cookies that are discarded for one reason or another, delay allocations until after verifications are done.
Instead of keeping both versions around.
bagder
force-pushed
the
bagder/cookie-alloc
branch
from
December 7, 2025 16:25
ed8226b to
e37e3cd
Compare
bagder
added a commit
that referenced
this pull request
Dec 8, 2025
To avoid wasting time allocating data for incoming cookies that are discarded for one reason or another, delay allocations until after verifications are done. Closes #19864
bagder
added a commit
that referenced
this pull request
Dec 8, 2025
Instead of keeping both versions around. Closes #19864
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.
To measure what difference this patch series has, I counted the number of allocations done when running the first eight cookie-using test cases and compared before and after this PR. The allocations counted are for the entire test case.