ci(nanonbt-bench): bench pull requests against main and comment - #12
Merged
Merged
Conversation
topi-banana
marked this pull request as ready for review
September 27, 2026 07:12
Benchmark results
Every counting platform compares its own base and pull request; a count is exact on the platform that produced it, but counts from different architectures are different instruction sets and are not comparable with each other. Platforms
linux-x86_64Callgrind instruction counts: exact within this platform, so the two sides compare without a threshold; lower is better. 0 improved · 0 regressed · 173 unchanged of 173 entries compared — 29 reference. All 202 entriesparse
skip
write
linux-aarch64Callgrind instruction counts: exact within this platform, so the two sides compare without a threshold; lower is better. 0 improved · 0 regressed · 173 unchanged of 173 entries compared — 29 reference. All 202 entriesparse
skip
write
wasm32-wasip1Wasmi fuel: the wasm instructions each entry runs, exact within this platform, so the two sides compare without a threshold; lower is better. 0 improved · 0 regressed · 115 unchanged of 115 entries compared — 29 reference. All 144 entriesparse
skip
write
|
The `Bench` workflow needs to say what a pull request changed, not just that the bench ran. The new `bench-summary` example walks criterion's output tree and reads each entry's `new` estimates, the saved `main` baseline and the `change` between them into a Markdown summary: the counts, the changes that cleared criterion's noise threshold, and every entry under a collapsed table. An entry counts as improved or regressed by the same rule criterion's own report uses, the 95% confidence interval of the mean change clearing the threshold on one side; the saved files carry no p-value, so the interval alone decides. The tool is an example rather than a `src/bin` target: a bin target trips Cargo's unused-dependencies lint for the optional comparison dependencies, which only the bench targets use.
The bench is numbers; without a baseline they mean nothing across runners. The new workflow benches the base commit with `--save-baseline main` and the pull request with `--baseline-lenient main`, back to back on one runner, so the percentages describe one machine at one moment. The base pass's `new/` numbers are cleared before the second pass, which leaves each side in its own directory and keeps an entry the pull request removed visible to the summary; lenient comparison keeps an entry it added from failing the pass instead of joining the baseline. The summary the `bench-summary` example renders is posted as a sticky comment. A pull request from a fork, or one Dependabot opened, runs with a read-only token, so that step is skipped and its numbers stay in the log. The job sits outside the required `ci` workflow: a shared runner is noisy, and two full passes are not a check to gate on.
The two passes run in different processes, and randomization laid their input buffers out differently, which moves an entry by tens of percent where its walk is sensitive to the placement. The first run showed it: the pull request side of `skip/fastnbt/double-list` measured 55% slower while `skip/fastnbt/int-list`, benchmarked in the same minutes, matched the base pass's iteration count exactly, and the same document moved 0-3% on three other targets. The two revisions differ by a doc comment, so none of it was the code. `setarch -R` disables the randomization: both passes place the same buffers the same way, and the percentages describe the code. Noise from the shared host between the passes remains; the comment keeps its caveat.
One pair of passes could not tell a change in the code from a change in the host. The same comparison was run twice on the same two revisions and one run called 32 entries improved and 43 regressed where the other called 63 and 42; the head binary itself differed by up to 42% between the two runs. Criterion's confidence interval is a within-run one, so no amount of it can rule that out. The tool now reads the `base` and `head` estimates of two jobs — one benching the base first, one benching the pull request first — and calls an entry improved or regressed only when both jobs clear criterion's ±1% noise threshold in the same direction, at the less extreme of the two percentages. A pair that disagrees is called unstable instead, and both jobs' numbers are always shown side by side. The mirrored order is what gives the agreement its force: anything that follows the pass order moves the two jobs' changes in opposite directions, while only a change in the code moves both the same way. The tool no longer reads criterion's `change` directories, only the named baselines, so the workflow no longer needs `--baseline-lenient`.
The single pair of back-to-back passes was still at the mercy of the host: the passes are different processes minutes apart, and the same code moved by tens of percent between runs of the same comparison. Both sides are now benched in each of two parallel jobs, in opposite orders: `forward` benches the base commit first, `backward` the pull request. Every number a job reports is still set against another pass on the same runner; only the comparison crosses runners, and the summary keeps it only where the two agree. Each job passes `--save-baseline base` then `--save-baseline head`, so the passes no longer compare and the tool reads the two named baselines. That drops the step clearing the base pass's `new/` numbers and the lenient comparison. Each job uploads its criterion tree; a third job downloads both, runs the summary and posts the comment, so the `pull-requests: write` permission moves to it alone.
Criterion measures wall time, which moves with the machine and the run. The workflow tried to cancel that with two mirrored passes and a within-run confidence interval; neither sees the host itself, and the same comparison still disagreed with itself by tens of percent. iai-callgrind counts instructions under callgrind instead. The count does not depend on the machine or the run, so one pass per side compares as exact numbers, and the two jobs and their artifacts go away with it. The `compare` suite is now one `#[library_benchmark]` per entry, named `<kind>_<target>_<id>`, with the document bytes and the write setup hoisted out of the measured function; `--cache-sim=no` keeps the instruction count while skipping the cache simulation the report never reads. The workflow benches both revisions in one job: it checks the base commit out with the pull request's bench crate laid over it, runs the suite with `--save-summary=json`, moves the summaries aside, benches the head and hands both trees to the rewritten `bench-summary` example. Its comment pairs each entry by id and leads with improved, regressed and unchanged; `pumpkin` entries are listed for reference only, because their compounds are `std` hash maps whose serialization order follows the process's random seed, which moved their counts by up to nine percent across thirty runs of the same binary. The runner comes from `taiki-e/install-action` at the version the bench's lock file pins and valgrind from apt; both sides run under that same pair, and the runner disables ASLR itself, so the `setarch` wrappers go away too.
topi-banana
force-pushed
the
ci/bench-pr-result-comment
branch
from
September 28, 2026 13:59
8486bdb to
20f5368
Compare
Callgrind only runs where valgrind does, Linux x86_64 and aarch64. The same four target files now also compile into the crate library, against a runtime registry in place of the iai macros, so the two harnesses that cover the other platforms can walk the same entries: the wasmi fuel runner on wasm32 and the smoke runner on Windows and macOS. Both backends name every entry `<kind>_<target>_<id>`, and the shared `report::display_id` turns those names into the report id a comparison pairs by.
Run callgrind on Linux x86_64 and aarch64, wasmi fuel on wasm32, and a smoke run of every entry on Windows and macOS, then render all of them into one sticky comment with a section per platform and exact counts within each. A platform that failed to produce results reads as `did not run` instead of blocking the others, and the tables drop to a compact form if the comment would outgrow GitHub's size limit. RISC-V gets its own dispatch-only workflow, since it has no hosted runner and needs QEMU and valgrind 3.25 or newer.
The smoke platforms run every entry but count nothing, so the intro's "every platform counts its own base and pull request" overstated what Windows and macOS do.
topi-banana
force-pushed
the
ci/bench-pr-result-comment
branch
from
September 29, 2026 08:57
a488dec to
a21b19a
Compare
`--locked` makes cargo fail when the lock does not match the manifests, but the base commit's manifest decides its own version and dependencies, which a pull request may change; a version bump or a dependency change made the base bench, and with it the whole platform, fail before it ran. A stale lock re-resolves only as far as the base library needs and keeps every other pin, the comparison targets above all, and the pull request's checkout restores the lock before the head runs, which stay `--locked`.
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
1 participant
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.
Summary
ci.ymlbuilds and lints the comparison bench but never runs it: criterion reported wall time, and timing noise between jobs on a shared runner would drown the numbers. This pull request adds aBenchworkflow that runs the suite on every pull request and comments its results againstmain.The bench now measures with
iai-callgrind, which counts instructions under callgrind. The count does not depend on the machine or the run, so one pass per side compares as exact numbers: no noise threshold, no repeated passes, no mirrored order. Earlier revisions of this branch still tried to make criterion's wall-clock numbers comparable — two parallel jobs in opposite orders, an agreement rule,setarch -R— and could not: the same comparison, run twice on the same two revisions, called 32 entries improved and 43 regressed in one run where the other called 63 and 42, and the same head binary differed by up to 42% between the two runs. Criterion's confidence interval is a within-run one and cannot see either. Those jobs, their artifacts and thesetarchwrappers are gone.The workflow is one
comparejob. It checks the base commit out with the pull request'scrates/nanonbt-benchlaid over it — the base may not carry the suite yet, and both revisions should be measured by the same entries — benches it, then checks the head out and benches it again. Both passes run under the one valgrind and the oneiai-callgrind-runnerthe job installs, the runner at the version the bench's lock file pins.--save-summary=jsonleaves one summary per entry belowtarget/iai;target/iaiis cleared before the first pass, and the base tree is moved aside before the head pass writes its own. A pull request from a fork, or one Dependabot opened, runs with a read-only token: the comment step is skipped and its numbers stay in the job log.Every entry is now one
#[library_benchmark]named<kind>_<target>_<id>, so the summarizer can rebuild the report id from the function name and the id its#[bench]attribute carries —parse_nanonbt_serde_smallisparse/nanonbt-serde/small. The document bytes and the write setup are hoisted out of the measured function:iai-callgrindevaluatesargsandsetuponce, outside the callgrind toggle, so neither is counted.--cache-sim=noskips the cache simulation the report never reads while keeping the instruction count. A value that borrows from its input goes throughwrite_bench_leaked!, which leaks the bytes so the borrow can outlive setup;empty_group!stands in for a target whose feature is off, somain!always names the same four groups.examples/bench-summary.rsreads the two trees ofsummary.json— the first profile's totalIr, keyed by benchmark id — and renders the comment under thebenchheader. It leads with the counts — improved, regressed and unchanged, plus new and removed when there are any — then the changes themselves, green for fewer instructions and red for more, each with both instruction counts and the percentage, then a collapsed table of every entry grouped by parse, write and skip. Equal work counts equal instructions, so there is no threshold and no unstable bucket. Thepumpkinentries are the exception and are listed for reference only: their compounds arestdhash maps, serialization walks them in the order the process's random seed builds, and that moved their counts by up to nine percent across thirty runs of the same binary.bench.ymlis its own workflow and does not feedci's aggregate; the bench job incikeeps compiling and linting the crate, and the numbers live here.Changes
bench(nanonbt-bench): count instructions with iai-callgrind(20f5368).github/workflows/bench.yml: the singlecomparejob — base with the pull request's bench, then head, both under callgrind — and theiai-callgrind-runnerinstall,--save-summary=json, the summary example and the sticky comment.github/workflows/ci.yml: the bench job's comment now points atbench.ymlcrates/nanonbt-bench/Cargo.toml,Cargo.lock:iai-callgrindreplacescriterion,serde_jsonserves the example, and[profile.bench] debug = truegives callgrind the symbols it matches names bycrates/nanonbt-bench/benches/compare/macros.rs:parse_bench!,write_bench!,write_bench_leaked!andempty_group!crates/nanonbt-bench/benches/compare/main.rs,documents.rs,targets/*: every entry is one#[library_benchmark], with documents and write setup hoisted out of the measurementcrates/nanonbt-bench/examples/bench-summary.rs: reads the two iai-callgrind summary trees, pairs the entries by id and renders the commentTesting
Benchworkflow on this pull request: its head changes no library source, so every comparable entry must count the same instructions; the run reports 0 improved, 0 regressed and 173 unchanged of the 173 entries compared, with the 29pumpkinentries listed for reference onlyciworkflow on the same head, including the bench crate'scargo fmt --all --checkandcargo clippy --all-targets --locked -- -D warningspumpkincaveat: their counts moved by up to nine percent between runs