bench by daniel-noland · Pull Request #1759 · githedgehog/dataplane

daniel-noland · 2026-08-26T02:59:40Z

No description provided.

coderabbitai · 2026-08-26T02:59:48Z

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-08-26T03:12:40Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1653	1	1652	0

View the top 1 failed test(s) by shortest run time

dataplane-routing::router::rio::tests::a_large_answer_arrives_whole

Stack Traces | 122s run time

thread 'router::rio::tests::a_large_answer_arrives_whole' (119398) panicked at ..../src/router/rio.rs:933:18:
rio should answer within the read timeout: Os { code: 11, kind: WouldBlock, message: "Resource temporarily unavailable" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

`Fib::lpm_entry_prefix` walks a route's fib groups twice -- once to count entries, once to turn the hashed index back into a group and an offset -- which looks like a per-packet cost worth removing. It is not. Caching the count regresses the common case. One group's two walks touch the same cache lines, so the second is nearly free, while the extra field grows `FibRoute` from 24 to 32 bytes and costs more in the trie than it saves: 1g x1e goes from 17.1ns to 17.5ns. It only pays from about eight groups up, reaching -16% at 16g x1e and -22% at 16g x4e. Whether that trade is worth making is a question about how wide real ECMP gets, and the answer is not in the code. The proportions are the useful part: of 17.1ns, the trie lookup is 10.0 and pulling the destination out of the packet is 2.3, so entry selection is under a third of a lookup that is itself mostly trie. The assertion inside the loop is load-bearing rather than decorative. The test packet is addressed to 5.6.7.8, so a route installed on 10.0.0.0/8 sends every shape to the default route and returns the same number -- a bench that measures nothing and says so nowhere. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Puts callgrind next to the criterion bench it already has, so a benchmark can count work as well as time it, and calibrates the two against each other while there is a known answer to calibrate against. Taking the caching change from the previous commit and measuring it both ways: callgrind reports -4.3% instructions on a one-group route where the machine is 3.1% slower. It counts the instructions the change removes and cannot see that `FibRoute` grew from 24 to 32 bytes, or that the walk it removed was hitting L1 anyway. Its modelled cache does not catch it either -- a fixed generic L1 that this fixture fits in whichever layout it has. Where the change is algorithmic rather than structural the two agree on sign and land within a small factor. So it is worth gating on and not worth deciding with: instruction counts are bit-identical run to run, and blind to layout. Both halves are in development/code/benchmarking.md, along with the two ways these benchmarks were wrong first -- a fixture that missed the code under test, and a measured function that owned its fixture and so mostly timed the drop. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…they lie Three things worth knowing before somebody needs one of these in a hurry. The family is wired up and exercised, so reaching for DHAT or massif is a two-line edit. Only callgrind scopes to the benchmark function, though: cachegrind has no call graph and counts the whole process, reporting 453,358 instructions against callgrind's 349 for the same benchmark. Cachegrind can be handed the real cache geometry and now is, but it already reads CPUID and gets L1 right unaided; what it gets wrong is collapsing L2 and L3 into one "LL" and guessing 8 MB against this host's 32 MB. That changed nothing here, because the fixture never leaves L1 -- an argument for realistic fixture sizes rather than for tuning the model. DPDK is the one that matters. Valgrind does not fall over on rte_acl: the whole ACL suite runs, 39.5 billion instructions, no crash. It instead reports a CPU it can emulate, so DPDK's runtime dispatch chose scalar and AVX2 and executed no AVX-512 at all -- on a Zen 4 part that has it, and would use avx512x16/x32 in production. That is not a modelling error to correct for; it is a measurement of a different function. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Each bench had its own copy of the fixture builder, the measured body and the list of route shapes. Two benchmarks that drift apart do not fail -- they answer different questions while still looking comparable, which turns the calibration between them into a lie that compiles. `benches/common` now holds all three. The shape list is a macro rather than a `const` because the callgrind harness needs one `#[bench::id(..)]` per shape and attributes cannot be looped over, so one definition expands into an array for criterion and into attributes for callgrind. Sharing the body also fixed an asymmetry that was already there: criterion hoisted the read guard out of its loop and callgrind did not, so the pair had been measuring slightly different regions all along. Both include it now, with `enter_only` to subtract. Callgrind reports "No change" on every counter across the refactor, which is the check that the shared body is the same code. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`just bench-compare` prints a headline, a table of every metric, and a bar chart when something moved past a threshold. The report needs no memory of previous runs, which is the part worth explaining. Storing yesterday's numbers would make the answer depend on which runner recorded them. Callgrind counts work rather than measuring time, so the comparison only requires both sides to run on the same machine -- and they can do so in the same job: base with `--save-baseline`, head with `--baseline`. Each record then carries both values and the delta already, and nothing outlives the run. The full report belongs in `$GITHUB_STEP_SUMMARY` with the headline as a sticky comment linking to it. A gist would need a personal access token with `gist` scope, since `GITHUB_TOKEN` cannot create one -- a credential to rotate in exchange for an artifact that is neither permalinked to the run nor able to render mermaid. The workflow itself is described rather than written: it cannot be exercised from here, and an untested workflow that comments on every pull request is worse than a documented one somebody runs once by hand. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Running the report against a real change -- the FibRoute caching experiment -- turned up two things a synthetic test could not. The headline was picking whichever number moved most, which meant `EstimatedCycles` beat `Ir` by four hundredths of a percent and put the least trustworthy metric at the top. It is a formula over the counters, not a time, and it moves alongside `Ir` without adding anything. More useful: DHAT sees what callgrind cannot. That change grows `FibRoute` from 24 bytes to 32, which is invisible to an instruction count and is the whole reason it loses on a real machine -- but allocation totals move, 7,226 bytes to 7,354. So the headline now names both, and "less work, more memory" is called out in the guide as the shape of change most likely to read as a win here and lose in production. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Follows `ci` and `miri`: `just bench criterion`, `just bench callgrind`, `just bench compare`, `just bench baseline`. A module cannot share a name with a recipe, so the criterion one moved too and now invokes `just build benches` rather than depending on it. Which turned up a bug the flat layout had hidden. The nix `benches` build produces `fib_lookup_callgrind` alongside the criterion binaries, and the loop ran everything it found -- so `just bench` was respawning the callgrind target under valgrind and overwriting whatever baseline `compare` had stored. The loop now skips `*_callgrind`, which is what the naming convention was for. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Criterion's html report needs `html_reports` and a `plotters` backend, neither of which survived `default-features = false`. Both members wanted the same set, so it moves to the workspace. `rayon` stays off: it parallelises sampling, trading run-to-run stability for wall-clock we do not need back. Opening the result from `file://` does not work -- the pages fetch their siblings relatively and the origin refuses, so the index renders and everything under it is empty. `static-web-server` joins the dev shell, with a generic `just serve` behind `just bench serve` and `just serve-coverage`, since llvm-cov's report has the same problem. Verifying that turned up something worse than the missing feature. The nix `benches` build inherits the root justfile's `profile`, which is `debug`, so this recipe has always benchmarked unoptimised binaries: the same benchmark reads 20.9ns there against 2.35ns from a release build. Nine times slow, no inlining, and wrong in the way that still looks like a measurement. The module now asks for `release`. `cargo bench` was never affected -- its profile inherits from `release` -- so the numbers reported from it stand. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

They went to the nix build, where nobody wants them, leaving no way to scope a run. A full sweep is over half an hour -- nearly all of it the rte_acl benches walking fifteen rule counts -- so iterating on one benchmark meant waiting for all of them. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

static-web-server defaults to `::`, so `just serve-coverage` was offering the whole source tree to anything that could reach the port. Turned up while checking the html reports serve at all, which they now do end to end: index, per-benchmark pages, and plots. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`iai-callgrind` serializes its results with `bincode` 1.3.3, so adding the callgrind harness made `check-dependencies` fail on RUSTSEC-2025-0141. There is nothing to upgrade to: the advisory records a team that stopped deliberately and considers 1.3.3 complete. The entry states the invariant that makes it tolerable -- dev-dependency only, absent from every shipped artifact -- so a later reviewer can re-check that rather than re-derive it. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

`compare` tested `-d target/iai`, which `just bench callgrind` also creates, and `--baseline=name` exits 0 when `name` is absent rather than failing. Between them the save branch was unreachable once anything had written to `target/iai`: a `compare` against a name that was never recorded reported "recorded a new one" and recorded nothing, so the next run had nothing to compare against either. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

A just module is a closed scope: it does not inherit the root justfile's variables and there is no command-line syntax that sets one. So moving these recipes into `bench.just` and replacing the `(build "benches")` dependency with a `just build benches` subprocess put every build argument out of reach -- `features`, `platform`, `libc`, `kernel`, `default_features`, `instrument`, `jobs`, `cores`. The `just features=dpdk bench criterion` the module's own documentation asked for reached nix as `--argstr features ""`, so the rte_acl benches it names could not be run at all. Back to flat recipes and a real dependency, which shares this invocation's variables. `just bench <sub>` becomes `just bench-<sub>`, as it was before, and the module's genuine fixes stay: skipping `*_callgrind` in the criterion loop, the callgrind target variables, the html report, and `bench-baseline`. The module also carried `profile := "release"`, which a flat recipe cannot have without diverging from the global knob every other recipe reads. `bench` refuses any other profile instead of quietly benchmarking a debug build. Signed-off-by: Daniel Noland <daniel@githedgehog.com>

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 3c941d2 to 6a95b47 Compare August 26, 2026 17:30

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch 2 times, most recently from 640edd0 to 47d1350 Compare August 26, 2026 19:36

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch 2 times, most recently from 1905efe to 68c58e4 Compare August 26, 2026 20:41

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch 4 times, most recently from afe8933 to ab17860 Compare August 26, 2026 21:25

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 5a8079e to b08c42c Compare August 27, 2026 01:29

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from ab17860 to b0a5e97 Compare August 27, 2026 01:29

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from b08c42c to a774e67 Compare August 27, 2026 01:41

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from b0a5e97 to c2c48ab Compare August 27, 2026 01:41

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from a774e67 to 8fcbc10 Compare August 27, 2026 04:34

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from c2c48ab to 61fb4c7 Compare August 27, 2026 04:35

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 8fcbc10 to 0207c9d Compare August 27, 2026 05:12

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 61fb4c7 to b18bba9 Compare August 27, 2026 05:12

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 0207c9d to 9b444b3 Compare August 27, 2026 17:59

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from b18bba9 to 9a66fc6 Compare August 27, 2026 18:00

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 9b444b3 to 7b60ba9 Compare August 27, 2026 18:29

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch 2 times, most recently from 4517822 to bb9404d Compare August 28, 2026 02:13

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from e555d80 to 753a8cf Compare August 28, 2026 02:19

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from bb9404d to 54309b8 Compare August 28, 2026 02:19

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 753a8cf to f62239a Compare August 28, 2026 03:05

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 54309b8 to b721f06 Compare August 28, 2026 03:05

daniel-noland changed the title ~~feat(bench): a callgrind harness, and reports worth reading~~ bench Aug 28, 2026

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from f62239a to ce410c4 Compare August 28, 2026 04:22

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 79c3ec9 to aed41bb Compare August 28, 2026 05:27

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from aaa09da to d522540 Compare August 28, 2026 05:45

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from aed41bb to d3e094f Compare August 28, 2026 05:45

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from d522540 to 711442d Compare August 28, 2026 06:05

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from d3e094f to 3068443 Compare August 28, 2026 06:06

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 711442d to 15ce0ad Compare August 28, 2026 06:40

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 3068443 to 625a6d0 Compare August 28, 2026 06:40

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 15ce0ad to 10bbc35 Compare August 28, 2026 07:07

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 625a6d0 to 77ea1c0 Compare August 28, 2026 07:07

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 10bbc35 to 7300644 Compare August 28, 2026 07:31

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 77ea1c0 to 5f95720 Compare August 28, 2026 07:31

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 7300644 to 508e5f4 Compare August 28, 2026 07:43

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 5f95720 to 5a2aaf7 Compare August 28, 2026 07:43

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 508e5f4 to 37b1a91 Compare August 28, 2026 09:14

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 5a2aaf7 to 2777622 Compare August 28, 2026 09:14

daniel-noland and others added 13 commits August 28, 2026 11:14

daniel-noland force-pushed the pr/daniel-noland/ip-address-refactors branch from 37b1a91 to e1937b5 Compare August 28, 2026 17:15

daniel-noland force-pushed the pr/daniel-noland/bench-harness branch from 2777622 to 3b54931 Compare August 28, 2026 17:16

Navigation Menu

Uh oh!

bench - #1759

bench#1759
daniel-noland wants to merge 13 commits into
pr/daniel-noland/ip-address-refactorsfrom
pr/daniel-noland/bench-harness

daniel-noland commented Aug 26, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 26, 2026

Draft PR not reviewed

Uh oh!

codecov Bot commented Aug 26, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

daniel-noland commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 26, 2026

Draft PR not reviewed

Uh oh!

codecov Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 1 Tests Failed:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

daniel-noland commented Aug 26, 2026 •

edited

Loading

codecov Bot commented Aug 26, 2026 •

edited

Loading