Repository navigation
ci(benchmark): compare base and head on the same runner - #36
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 |
|---|---|
Workflow scope and checkouts .github/workflows/benchmark.yml |
The workflow runs only for pull requests. It uses read-only contents permission and checks out the pull request head and base separately. |
Base and head benchmark rounds .github/workflows/benchmark.yml |
The workflow runs five alternating base and head rounds. Missing or failed base runs emit a warning and continue; a failed head run fails the workflow. |
Comparison output and benchmark comment .github/workflows/benchmark.yml |
The workflow computes per-benchmark medians, ranges, and round values, then writes merged head results and a comparison table. The benchmark comment action runs only for same-repository pull requests. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Other
Sequence Diagram(s)
sequenceDiagram
participant GitHubActions
participant BaseHeadCheckouts
participant BenchmarkRuns
participant ComparisonStep
participant BenchmarkCommentAction
GitHubActions->>BaseHeadCheckouts: Check out pull request base and head
BaseHeadCheckouts->>BenchmarkRuns: Provide base and head worktrees
BenchmarkRuns->>ComparisonStep: Provide results from alternating rounds
ComparisonStep->>BenchmarkCommentAction: Provide merged head results and benchmark data
Possibly related PRs
- microlinkhq/tinyspawn#34: Adds the benchmark workflow that this change rewrites, including the earlier cache-based baseline flow.
Merge Risk: 
Minimal · up to 3f33d
-
The workflow is ready to merge after normal checks. No actionable benchmark comparison or comment-permission risk remains established.
Pre-merge checks: 
4
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.
There was a problem hiding this comment.
tinyspawn
Details
| Benchmark suite | Current: 650a7b2 | Previous: c36bead | Ratio |
|---|---|---|---|
spawn node -e 0 |
283.3 ms (281.4-298.9) |
281.9 ms (272.6-290.2) |
1.00 |
spawn 50MB stdout |
73.1 ms (71.3-74.3) |
71.4 ms (70.2-73) |
1.02 |
read 50MB stdout x30 |
0 ms (0-0) |
0 ms (0-0) |
1 |
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>
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(build(deps): bump actions/setup-node from 5 to 6 #25). The case build(deps): bump actions/setup-node from 5 to 6 #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