Repository navigation
headers: avoid O(n^2) rescans in curl_easy_nextheader() - #22898
MegaManSec wants to merge 3 commits into
Conversation
Every call rescanned the entire header list from the head and did a full name comparison against each entry just to recompute amount/ index for the one header being returned, making a sequential enumeration of N headers O(n^2). Cache the previous pick's name/origin/request/amount/index; when the next call continues from exactly that node with the same filters, reuse the cached amount and increment the index instead of rescanning. Any other pattern (restart, filter change, non-sequential prev) falls back to the original full scan, so behavior is unchanged.
curl_easy_nextheader() may get called while headers are still arriving, for example from a callback or in between curl_multi_perform() calls. The cached 'amount' was then reused for a list that had since had more headers appended, so a continued iteration could report a stale amount together with an index at or beyond it. Only use the cache when the httphdrs count is unchanged since it was filled.
|
Analysis of PR #22898 at b1d4e55d: Test 1041 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 12 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (2, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR. Generated by Testclutch |
There was a problem hiding this comment.
🟡 Changes recommended
Iteration over distinct header names remains O(N²), so the primary performance objective is not achieved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds caching to reduce repeated header-list scans during curl_easy_nextheader() iteration.
Changes:
- Stores the previous header’s iteration metadata.
- Reuses metadata for consecutive same-named headers.
- Resets cached pointers when headers are cleared.
File summaries
| File | Description |
|---|---|
lib/urldata.h |
Adds iteration-cache state. |
lib/headers.c |
Implements and resets the cache. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Build amount and index metadata for every matching header together, sorted case-insensitively by name and original list order. Sequential iteration then reuses the cached values for mixed header names instead of rescanning the full list for each result. Fall back to the original scan if the temporary allocation fails.
|
Thanks! |
curl_easy_nextheader() previously rescanned the complete header list for every returned header to calculate amount and index, making a sequential traversal quadratic.
Build the metadata for all headers matching the active origin and request together. A temporary array is sorted case-insensitively by name and original list order, then each stored header receives its amount and index. The cache remains valid until the list or filters change, so mixed-name iteration no longer performs a full scan per result.
If the temporary allocation fails, the function falls back to the original scan. Test 1945 also covers case-insensitive duplicate names.