Repository navigation
perf(headers): precompute per-browser header order sets - #25
Conversation
sortHeaders built `new Set(order)` from the browser's header order on every call, and getHeader allocated an `[key, value]` pair per header through Object.entries. The order Sets now get built once when the module loads, and getHeader walks Object.keys and returns on the first match. Output is unchanged: same key order and values as master on 80,024 checks (bench fixtures, 20k random header sets with mixed-case and numeric keys, missing or empty user agents, null-prototype and inherited-property objects, null and undefined input). Local A/B, node v24.16.0 arm64, 6 alternating runs per side, median of medians: | case | base | change | delta | | ----------------------- | ------ | ------ | ------ | | sortHeaders chrome | 90.3ms | 50.4ms | -44.2% | | sortHeaders firefox | 79.9ms | 42.9ms | -46.3% | | sortHeaders safari | 66.1ms | 40.6ms | -38.6% | | getBrowser (unchanged) | 155.5ms | 156.4ms | +0.6% | | tls options (unchanged) | 103.9ms | 104.8ms | +0.9% | Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Cached header lookup and sorting src/headers.js |
The module precomputes header orders and membership sets for each browser. getHeader scans header keys by index, and sortHeaders uses the cached order and set, with empty values as the fallback for unrecognized browsers. |
Header behavior tests test/headers.js, test/hook.js |
Tests cover lookup casing and missing values, browser-specific sort order, inherited and duplicate-case headers, getter access, nullish inputs, and hook decoration with a mixed-case User-Agent header. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Refactor
Possibly related PRs
- Kikobeats/https-tls#24: Adds benchmarks for
sortHeadersacross Chrome, Firefox, and Safari. The benchmarks measure the sorting behavior changed here.
Merge Risk: 
c98ee
-
The header optimization appears mergeable after normal checks; no actionable behavior regression was identified.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "perf(headers): precompute per-browser header order sets" matches the main change. The PR builds each browser's header-order keys and Set once at module load in src/headers.js, and the title… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "perf(headers): precompute per-browser header order sets" matches the main change. The PR builds each browser's header-order keys and Set once at module load in src/headers.js, and the title names that change. It also names the performance goal. The PR also changes getHeader to scan keys instead of entries, and adds tests, but those changes are secondary to the main point. The title is specific, short, and follows a conventional commit style.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 3 files.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
https-tls
Details
| Benchmark suite | Current: c98ee70 | Previous: 42f2730 | Ratio |
|---|---|---|---|
sortHeaders chrome |
102.9 ms (102.2-108.2) |
190.2 ms (188.8-210.1) |
0.54 |
sortHeaders firefox |
90.3 ms (89.9-93.4) |
170.4 ms (166.6-172.1) |
0.53 |
sortHeaders safari |
87.1 ms (86.7-88.1) |
141.9 ms (138.0-147.8) |
0.61 |
getBrowser top 100 user agents |
328.7 ms (327.6-334.4) |
324.8 ms (323.7-326.7) |
1.01 |
tls options top 100 user agents |
220.2 ms (218.8-224.0) |
215 ms (211.7-221.6) |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
Coverage Report for CI Build 40589939009Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 98.502%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
ava's deepEqual ignores key order, so the sort tests stayed green when sorting was replaced with a copy. Assert key order, and cover the no-browser fallback, inherited keys, and the hook's header lookup. Co-authored-by: Cursor <cursoragent@cursor.com>
What changed
sortHeaders(src/headers.js) builtnew Set(order)from the browser's header-order list, 31 to 43 entries, on every call.getHeaderwent throughObject.entries, which allocates a[key, value]pair for every header just to finduser-agent. The fix:{ keys, set }pairs get built once, when the module loads.getHeaderwalksObject.keysand returns on the first match.The loops still use
key in headersfor the ordered pass andObject.keysfor the remaining pass, the same as master. Own and inherited properties, and throwing onnull/undefined, behave as before.A/B (local)
node benchmark/index.js, base =origin/masterfromgit archive, 6 runs per side alternating, median of each run's median, node v24.16.0, darwin arm64 (12 cores), load average 5 to 13 (interleaved).Behavior changes
None. The test suite passes unchanged. The benchmark's own order assertions pass. A fuzz comparison against master gave identical key order and values in 80,024 of 80,024 checks. It covered:
nullandundefinedinputHow tested
npm test: standard-markdown, standard, ava (11 passed).node benchmark/index.js: 0 errors on both sides.🤖 Generated with Claude Code
Note
Low Risk
Internal performance refactor in header utilities with expanded regression tests; intended behavior is unchanged aside from documented edge-case coverage.
Overview
Speeds up header sorting and lookup by building each browser’s ordered header list and a matching
Setonce at module load (ORDERS), instead of allocating a newSeton everysortHeaderscall.getHeadernow scansObject.keys(headers)for a case-insensitive match instead of iteratingObject.entries, avoiding per-header pair allocation.Tests add an
assertHeadershelper (key order + values), broaden coverage forgetHeader/sortHeaders(Firefox, unknown browsers, inherited/null-prototype objects, getters, null/undefined), and assert the hook reads a mixed-caseUser-Agentwhen it is not the first header.Reviewed by Cursor Bugbot for commit c98ee70. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit