(07) bench

.github/workflows/dev.yml

@@ -537,6 +537,10 @@ jobs:
         profile: "${{ fromJSON(needs.plan.outputs.profiles) }}"
         exclude:
           - profile: release
+          # TEMPORARY: `coverage/fuzz` exhausts the runner's memory. Disabled
+          # until that is diagnosed; a ticket tracks putting it back. Delete
+          # this one line to restore it.
+          - profile: fuzz
     steps:
       - *checkout
       - *nix-setup
@@ -652,7 +656,7 @@ jobs:
         env:
           # Miri bypasses `nix build`, so cap it through its own module.
           JUST_VARS: >-
-            miri::cores=8
+            miri::cores=10
             miri::cpu=${{ matrix.cpu }}
         with:
           recipe: "miri::test"
@@ -668,7 +672,7 @@ jobs:
         uses: *just
         env:
           JUST_VARS: >-
-            miri::cores=8
+            miri::cores=10
             miri::seeds=8
             miri::cpu=${{ matrix.cpu }}
             miri::provenance=permissive
@@ -679,7 +683,7 @@ jobs:
         uses: *just
         env:
           JUST_VARS: >-
-            miri::cores=8
+            miri::cores=10
             miri::cpu=${{ matrix.cpu }}
             miri::provenance=strict
         with:

Cargo.toml

@@ -133,7 +133,12 @@ chrono = { version = "0.4.45", default-features = false, features = [] }
 clap = { version = "4.6.6", default-features = true, features = [] }
 color-eyre = { version = "0.6.5", default-features = false, features = [] }
 colored = { version = "3.1.1", default-features = false, features = [] }
-criterion = { version = "0.8.2", default-features = false, features = [] }
+criterion = { version = "0.8.2", default-features = false, features = [
+  "cargo_bench_support",
+  "html_reports",
+  "plotters",
+] }
+iai-callgrind = { version = "0.16.1" }
 crossbeam-utils = { version = "0.8.22", default-features = false, features = [] }
 dashmap = { version = "6.2.1", default-features = false, features = [] }
 derive_builder = { version = "0.20.2", default-features = false, features = [] }

acl/Cargo.toml

@@ -27,7 +27,7 @@ clock = { workspace = true, features = ["virtual"] }
 # differential-test against it, so make it available whenever test/bench targets are built.
 dataplane-acl = { path = ".", features = ["reference"] }
 bolero = { workspace = true, features = ["std"] }
-criterion = { workspace = true, features = ["cargo_bench_support"] }
+criterion = { workspace = true }
 dpdk = { workspace = true, features = ["test"] }
 match-action = { workspace = true, features = ["derive", "bolero"] }
 net = { workspace = true, features = ["test_buffer", "builder"] }

default.nix

@@ -170,6 +170,7 @@ let
       duvet
       gateway-crd
       gettext
+      iai-callgrind-runner
       jq
       just
       kopium
@@ -188,6 +189,8 @@ let
       rust-toolchain
       shellcheck
       skopeo
+      static-web-server
+      valgrind
       wasmtime
       wget
       yq

deny.toml

@@ -25,6 +25,12 @@ ignore = [
     "RUSTSEC-2024-0436",
     # proc-macro-error2 is unmaintained but is needed by multi_index_map_derive, and our own fixin; ignore until both dependencies have migrated away from it.
     "RUSTSEC-2026-0173",
+    # bincode 1.3.3 is unmaintained -- the team stopped deliberately and considers it
+    # complete, so there is nothing to upgrade to. It reaches us only through
+    # `iai-callgrind`, which is a dev-dependency of `routing` alone and is absent from every
+    # shipped artifact. That is the invariant to re-check rather than re-derive: it stops
+    # holding the moment `iai-callgrind` appears in a normal `[dependencies]` table.
+    "RUSTSEC-2025-0141",
 ]
 
 [licenses]

development/code/README.md

@@ -9,7 +9,8 @@
 - Enforce invariants at compile time when the language or framework supports it.
 - Require runtime validation and for invariants which cannot be compile-time enforced
 - Performance is important, but it is less important than correctness; it does not matter how quickly you can do the
-  wrong thing.
+  wrong thing. When it does matter, measure it: see the [benchmarking note][benchmarking] for the
+  two harnesses, and for how far apart their answers can be.
 
 If you need to write a test, prefer [property-based tests] over simple unit tests.
 To find out whether they are saying what the specification asked for, see the
@@ -23,6 +24,7 @@ If you need to [handle an error][error], follow the guidelines.
 
 [avoid-global-reasoning]: ./avoid-global-reasoning.md
 [property-based tests]: ./property-testing.md
+[benchmarking]: ./benchmarking.md
 [duvet]: ./spec-compliance.md
 [error]: ./error-handling.md
 

development/code/benchmarking.md

