diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 966f05ee8c..d9ba6dc0bc 100644 --- a/.github/workflows/dev.yml +++ b/.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: diff --git a/Cargo.lock b/Cargo.lock index b8db85df5c..47b54c27c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -398,6 +398,15 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + [[package]] name = "bindgen" version = "0.72.1" @@ -1003,6 +1012,7 @@ dependencies = [ "num-traits", "oorandom", "page_size", + "plotters", "regex", "serde", "serde_json", @@ -1789,6 +1799,7 @@ dependencies = [ "bolero", "bytes", "chrono", + "criterion", "dataplane-args", "dataplane-cli", "dataplane-clock", @@ -1800,10 +1811,12 @@ dependencies = [ "dataplane-lifecycle", "dataplane-lpm", "dataplane-net", + "dataplane-routing", "dataplane-tracectl", "derive_builder", "dplane-rpc", "futures-util", + "iai-callgrind", "inotify", "ipnet", "left-right 0.11.7", @@ -2882,6 +2895,42 @@ dependencies = [ "tower-service", ] +[[package]] +name = "iai-callgrind" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b1e4910d3a9137442723dfb772c32dc10674c4181ca078d2fd227cd5dce9db0" +dependencies = [ + "bincode", + "derive_more", + "iai-callgrind-macros", + "iai-callgrind-runner", +] + +[[package]] +name = "iai-callgrind-macros" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4d03775318d3f9f01b39ac6612b01464006dc397a654a89dd57df2fd34fb68c3" +dependencies = [ + "derive_more", + "proc-macro-error2", + "proc-macro2", + "quote", + "serde", + "serde_json", + "syn 2.0.119", +] + +[[package]] +name = "iai-callgrind-runner" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b74c9743c00c3bca4aaffc69c87cae56837796cd362438daf354a3f785788c68" +dependencies = [ + "serde", +] + [[package]] name = "iana-time-zone" version = "0.1.65" @@ -4366,6 +4415,34 @@ dependencies = [ "time", ] +[[package]] +name = "plotters" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5aeb6f403d7a4911efb1e33402027fc44f29b5bf6def3effcc22d7bb75f2b747" +dependencies = [ + "num-traits", + "plotters-backend", + "plotters-svg", + "wasm-bindgen", + "web-sys", +] + +[[package]] +name = "plotters-backend" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df42e13c12958a16b3f7f4386b9ab1f3e7933914ecea48da7139435263a4172a" + +[[package]] +name = "plotters-svg" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51bae2ac328883f7acdfea3d66a7c35751187f870bc81f94563733a154d7a670" +dependencies = [ + "plotters-backend", +] + [[package]] name = "portable-atomic" version = "1.15.0" diff --git a/Cargo.toml b/Cargo.toml index fb7253843d..9db3b920c5 100644 --- a/Cargo.toml +++ b/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 = [] } diff --git a/acl/Cargo.toml b/acl/Cargo.toml index f9d01cb8a9..b4e9ad0f69 100644 --- a/acl/Cargo.toml +++ b/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"] } diff --git a/default.nix b/default.nix index 4f8964ec67..c2a06e9055 100644 --- a/default.nix +++ b/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 diff --git a/deny.toml b/deny.toml index cc4c4356e6..9d2f706e84 100644 --- a/deny.toml +++ b/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] diff --git a/development/code/README.md b/development/code/README.md index 3e1ad73413..b3723e75f8 100644 --- a/development/code/README.md +++ b/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 diff --git a/development/code/benchmarking.md b/development/code/benchmarking.md new file mode 100644 index 0000000000..f286a3ffb9 --- /dev/null +++ b/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. diff --git a/justfile b/justfile index c0cbb2f3dd..5dd7a02f82 100644 --- a/justfile +++ b/justfile @@ -41,6 +41,9 @@ kernel := if platform == "wasm32-wasip1" { "wasip1" } else { "linux" } # cargo build profile (debug/release/fuzz) profile := "debug" +export callgrind_package := "dataplane-routing" +export callgrind_bench := "fib_lookup_callgrind" + # sanitizer to use (address/thread/safe-stack/cfi/"") sanitize := "" @@ -199,14 +202,78 @@ fuzz target time="60s" *args="": {{ if sanitize == "thread" { "--build-std" } else { "" } }} \ {{ _cargo_feature_flags }} {{ args }} -# Build and run the criterion benches. The rte_acl benches are gated behind the -# `dpdk` feature, so run `just features=dpdk bench` to exercise them; a plain -# `just bench` builds them as empty `main()` and only runs the reference benches. +[private] +[script] +_bench-release-only: + {{ _just_debuggable_ }} + if [ '{{ profile }}' != "release" ]; then + echo "error: benchmarks want profile=release, not '{{ profile }}'" >&2 + echo " run: just profile=release bench" >&2 + exit 1 + fi + +[doc("Wall-clock time, via criterion")] [script] -bench: (build "benches") +bench *args: _bench-release-only (build "benches") {{ _just_debuggable_ }} shopt -s nullglob - for bench in ./results/benches/bin/*; do "$bench" --bench; done + for bench in ./results/benches/bin/*; do + case "${bench}" in + *_callgrind) continue ;; + esac + "${bench}" --bench {{ args }} + done + if [ -f target/criterion/report/index.html ]; then + echo + echo "html report: target/criterion/report/index.html" + fi + +[doc("Instructions and cache traffic, via iai-callgrind")] +[script] +bench-callgrind *args: + {{ _just_debuggable_ }} + cargo bench -p "${callgrind_package}" --bench "${callgrind_bench}" {{ args }} + +[doc("Compare against a baseline and print a markdown report")] +[script] +bench-compare baseline="base" *args: + {{ _just_debuggable_ }} + mkdir -p results/bench + # Decide save-or-compare *before* running, and scope the question to this suite's own + # output directory. Two things went wrong when this was one condition. A file anywhere + # under `target/iai` -- another package, another benchmark -- answered "a baseline + # exists", and the run then found none for any benchmark here, succeeded, and reported a + # new baseline it had not written; every repeat did the same. And with the comparison run + # inside the `if`, any non-zero exit (a compile error, a panicking benchmark, a runner + # version mismatch) fell through to the `else` and overwrote the baseline being compared + # against -- with stderr discarded, so nothing said why. + # iai-callgrind writes to `target/iai[/]///`, + # so match on the two components that identify this suite and stay agnostic about the + # optional target triple above them and the group nesting below. + scope="*/${callgrind_package}/${callgrind_bench}/*" + if find target/iai -type f -path "${scope}" -name '*base@{{ baseline }}*' -print -quit 2>/dev/null | grep -q .; then + cargo bench -p "${callgrind_package}" --bench "${callgrind_bench}" -- \ + --baseline='{{ baseline }}' --output-format=json > results/bench/run.jsonl + else + echo "no baseline '{{ baseline }}' for ${callgrind_package}/${callgrind_bench}; recording one" >&2 + cargo bench -p "${callgrind_package}" --bench "${callgrind_bench}" -- \ + --save-baseline='{{ baseline }}' --output-format=json > results/bench/run.jsonl + fi + ./scripts/bench-report.ts results/bench/run.jsonl {{ args }} + +[doc("Record a baseline for `bench-compare`, without reporting")] +[script] +bench-baseline name="base": + {{ _just_debuggable_ }} + cargo bench -p "${callgrind_package}" --bench "${callgrind_bench}" -- \ + --save-baseline='{{ name }}' > /dev/null + echo "recorded baseline '{{ name }}'" + +[doc("Serve the criterion html report over http")] +[script] +bench-serve port="8080": + {{ _just_debuggable_ }} + just serve ./target/criterion '{{ port }}' report/index.html [script] build-each *args: (build "workspace" args) @@ -754,6 +821,31 @@ coverage *args: cargo llvm-cov report --branch --lcov --output-path="${out}/lcov.info" cargo llvm-cov report --branch --codecov --output-path="${out}/codecov.json" cargo llvm-cov report --branch --summary-only + echo + echo "html report: ${out}/html/index.html (\`just serve-coverage\` to browse it)" + +serve_host := "127.0.0.1" + +[doc("Serve a directory of generated html over http")] +[script] +serve dir port="8080" index="index.html": + {{ _just_debuggable_ }} + if [ ! -d '{{ dir }}' ]; then + echo "error: no such directory: {{ dir }}" >&2 + exit 1 + fi + server="$(command -v static-web-server || true)" + if [ -z "${server}" ] && [ -x ./devroot/bin/static-web-server ]; then + server="$(pwd)/devroot/bin/static-web-server" + fi + if [ -z "${server}" ]; then + echo "error: static-web-server not found; re-enter the dev shell, or \`just setup-roots\`" >&2 + exit 1 + fi + echo "serving {{ dir }} at http://{{ serve_host }}:{{ port }}/{{ index }} (ctrl-c to stop)" + "${server}" --root '{{ dir }}' --host '{{ serve_host }}' --port '{{ port }}' --log-level warn + +serve-coverage port="8080": (serve "./target/nextest/coverage/html" port) [script] duvet *args: diff --git a/k8s-intf/src/bolero/support.rs b/k8s-intf/src/bolero/support.rs index 1d7c7fb96a..c8a6839936 100644 --- a/k8s-intf/src/bolero/support.rs +++ b/k8s-intf/src/bolero/support.rs @@ -184,6 +184,7 @@ mod test { const UNIQUE_COUNTS: [u16; 4] = [0, 1, 10, 16]; #[test] + #[cfg_attr(miri, ignore = "the uniqueness sweep is too slow under miri")] fn test_unique_v4_interface_address_generator() { for count in UNIQUE_COUNTS { let generator = crate::bolero::support::UniqueV4InterfaceAddressGenerator::new(count); @@ -219,6 +220,7 @@ mod test { } #[test] + #[cfg_attr(miri, ignore = "the uniqueness sweep is 124s under miri")] fn test_unique_v6_interface_address_generator() { for count in UNIQUE_COUNTS { let generator = crate::bolero::support::UniqueV6InterfaceAddressGenerator::new(count); diff --git a/miri.just b/miri.just index f58c04babc..28f106688f 100644 --- a/miri.just +++ b/miri.just @@ -27,7 +27,7 @@ export schedule_seed := choose('5', "0123456789") export seeds := "1" export stacked_borrow_check := env("STACKED_BORROW_CHECK", "disabled") -export bolero_test_time_ms := env("BOLERO_TEST_TIME_MS", "30000") +export bolero_test_time_ms := env("BOLERO_TEST_TIME_MS", "10000") export preemption_rate := "0.10" export weak_failure_rate := "0.05" export randomize_struct_layout := "enabled" diff --git a/nat/src/masquerade/apalloc/pool_fuzz.rs b/nat/src/masquerade/apalloc/pool_fuzz.rs index 3e482d8763..d5dda07b08 100644 --- a/nat/src/masquerade/apalloc/pool_fuzz.rs +++ b/nat/src/masquerade/apalloc/pool_fuzz.rs @@ -268,7 +268,7 @@ fn re_reservation_after_a_config_change_is_honoured() { //# REQ-3: A NAT MUST NOT have a "Port assignment" behavior of "Port //# overloading". #[test] -#[cfg_attr(miri, ignore = "exhaustive allocator walk is too slow under miri")] +#[cfg_attr(emulated, ignore = "exhaustive allocator walk is too slow to emulate")] fn a_region_can_be_allocated_dry() { const PORTS_PER_ADDRESS: usize = 65536 - 1024; const ADDRESSES: usize = 2; diff --git a/nat/src/masquerade/apalloc/test_alloc.rs b/nat/src/masquerade/apalloc/test_alloc.rs index 32153ede88..8ce0960189 100644 --- a/nat/src/masquerade/apalloc/test_alloc.rs +++ b/nat/src/masquerade/apalloc/test_alloc.rs @@ -1029,6 +1029,7 @@ mod std_tests { } #[test] + #[cfg_attr(miri, ignore = "a subnet-sized v6 pool is too slow under miri")] fn a_v6_subnet_sized_pool_can_be_printed() { let base = u128::from_be_bytes( "2001:db8::" diff --git a/nat/src/masquerade/expiry.rs b/nat/src/masquerade/expiry.rs index 0ce5ca3aee..8a31f7513c 100644 --- a/nat/src/masquerade/expiry.rs +++ b/nat/src/masquerade/expiry.rs @@ -144,7 +144,7 @@ fn reply_to( //# (Section 5) Behavior at any point in time, or under any particular //# conditions. #[test] -#[cfg_attr(miri, ignore = "the 65k-session pool walk is too slow under miri")] +#[cfg_attr(emulated, ignore = "the 65k-session pool walk is too slow to emulate")] fn pool_exhaustion_splits_a_host_across_public_addresses() { use std::collections::{BTreeMap, BTreeSet}; with_paused_clock(|| async { diff --git a/nat/src/masquerade/fuzz.rs b/nat/src/masquerade/fuzz.rs index a9fc4896f9..cb9041f765 100644 --- a/nat/src/masquerade/fuzz.rs +++ b/nat/src/masquerade/fuzz.rs @@ -147,6 +147,7 @@ impl Tally { } #[test] +#[cfg_attr(miri, ignore = "one configuration is ~15 min under miri")] fn a_masqueraded_flow_comes_back() { let tally = Tally::default(); @@ -198,6 +199,7 @@ fn a_masqueraded_flow_comes_back() { } #[test] +#[cfg_attr(miri, ignore = "one configuration is ~15 min under miri")] fn a_flow_keeps_its_translation() { let tally = Tally::default(); @@ -247,6 +249,7 @@ fn out_unchanged(out: &[Packet], before: (IpAddr, u16)) -> bool { //# REQ-2: It is RECOMMENDED that a NAT have an "IP address pooling" //# behavior of "Paired". #[test] +#[cfg_attr(miri, ignore = "one configuration is ~15 min under miri")] fn an_internal_endpoint_keeps_one_public_address() { let tally = Tally::default(); @@ -316,6 +319,7 @@ fn an_internal_endpoint_keeps_one_public_address() { //# REQ-3: A NAT MUST NOT have a "Port assignment" behavior of "Port //# overloading". #[test] +#[cfg_attr(miri, ignore = "one configuration is ~15 min under miri")] fn distinct_flows_do_not_share_a_translation() { let tally = Tally::default(); @@ -358,6 +362,7 @@ fn distinct_flows_do_not_share_a_translation() { } #[test] +#[cfg_attr(miri, ignore = "one configuration is ~15 min under miri")] fn a_translation_stays_inside_the_public_range() { let tally = Tally::default(); @@ -401,6 +406,7 @@ fn a_translation_stays_inside_the_public_range() { } #[test] +#[cfg_attr(miri, ignore = "one configuration is ~15 min under miri")] fn nothing_is_masqueraded_without_permission() { let tally = Tally::default(); @@ -448,6 +454,7 @@ fn nothing_is_masqueraded_without_permission() { } #[test] +#[cfg_attr(miri, ignore = "one configuration is ~15 min under miri")] fn a_flow_that_cannot_be_masqueraded_says_so() { let tally = Tally::default(); diff --git a/nat/src/masquerade/state_machine.rs b/nat/src/masquerade/state_machine.rs index 882a86101e..d819f0fddd 100644 --- a/nat/src/masquerade/state_machine.rs +++ b/nat/src/masquerade/state_machine.rs @@ -96,6 +96,7 @@ fn expected_tcp(action: NatAction, status: NatFlowStatus, f: Flags) -> NatFlowSt } #[test] +#[cfg_attr(miri, ignore = "the full close sequence is 69s under miri")] fn the_tcp_state_machine_follows_the_close_sequence() { for action in [NatAction::SrcNat, NatAction::DstNat] { for status in STATUSES { diff --git a/nat/src/portfw/fuzz.rs b/nat/src/portfw/fuzz.rs index 89c6557718..d6b7d03cb8 100644 --- a/nat/src/portfw/fuzz.rs +++ b/nat/src/portfw/fuzz.rs @@ -222,6 +222,7 @@ fn a_forwarded_packet_lands_inside_the_published_target() { } #[test] +#[cfg_attr(miri, ignore = "one configuration is minutes under miri")] fn distinct_published_tuples_reach_distinct_targets() { let tally = Tally::default(); diff --git a/nat/src/static_nat/fuzz.rs b/nat/src/static_nat/fuzz.rs index 06a8b5ef21..5b2f92b9c7 100644 --- a/nat/src/static_nat/fuzz.rs +++ b/nat/src/static_nat/fuzz.rs @@ -251,6 +251,7 @@ macro_rules! drive_injectivity { } #[test] +#[cfg_attr(miri, ignore = "one configuration is 52s under miri")] fn distinct_sources_stay_distinct() { drive_injectivity!(Scenario::addresses(false)); } @@ -322,6 +323,7 @@ fn translation_touches_only_the_source() { } #[test] +#[cfg_attr(miri, ignore = "one configuration is 41s under miri")] fn port_translation_touches_only_the_source() { drive_frame!(Scenario::ports(false)); } diff --git a/nat/src/static_nat/setup/range_builder.rs b/nat/src/static_nat/setup/range_builder.rs index 9fdd32dd03..d8d4d38a9b 100644 --- a/nat/src/static_nat/setup/range_builder.rs +++ b/nat/src/static_nat/setup/range_builder.rs @@ -1270,6 +1270,7 @@ mod bolero_tests { } #[test] + #[cfg_attr(miri, ignore = "the range builder sweep is 59s under miri")] fn test_bolero() { let generator = PrefixListsGenerator {}; bolero::check!().with_generator(generator).for_each( diff --git a/nix/overlays/dataplane-dev.nix b/nix/overlays/dataplane-dev.nix index 16e84046d8..2edaed6b49 100644 --- a/nix/overlays/dataplane-dev.nix +++ b/nix/overlays/dataplane-dev.nix @@ -25,6 +25,16 @@ in opengrep = final.callPackage ../pkgs/opengrep { src = sources.opengrep; }; + # The runner binary and the `iai-callgrind` crate must be the same version: a mismatch is a + # hard error at bench time, not a degraded run, and no CI job runs a bench -- so it would + # surface months later on a developer's machine as what looks like a broken dev shell. Read + # the version rather than restate it, so `cargo upgrade` and dependabot move both halves. + iai-callgrind-runner = final.callPackage ../pkgs/iai-callgrind-runner { + inherit (override-packages) rustPlatform; + version = + (builtins.fromTOML (builtins.readFile ../../Cargo.toml)) + .workspace.dependencies.iai-callgrind.version; + }; cargo-bolero = prev.cargo-bolero.override { inherit (override-packages) rustPlatform; }; cargo-deny = prev.cargo-deny.override { inherit (override-packages) rustPlatform; }; cargo-edit = prev.cargo-edit.override { inherit (override-packages) rustPlatform; }; diff --git a/nix/pkgs/iai-callgrind-runner/default.nix b/nix/pkgs/iai-callgrind-runner/default.nix new file mode 100644 index 0000000000..7033648528 --- /dev/null +++ b/nix/pkgs/iai-callgrind-runner/default.nix @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright Open Network Fabric Authors +{ + fetchCrate, + rustPlatform, + version, + ... +}: +rustPlatform.buildRustPackage (final: { + pname = "iai-callgrind-runner"; + inherit version; + src = fetchCrate { + inherit (final) pname version; + hash = "sha256-wJTwaqAz8GWCJ/l9GRXYBVBkpPYrWxN4VQ7GdRFXmzM="; + }; + cargoHash = "sha256-4N7P23bCeeJee/Cm3sSORByh+HzflOENqYqpu629mpA="; + doCheck = false; +}) diff --git a/routing/Cargo.toml b/routing/Cargo.toml index 90e72b2c48..d7c2376794 100644 --- a/routing/Cargo.toml +++ b/routing/Cargo.toml @@ -56,11 +56,22 @@ procfs = { workspace = true } netdev = { workspace = true } [dev-dependencies] +dataplane-routing = { path = ".", features = ["testing"] } +lpm = { workspace = true, features = ["testing"] } clock = { workspace = true, features = ["virtual"] } +criterion = { workspace = true } +iai-callgrind = { workspace = true } bolero = { workspace = true, default-features = false } concurrency = { workspace = true } -lpm = { workspace = true, features = ["testing"] } net = { workspace = true, features = ["test_buffer"] } rand = { workspace = true, default-features = false, features = ["thread_rng"] } tokio = { workspace = true, features = ["time", "test-util"] } tracing-test = { workspace = true, features = [] } + +[[bench]] +name = "fib_lookup" +harness = false + +[[bench]] +name = "fib_lookup_callgrind" +harness = false diff --git a/routing/benches/common/mod.rs b/routing/benches/common/mod.rs new file mode 100644 index 0000000000..506f8c3e9f --- /dev/null +++ b/routing/benches/common/mod.rs @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +#![allow(dead_code)] + +use lpm::prefix::Prefix; +use net::buffer::TestBuffer; +use net::interface::InterfaceIndex; +use net::ip::NextHeader; +use net::packet::Packet; +use net::packet::test_utils::build_test_ipv4_packet_with_transport; + +use dataplane_routing::testing::{Fib, FibGroup, FibWriter, FwAction, NhopKey, RouteOrigin}; +use dataplane_routing::{EgressObject, FibEntry, PktInstruction}; + +pub const ROUTE_ADDR: (&str, u8) = ("5.0.0.0", 8); + +#[macro_export] +macro_rules! for_each_shape { + ($expand:ident) => { + $expand! { + g1_e1 = (1, 1), + g1_e4 = (1, 4), + g4_e1 = (4, 1), + g4_e4 = (4, 4), + g8_e1 = (8, 1), + g16_e1 = (16, 1), + g16_e4 = (16, 4), + } + }; +} + +pub struct Fixture { + pub writer: FibWriter, + pub packet: Packet, +} + +fn nhop_key(n: u8) -> NhopKey { + NhopKey::new( + RouteOrigin::default(), + Some(format!("10.0.{n}.1").parse().expect("valid address")), + InterfaceIndex::try_new(u32::from(n) + 1).ok(), + None, + FwAction::Forward, + ) +} + +fn fib_group(n: u8, entries: u8) -> FibGroup { + let mut group = FibGroup::new(); + for e in 0..entries { + group.add(FibEntry::with_inst(PktInstruction::Egress( + EgressObject::new( + InterfaceIndex::try_new(u32::from(n) * 256 + u32::from(e) + 1).ok(), + Some(format!("10.{n}.{e}.1").parse().expect("valid address")), + ), + ))); + } + group +} + +pub fn packet() -> Packet { + build_test_ipv4_packet_with_transport(64, Some(NextHeader::UDP)) + .expect("a well-formed test packet") +} + +pub fn fixture(groups: u8, entries_per_group: u8) -> &'static Fixture { + let (mut writer, _reader) = FibWriter::new(0); + let keys: Vec = (0..groups).map(nhop_key).collect(); + for (n, key) in keys.iter().enumerate() { + let n = u8::try_from(n).expect("group count fits a byte"); + writer.register_fibgroup(key, &fib_group(n, entries_per_group), false); + } + writer.add_fibroute(Prefix::expect_from(ROUTE_ADDR), keys, true); + + let packet = packet(); + + { + let fib = writer.enter().expect("fib is readable"); + let (hit, _) = Fib::lpm_entry_prefix(&fib, &packet); + assert_eq!( + hit, + Prefix::expect_from(ROUTE_ADDR), + "{groups}g x{entries_per_group}e: lookup missed the installed route" + ); + } + + Box::leak(Box::new(Fixture { writer, packet })) +} + +#[inline(always)] +pub fn lookup(fixture: &Fixture) { + let fib = fixture.writer.enter().expect("fib is readable"); + let (prefix, entry) = Fib::lpm_entry_prefix(&fib, std::hint::black_box(&fixture.packet)); + std::hint::black_box((prefix, entry)); +} diff --git a/routing/benches/fib_lookup.rs b/routing/benches/fib_lookup.rs new file mode 100644 index 0000000000..2b913ab843 --- /dev/null +++ b/routing/benches/fib_lookup.rs @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +use std::hint::black_box; + +use criterion::{BenchmarkId, Criterion, Throughput, criterion_group, criterion_main}; + +mod common; +use common::{Fixture, fixture, lookup, packet}; + +macro_rules! shape_table { + ($($id:ident = ($groups:expr, $entries:expr)),* $(,)?) => { + &[$((stringify!($id), $groups, $entries)),*] + }; +} +const SHAPES: &[(&str, u8, u8)] = for_each_shape!(shape_table); + +fn bench_lookup(c: &mut Criterion) { + let mut group = c.benchmark_group("fib_lpm_entry_prefix"); + for &(id, groups, entries) in SHAPES { + let fixture: &'static Fixture = fixture(groups, entries); + let total = u64::from(groups) * u64::from(entries); + group.throughput(Throughput::Elements(1)); + group.bench_with_input( + BenchmarkId::from_parameter(format!("{id} ({total} entries)")), + &fixture, + |b, fixture| b.iter(|| lookup(black_box(fixture))), + ); + } + group.finish(); +} + +fn bench_trie_floor(c: &mut Criterion) { + let mut group = c.benchmark_group("fib_lpm_floor"); + let packet = packet(); + let destination = packet + .ip_destination() + .expect("the test packet has a destination"); + + // A control rather than a measurement: `lpm_with_prefix` returns the `FibRoute` without + // touching its groups, so these two shapes must report the same number. A difference means + // the trie lookup has started depending on what the route holds. + for &(groups, entries) in &[(1u8, 1u8), (16, 1)] { + let fixture = fixture(groups, entries); + group.bench_function( + BenchmarkId::from_parameter(format!("{groups}g x{entries}e")), + |b| { + let fib = fixture.writer.enter().expect("fib is readable"); + b.iter(|| black_box(fib.lpm_with_prefix(black_box(&destination)))); + }, + ); + } + group.finish(); +} + +fn bench_destination(c: &mut Criterion) { + let packet = packet(); + c.bench_function("packet_ip_destination", |b| { + b.iter(|| black_box(black_box(&packet).ip_destination())); + }); +} + +criterion_group!(benches, bench_lookup, bench_trie_floor, bench_destination); +criterion_main!(benches); diff --git a/routing/benches/fib_lookup_callgrind.rs b/routing/benches/fib_lookup_callgrind.rs new file mode 100644 index 0000000000..a83ec0e6a3 --- /dev/null +++ b/routing/benches/fib_lookup_callgrind.rs @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! The same fib lookup as `fib_lookup.rs`, counted instead of timed. +//! +//! Callgrind executes the program on a synthetic machine and counts what it does: instructions +//! retired, and hits and misses against a modelled cache hierarchy. That buys repeatability a +//! wall-clock benchmark cannot have -- no scheduler, no frequency scaling, no neighbours -- at the +//! cost of measuring a CPU nobody ships. It does not model this machine's out-of-order execution, +//! it has no AVX-512, and its cache model is a simplification of any real one. +//! +//! So the numbers here are not predictions of time. They are useful for two things: catching a +//! change in how much *work* a path does, which is stable enough to gate in CI; and, read next to +//! `fib_lookup.rs`, showing where counting and timing disagree -- which is where a real machine's +//! behaviour is doing something the model cannot see. +//! +//! The fixtures are deliberately identical to the criterion bench so the two can be compared. +//! +//! `development/code/benchmarking.md` measures how far apart the two harnesses get on a real +//! change, and records what the other valgrind tools do and do not scope to. Read it before +//! pointing any of them at DPDK: valgrind reports a CPU it can emulate, so `rte_acl` runs to +//! completion having executed no AVX-512 at all. + +use std::hint::black_box; + +use iai_callgrind::{ + Cachegrind, Dhat, LibraryBenchmarkConfig, library_benchmark, library_benchmark_group, main, +}; + +mod common; +use common::{Fixture, fixture, lookup}; + +#[library_benchmark] +#[bench::guard_only(args = (1, 1), setup = fixture)] +fn enter_only(fixture: &'static Fixture) { + black_box(fixture.writer.enter().expect("fib is readable")); +} + +macro_rules! shape_benches { + ($($id:ident = ($groups:expr, $entries:expr)),* $(,)?) => { + #[library_benchmark] + $(#[bench::$id(args = ($groups, $entries), setup = fixture)])* + fn lpm_entry_prefix(fixture: &'static Fixture) { + lookup(fixture); + } + }; +} +for_each_shape!(shape_benches); + +#[library_benchmark( + config = LibraryBenchmarkConfig::default() + .tool(Cachegrind::default().args([ + "--D1=32768,8,64", + "--I1=32768,8,64", + "--LL=33554432,16,64", + ])) + .tool(Dhat::default()) +)] +#[bench::g1_e1(args = (1, 1), setup = fixture)] +#[bench::g16_e4(args = (16, 4), setup = fixture)] +fn under_other_tools(fixture: &'static Fixture) { + lookup(fixture); +} + +library_benchmark_group!( + name = fib_lookup; + benchmarks = enter_only, lpm_entry_prefix, under_other_tools +); +main!(library_benchmark_groups = fib_lookup); diff --git a/routing/src/fib/test.rs b/routing/src/fib/test.rs index 83d51723fb..88c776c16a 100644 --- a/routing/src/fib/test.rs +++ b/routing/src/fib/test.rs @@ -113,7 +113,10 @@ mod tests { emulated => 50, _ => 100_000, }; - const NUM_WORKERS: u16 = 4; + const NUM_WORKERS: u16 = cfg_select! { + emulated => 2, + _ => 4, + }; // sync main thread - worker thread(s) let done = Arc::new(AtomicU16::new(0)); @@ -249,7 +252,10 @@ mod tests { #[allow(clippy::too_many_lines)] fn test_concurrency_fibtable() { // number of threads looking up fibtable - const NUM_WORKERS: u16 = 6; + const NUM_WORKERS: u16 = cfg_select! { + emulated => 2, + _ => 6, + }; const NUM_PACKETS: u64 = cfg_select! { emulated => 30, // Coverage instruments every counter; the full count costs ~47s diff --git a/routing/src/lib.rs b/routing/src/lib.rs index f5d1b3b82a..82496d120b 100644 --- a/routing/src/lib.rs +++ b/routing/src/lib.rs @@ -45,6 +45,14 @@ pub use rib::encapsulation::{ }; pub use rib::vrf::{RouterVrfConfig, VrfId}; +#[cfg(any(test, feature = "testing"))] +pub mod testing { + pub use crate::fib::fibobjects::FibGroup; + pub use crate::fib::fibtype::{Fib, FibReader, FibWriter}; + pub use crate::rib::nexthop::{FwAction, NhopKey}; + pub use crate::rib::vrf::RouteOrigin; +} + pub use bmp::spawn_bmp_server; pub use router::ctl::RouterCtlSender; pub use router::{BmpServerParams, CliSources, Router, RouterParams, RouterParamsBuilder}; diff --git a/routing/src/rib/nexthop.rs b/routing/src/rib/nexthop.rs index 14a74abb62..efa2f137d6 100644 --- a/routing/src/rib/nexthop.rs +++ b/routing/src/rib/nexthop.rs @@ -93,7 +93,7 @@ impl NhopKey { } } #[cfg(test)] - pub fn from_address(address: &str) -> Self { + pub(crate) fn from_address(address: &str) -> Self { Self { address: Some(IpAddr::from_str(address).expect("Bad address")), ..Default::default() @@ -101,7 +101,7 @@ impl NhopKey { } #[cfg(test)] #[must_use] - pub fn with_addr_ifindex(address: &str, ifindex: u32) -> Self { + pub(crate) fn with_addr_ifindex(address: &str, ifindex: u32) -> Self { Self { address: Some(IpAddr::from_str(address).expect("Bad address")), ifindex: Some(InterfaceIndex::try_new(ifindex).expect("Bad ifindex")), @@ -110,7 +110,7 @@ impl NhopKey { } #[cfg(test)] #[must_use] - pub fn with_address(address: &IpAddr) -> Self { + pub(crate) fn with_address(address: &IpAddr) -> Self { Self { address: Some(*address), ..Default::default() @@ -118,7 +118,7 @@ impl NhopKey { } #[cfg(test)] #[must_use] - pub fn with_ifindex(ifindex: u32) -> Self { + pub(crate) fn with_ifindex(ifindex: u32) -> Self { Self { ifindex: Some(InterfaceIndex::try_new(ifindex).unwrap()), ..Default::default() diff --git a/scripts/bench-report.ts b/scripts/bench-report.ts new file mode 100755 index 0000000000..b73c418236 --- /dev/null +++ b/scripts/bench-report.ts @@ -0,0 +1,214 @@ +#!/usr/bin/env -S deno run --allow-read +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + + +type Metric = { Int: number } | { Float: number }; + +type Metrics = { Both: [Metric, Metric] } | { Left: Metric } | { + Right: Metric; +}; + +interface Record_ { + id: string | null; + function_name: string; + module_path: string; + profiles: Array<{ + tool: string; + summaries: { + total: { + summary: Record< + string, + Record< + string, + { + diffs?: { diff_pct?: string }; + metrics: Metrics; + } + > + >; + }; + }; + }>; +} + +const HEADLINE: Record = { + Callgrind: ["Ir", "EstimatedCycles"], + Cachegrind: ["Ir", "EstimatedCycles"], + DHAT: ["TotalBytes", "TotalBlocks"], + Memcheck: ["Errors"], + Massif: ["PeakBytes"], +}; + +function value(m: Metric): number { + return "Int" in m ? m.Int : m.Float; +} + +interface Row { + bench: string; + tool: string; + metric: string; + now: number; + before: number | null; + pct: number | null; +} + +function rows(records: Record_[]): Row[] { + const out: Row[] = []; + for (const rec of records) { + const bench = rec.id ? `${rec.function_name} ${rec.id}` : rec.function_name; + for (const profile of rec.profiles ?? []) { + const summaries = profile.summaries?.total?.summary ?? {}; + const summary = summaries[profile.tool] ?? Object.values(summaries)[0]; + if (!summary) continue; + for (const metric of HEADLINE[profile.tool] ?? []) { + const entry = summary[metric]; + if (!entry) continue; + const m = entry.metrics; + // `Both` is [new, baseline]. `Left` is new-only -- a benchmark that did not exist in + // the baseline. `Right` is baseline-only, i.e. a benchmark that has been *removed*; + // reporting it as `now` with no `before` renders it as an addition, which is the + // wrong way round but is the only shape this table has for it. + const now = "Both" in m + ? value(m.Both[0]) + : "Left" in m + ? value(m.Left) + : value(m.Right); + const before = "Both" in m ? value(m.Both[1]) : null; + const pct = before === null || before === 0 + ? null + : ((now - before) / before) * 100; + out.push({ bench, tool: profile.tool, metric, now, before, pct }); + } + } + } + return out; +} + +const fmt = (n: number) => n.toLocaleString("en-US"); +const pct = (p: number | null) => + p === null ? "—" : `${p >= 0 ? "+" : ""}${p.toFixed(2)}%`; + +function bar(p: number | null, worst: number): string { + if (p === null || worst === 0 || Math.abs(p) < 0.005) return ""; + const width = Math.min(10, Math.round((Math.abs(p) / worst) * 10)); + return (p >= 0 ? "▰" : "▱").repeat(Math.max(1, width)); +} + +function table(rs: Row[]): string { + const worst = Math.max(0, ...rs.map((r) => Math.abs(r.pct ?? 0))); + const head = + "| benchmark | tool | metric | before | after | change | |\n|---|---|---|---:|---:|---:|---|"; + const body = rs.map( + (r) => + `| ${r.bench} | ${r.tool} | ${r.metric} | ${ + r.before === null ? "—" : fmt(r.before) + } | ${fmt(r.now)} | ${pct(r.pct)} | ${bar(r.pct, worst)} |`, + ); + return [head, ...body].join("\n"); +} + +function chart(rs: Row[]): string { + const shown = rs.filter( + (r) => r.tool === "Callgrind" && r.metric === "Ir" && r.pct !== null, + ); + if (shown.length === 0) return ""; + const labels = shown.map((r) => `"${r.bench.replace(/"/g, "")}"`).join(", "); + const values = shown.map((r) => (r.pct ?? 0).toFixed(2)).join(", "); + const span = Math.max(5, ...shown.map((r) => Math.abs(r.pct ?? 0))) * 1.2; + return [ + "```mermaid", + "xychart-beta", + ' title "Instructions retired, change vs baseline (%)"', + ` x-axis [${labels}]`, + ` y-axis "change (%)" ${(-span).toFixed(0)} --> ${span.toFixed(0)}`, + ` bar [${values}]`, + "```", + ].join("\n"); +} + +function main() { + const args = Deno.args.filter((a) => !a.startsWith("--")); + const flags = Deno.args.filter((a) => a.startsWith("--")); + const rawThreshold = flags.find((f) => f.startsWith("--threshold=")) + ?.split("=")[1] ?? "5"; + const threshold = Number(rawThreshold); + // `Math.abs(p) >= NaN` is false for every p, so a typo here would silently report "no + // change beyond NaN%" for every run rather than failing. + if (!Number.isFinite(threshold)) { + console.error(`error: --threshold=${rawThreshold} is not a number`); + return 1; + } + const headlineOnly = flags.includes("--headline-only"); + + if (args.length !== 1) { + console.error( + "usage: bench-report.ts [--threshold=N] [--headline-only]", + ); + Deno.exit(2); + } + + const records: Record_[] = Deno.readTextFileSync(args[0]) + .split("\n") + .filter((line) => line.trim().length > 0) + .map((line) => JSON.parse(line)); + + const rs = rows(records); + const compared = rs.filter((r) => r.pct !== null); + + const worstOf = (subset: Row[]) => + subset.reduce( + (acc: Row | null, r) => + acc === null || Math.abs(r.pct!) > Math.abs(acc.pct!) ? r : acc, + null, + ); + const worst = worstOf( + compared.filter((r) => r.tool === "Callgrind" && r.metric === "Ir"), + ) ?? worstOf(compared); + const stark = worst !== null && Math.abs(worst.pct!) >= threshold; + + const alloc = worstOf( + compared.filter( + (r) => + r.tool === "DHAT" && r.metric === "TotalBytes" && Math.abs(r.pct!) >= 1, + ), + ); + const allocNote = alloc === null + ? "" + : `; bytes allocated ${pct(alloc.pct)} (DHAT)`; + + if (compared.length === 0) { + console.log( + "**Benchmarks**: no baseline to compare against; recorded a new one.", + ); + } else if (!stark) { + console.log( + `**Benchmarks**: no change beyond ${threshold}% (largest: ${ + worst!.bench + } ${worst!.metric} ${pct(worst!.pct)})${allocNote}.`, + ); + } else { + const dir = worst!.pct! > 0 ? "more" : "less"; + console.log( + `**Benchmarks**: ${worst!.bench} does ${pct(worst!.pct)} ${dir} work (${ + worst!.metric + })${allocNote}.`, + ); + } + if (headlineOnly) return; + + console.log(""); + console.log(table(rs)); + if (stark) { + console.log(""); + console.log(chart(rs)); + } + console.log(""); + console.log( + "> Instruction counts from callgrind, not timings: repeatable across runners, and blind to " + + "anything that changes data layout rather than instruction count. See " + + "`development/code/benchmarking.md`.", + ); +} + +main();