Repository navigation
ci(benchmark): compare base and head on the same runner - #24
Merged
Merged
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>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
html-scripts
Details
| Benchmark suite | Current: ec49b69 | Previous: f6c89c9 | Ratio |
|---|---|---|---|
getScripts 1MB page |
55.4 ms (55-56.7) |
55.6 ms (55.2-56.6) |
1.00 |
getScripts 20KB page |
70.5 ms (69.8-72.4) |
70.8 ms (70.6-71.6) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
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>
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.
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(ci: allow pnpm builds through env instead of a per-install flag #25). The case ci: allow pnpm builds through env instead of a per-install flag #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