@@ -0,0 +1,119 @@
+# Benchmarking
+
+Two harnesses, measuring different things. Both live under `routing/benches/` as a worked pair;
+`fib_lookup.rs` (criterion) and `fib_lookup_callgrind.rs` (iai-callgrind) run the same fixtures
+against the same code so their answers can be compared.
+
+```sh
+just bench            # criterion: wall-clock on this machine
+just bench-callgrind  # iai-callgrind: instructions and modelled cache traffic
+```
+
+`valgrind` and `iai-callgrind-runner` come from the dev shell. The runner's version must equal the
+`iai-callgrind` version in the workspace `Cargo.toml`, and a mismatch fails loudly rather than
+quietly -- so `nix/overlays/dataplane-dev.nix` reads the version out of `Cargo.toml` rather than
+restating it. Nothing in CI runs a bench, so a drifted pin would otherwise surface months later on
+a developer's machine looking like a broken dev shell.
+
+## Which one answers which question
+
+**Criterion measures time on this machine.** That is the thing we actually care about, and it is
+the only one of the two that can see the machine: out-of-order execution, the real cache
+hierarchy, prefetchers, frequency scaling. It is also noisy, needs a quiet machine, and cannot be
+gated in CI without a dedicated runner.
+
+**Callgrind counts work on a synthetic machine.** It executes the program and counts instructions
+retired and hits and misses against a modelled cache. It is bit-for-bit repeatable -- two runs of
+unchanged code report "No change" on every counter -- which makes it the one that can gate CI.
+
+The trap is reading the second as a cheap substitute for the first. It is not, and the size of the
+gap is worth knowing rather than guessing.
+
+## How far apart they are, measured
+
+Taking a real change -- caching a route's entry count in `FibRoute` so the fib lookup walks its
+groups once instead of twice -- and measuring it both ways:
+
+| route shape | callgrind (Ir) | criterion (wall clock) |
+| ----------- | -------------- | ---------------------- |
+| 1 group, 1 entry | −4.3% | **+3.1%** |
+| 1 group, 4 entries | −4.2% | **+1.7%** |
+| 4 groups, 1 entry | −5.5% | −1.3% |
+| 4 groups, 4 entries | −5.8% | ~0% |
+| 8 groups, 1 entry | −7.7% | −6.7% |
+| 16 groups, 1 entry | −11.9% | −16.1% |
+| 16 groups, 4 entries | −11.1% | −21.8% |
+
+Read the top two rows. Callgrind reports an improvement; the machine is slower. The change removes
+instructions, which is all callgrind can see, but it also grows `FibRoute` from 24 to 32 bytes --
+costing more in the trie that holds the routes than the removed walk saves -- and the walk it
+removed was hitting L1 anyway, because with one group both traversals touch the same cache lines.
+None of that is visible to an instruction count, and the modelled cache does not catch it either:
+its L1 is a fixed generic configuration, and this fixture fits in it whichever layout is used.
+
+Lower down, where the change is algorithmic rather than structural, the two agree on sign and land
+within a small factor on magnitude.
+
+So, as a rule of thumb from this one data point: **callgrind is trustworthy about changes in how
+much work a path does, and unreliable about changes in how data is laid out.** A change that moves
+instructions is one it can score. A change that moves bytes is one it cannot.
+
+What it does get right, on the same measurements, is _ordering_. Across seven fixture shapes the
+two harnesses ranked every shape identically, including a non-monotonic pair that both reproduced
+for the same underlying reason. Relative comparisons between variants of the same code survive the
+model better than absolute claims do.
+
+## What callgrind is not
+
+- It is not this CPU, and not the target CPU. It does not model Genoa or Turin, it does not model
+  out-of-order execution, and there is no AVX-512. Vectorised code is where it will mislead most.
+- "Estimated Cycles" is a formula over the counters, not a cycle count. It did not rescue the case
+  above, and it should not be read as a time.
+- Its cache model is a simplification of any real hierarchy, so cache-driven effects are exactly
+  the ones to distrust.
+
+## The other valgrind tools, and where they lie
+
+The tool family is wired up in `fib_lookup_callgrind.rs`, so reaching for DHAT or massif is a
+two-line edit. Three things to know before doing that in a hurry.
+
+**Only callgrind scopes to the benchmark function.** Cachegrind has no call graph and counts the
+whole process: 453,358 instructions against callgrind's 349 for the same benchmark. Comparing the
+two tools' numbers without knowing that leads somewhere wildly wrong.
+
+**Cachegrind's cache geometry is a guess unless you hand it one.** It reads CPUID and gets L1
+right unaided; what it gets wrong is collapsing L2 and L3 into a single "LL" and guessing 8 MB
+against this host's 32 MB. It is given the real geometry here, which changed nothing, because the
+fixture never leaves L1 -- an argument for realistic fixture sizes rather than for tuning the
+model. The geometry in the file is this machine's, so it is a fixed reference point for
+comparability, not a description of wherever you are running.
+
+**Valgrind changes what DPDK executes.** This is the one that matters. It does not fall over on
+`rte_acl` -- the whole ACL suite runs, 39.5 billion instructions, no crash -- which is exactly
+what makes it dangerous. It 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`/`avx512x32` in production. That is not a modelling error to correct for; it is a
+measurement of a different function. Pointing callgrind at the `acl` benches is the obvious next
+step from here, and it is the step that produces plausible numbers for a CPU nobody is shipping
+to.
+
+## Writing a benchmark that measures what you think
+
+Both benchmarks in this repository were wrong the first time, in ways that produced plausible
+numbers rather than obvious failures. That is the normal case, so check for it.
+
+**Make the fixture prove it exercises the code under test.** The first version of `fib_lookup.rs`
+installed its route on `10.0.0.0/8` and sent a packet addressed to `5.6.7.8`. Every shape fell
+through to the fib's default route and returned the same number, and the giveaway was only that
+the results were suspiciously flat. Both benchmarks now assert that the lookup lands on the prefix
+the fixture installed.
+
+**Under callgrind, watch what the measured function owns.** Everything the benchmark function does
+is counted, including dropping its arguments. Taking a fixture by value put several thousand
+instructions of teardown inside the measured region -- swamping a lookup that should be a few
+hundred, and scaling with the fixture, so the result still looked like a measurement of the lookup.
+The fixture is leaked in `setup` so the drop is a no-op.
+
+**Sanity-check the magnitude against a hand estimate.** A lookup that takes 17 ns cannot be 9,000
+instructions. Knowing roughly what a number should be is what turns a wrong benchmark into an
+obviously wrong benchmark.