Repository navigation
ci(benchmark): compare base and head on the same runner - #26
Conversation
The stored baseline came from whatever runner and lts/* node the last master push got, so PR comments flagged untouched cases as 1.7-2.4x slower. Base and head now run alternately in the same job (5 rounds each, median), and the action compares head against that base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Configure and run base/head benchmarks .github/workflows/benchmark.yml |
The workflow runs only for pull requests, checks out the base and head separately, and runs five benchmark rounds. It skips a missing base benchmark, warns and continues when a base run fails, and fails when a head run fails. The previous single-run and cache restore/save flow was removed. |
Aggregate and report benchmark results .github/workflows/benchmark.yml |
The workflow calculates per-case medians and ranges, writes head results, seeds benchmark cache data from base results when available, and adds a base-versus-head table to the workflow summary. The pull-request comment step reads the head output. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related PRs
- Kikobeats/https-tls#24: Adds the benchmark workflow and harness that this pull request updates and uses for base and head benchmarks.
Merge Risk: 
474af
-
Pull requests now compare base and head benchmarks on the same runner. No merge-blocking issue was identified.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: comparing base and head benchmarks on the same runner. 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.
Finishing Touches
-
🧪 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: 88cb6ee | Previous: 37ae603 | Ratio |
|---|---|---|---|
sortHeaders chrome |
79.6 ms (78.8-79.8) |
80.3 ms (79.4-80.8) |
0.99 |
sortHeaders firefox |
69.5 ms (69.3-88.4) |
70.6 ms (69.4-70.8) |
0.98 |
sortHeaders safari |
65.8 ms (65.2-66.4) |
66.6 ms (66.2-67.7) |
0.99 |
getBrowser top 100 user agents |
200.2 ms (199.9-201) |
200.6 ms (199.9-202.4) |
1.00 |
tls options top 100 user agents |
154.8 ms (154.3-155.5) |
154.8 ms (154.3-157.6) |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
Coverage Report for CI Build 40666524021Warning No base build found for commit Coverage: 98.507%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
A failed base install, or a base run that exits without writing its output, now warns and compares head alone instead of failing the job. The summary shows each case's own unit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Why
The benchmark comment compared each PR against a stored baseline from the last master push. That baseline ran on whatever runner image and
lts/*Node version that push got. On send-http#30 the PR ran on runner image 20261012 with Node 24.21.0, and the baseline on image 20261025 with Node 24.22.1. Untouched cases showed as 1.75x to 2.4x slower. An earlier run of unchanged code on the same image as the PR gave the same false slowdowns.What changed
pull_request.base.sha) and head side by side in one job. It installs both, then runs both harnesses alternately for 5 rounds, switching which side goes first each round.github-action-benchmarkcompares head against base from the same runner and posts its usual comment. The same table also goes to the job summary, so fork PRs (read-only token, no comment) still get a result.github-action-benchmarkskips its comment when there is no previous entry (writeBenchmarkin v1.22.2), so in that case the head numbers appear only in the job summary. Each base failure emits a::warning::. A head install or head run that fails still fails the job.peak RSS deltain MB).pushtrigger and theactions/cachebaseline are gone, since nothing reads them now. Permissions drop tocontents: readandpull-requests: write, and both checkouts usepersist-credentials: false.How it was tested
actionlint1.7.12 with shellcheck passes on both variants, npm and pnpm.I ran the workflow's two
run:scripts locally, copied verbatim from the YAML, on debug-logfmt with base277cddfand headcb8f859(perf(headers): precompute per-browser header order sets #25). The case perf(headers): precompute per-browser header order sets #25 targets got faster, and the two untouched cases came out flat:I fed the seeded data file and head output to the action's own
addBenchmarkEntry(v1.22.2 dist). It finds the base entry and gives the same ratios. That rules out the trap where the action skips entries that share the head's commit id.I simulated a base with no
benchmark/(whoops before #59). The job passes, the summary shows head only, the data file is empty, and the action posts no comment.I also simulated a base whose
pnpm install404s and a base harness that exits 0 without writing output. Both warn, finish with exit 0, and report head only.End to end on whoops#61, which changes only the workflow: the bot comment shows ratios of 0.99 to 1.00 on every case. Before, untouched code showed up to 2.4x.
🤖 Generated with Claude Code