bench(cpu): put benchmark processes in the production decode topology by justinchuby · Pull Request #1766 · justinchuby/onnx-genai

justinchuby · 2026-08-22T18:14:25Z

Closes #1749.

None of the 12 CPU benchmarks called EpFactory::initialize, so none of them ran in the thread topology a served session runs in.

initialize is the earliest per-session hook and the only place an explicit ONNX_GENAI_CPU_DECODE_THREADS budget becomes a process-wide bound on prefill/MLAS Rayon parallelism and, on Linux, on CPU affinity (provider.rs:334-343). Without it, a t=N row measures N decode workers competing with a full-width Rayon pool across every logical CPU.

Measured, `int4_decode_loop_ab`, 16-physical / 32-logical host

budget	threads before	affinity before	threads after	affinity after
4	6	`0-31`	9	`0,2,4,6`
16	18	`0-31`	33	`0,2,...,30` (16 physical cores)

Before, the budget changed the decode pool width and nothing else. The process stayed spread across all 32 logical CPUs at every budget, so both SMT siblings of each core were in play and prefill/MLAS kept a full-width Rayon pool. That is not a smaller-budget measurement; it is the same machine with a differently sized decode pool bolted on.

After, the process reports what production reports:

onnx-genai: CPU decode budget 4 confined the process to 4 CPUs [0, 2, 4, 6] (prefill/MLAS + decode stay off the rest of the machine)
onnx-genai: CPU decode budget 4 bounded the global Rayon pool (prefill/MLAS parallelism capped at 4 workers)

Neither line could be emitted by any benchmark before this change, because nothing in benches/ reached the only call site that prints them.

Why a helper rather than inlining the current behaviour

common::init_decode_topology() calls the real CpuExecutionProvider::initialize. A reimplementation of "what initialize does today" would silently stop matching the moment initialize grows a second responsibility — exactly the drift that leaves a benchmark measuring a configuration nothing ships. It is a no-op unless a budget is set, and idempotent (the underlying bound latches on first call).

gqa_decode already had bind_decode_pool_width, but that caps the global Rayon pool rather than applying the production bound, so it does not stand in for this. It keeps its existing call; the two are not in conflict.

Scope

Benches only — no kernel, scheduler, or library code changes. No performance claim is made or revised here. The point is narrower and prior to any claim: rows taken from here on describe the configuration they are labelled with. Previously published t=N rows from these binaries should be treated as untrusted, not as re-baselined.

Related: #1763 (a requested width is still unverifiable even once a bench is initialized correctly). The two are complementary — this one makes a bench have production topology, that one makes it checkable. Both are needed before a t=N scaling curve means anything.

Validation

cargo build -p onnx-runtime-ep-cpu --benches (debug + release) — all 12 build
cargo clippy -p onnx-runtime-ep-cpu --all-targets -D warnings — clean, also under --features mlas, --no-default-features, and --target aarch64-unknown-linux-gnu
cargo fmt --all --check — clean
All 9 applicable Rust quality scripts — pass
Before/after topology measured directly from /proc/<pid>/task and taskset -cp, table above

None of the 12 CPU benchmarks called `EpFactory::initialize`, so none of them ran in the thread topology a served session runs in. `initialize` is the earliest per-session hook and the only place an explicit `ONNX_GENAI_CPU_DECODE_THREADS` budget becomes a process-wide bound on prefill/MLAS Rayon parallelism and, on Linux, on CPU affinity. Without it a `t=N` row measures N decode workers competing with a full-width Rayon pool across every logical CPU -- a configuration nothing ships. Measured on this 16-physical/32-logical host with `int4_decode_loop_ab`: | budget | threads before | affinity before | threads after | affinity after | |---|---|---|---|---| | 4 | 6 | 0-31 | 9 | 0,2,4,6 | | 16 | 18 | 0-31 | 33 | 16 physical cores | Before, the budget changed the decode pool width and nothing else: the process stayed spread across all 32 logical CPUs at every budget, so both SMT siblings of each core were in play and prefill/MLAS kept a full-width Rayon pool. That is not a smaller-budget measurement, it is the same machine with a differently sized decode pool bolted on. `gqa_decode` already had `bind_decode_pool_width`, but it caps the *global Rayon* pool rather than applying the production bound, so it does not stand in for this. The helper calls the real `CpuExecutionProvider::initialize` rather than reimplementing what it currently does, so it cannot drift the moment `initialize` grows a second responsibility -- which is precisely the drift that leaves a benchmark measuring something nothing ships. It is a no-op unless a budget is set, and idempotent. No kernel, scheduler, or library code changes; benches only. No performance claim is made or revised here -- the point is that rows taken from here on describe the configuration they are labelled with. Closes #1749. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

codecov · 2026-08-22T18:25:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.19%. Comparing base (9747b49) to head (4e34e84).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1766      +/-   ##
==========================================
- Coverage   80.58%   80.19%   -0.40%     
==========================================
  Files         411      411              
  Lines      198803   198803              
  Branches   198803   198803              
==========================================
- Hits       160210   159430     -780     
- Misses      33168    33948     +780     
  Partials     5425     5425

Flag	Coverage Δ
cli-ort-linux	`72.47% <ø> (ø)`
cli-ort-windows	`72.06% <ø> (ø)`
mlas	`85.20% <ø> (-0.14%)`	⬇️
offline	`80.32% <ø> (-0.41%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 7 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ain main Review caught that `kernels` -- one of the 12 the previous commit claimed to cover -- got no call at all, so `cargo bench --bench kernels` with a budget set still measured the unbounded topology #1749 is about. Verified by running the binary with a budget: no "confined the process" line was emitted. It now is. The criterion benches also had the call in only the first `criterion_group!` member. That happened to work, because criterion runs the first target's body before any measurement or analysis, but it made coverage ride on target order: prepending or removing a target would leave the new first benchmark measuring unbounded, and its post-measurement analysis builds the global Rayon pool at full width, after which every later call hits `build_global`'s already-built path and the Rayon bound is silently lost for the whole file. That is the same silent-drift failure the helper's own doc warns about, reintroduced through ordering. The call is idempotent, so every group member now makes it. Coverage is now 19 of 19 entry points across 12 of 12 benches, and all three criterion binaries emit the bound exactly once under a budget. `kernels` installs its own fixed-width Rayon pool (it sweeps `[1, 8]`), so under a smaller budget it will oversubscribe that pool onto the budget's cores. That is what production does with the same two settings, so it is applied there rather than special-cased, and the helper's doc now says so instead of leaving it to be rediscovered. Also corrected `EpFactory::initialize` to `CpuExecutionProvider::initialize` in the helper doc -- there is no Rust `EpFactory::initialize`; `EpFactory` is the ORT C ABI type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…logy

justinchuby · 2026-08-22T18:37:45Z

Adversarial review (Opus) — verdict

One BLOCKING, one SHOULD-FIX, one NIT. All three fixed in da4bfce90. The blocking finding was a real gap in my own change and the PR body's central claim.

BLOCKING — `kernels` was never wired up

I claimed "none of the 12 ... Closes #1749" and then covered 11. kernels.rs uses criterion_main! and received no call, so cargo bench --bench kernels with a budget set still measured exactly the unbounded topology this PR is about. The reviewer verified it by running the binary — no confined the process line — rather than by reading the diff, which is how it was caught at all.

Fixed, and re-verified the same way rather than by inspection:

kernels        bound_lines=1
gqa_decode     bound_lines=1
sdpa_simd      bound_lines=1

Coverage is now 19 of 19 entry points across 12 of 12 benches (was 10 of 19 across 11 of 12).

The reviewer also flagged the judgement call rather than treating it as mechanical: kernels installs its own fixed-width Rayon pool (it sweeps [1, 8]), so under a smaller budget it oversubscribes that pool onto the budget's cores. I applied it there anyway — that is what production does with the same two settings — but the point stands that it was previously neither done nor documented, and the helper's doc now states it.

SHOULD-FIX — coverage rode on `criterion_group!` target order

The call sat only in the first group member. That works today because criterion runs the first target's body before any measurement or analysis, and the reviewer confirmed empirically that filtering to a later benchmark still fires the bound. But prepending or removing a target would leave the new first benchmark measuring unbounded — and its post-measurement analysis builds the global Rayon pool at full width, after which every later call hits build_global's already-built path and the Rayon bound is silently lost for the entire file.

That is the same silent-drift failure mode the helper's own doc warns about for reimplementation, reintroduced through ordering. The call is idempotent, so every group member now makes it and coverage no longer depends on order.

NIT — `EpFactory::initialize` names a method that does not exist

There is no Rust EpFactory::initialize; EpFactory is the ORT C ABI type (OrtEpFactory). The method is ExecutionProvider::initialize, implemented by CpuExecutionProvider — which the same doc comment named correctly two lines later. Corrected to one name throughout.

Independently confirmed, no change needed

Late-call safety (the question I most wanted answered): criterion touches the global Rayon pool only in post-measurement statistics, and Criterion::default() / configure_from_args() do not — so build_global succeeds from inside the first bench function. Calling it there is not equivalent-in-risk to calling it in main, but it is safe for the first-run target, which is why the ordering issue above is latent rather than live.
.expect("the CPU EP must initialize") cannot panic in a legitimate environment — initialize is infallible, returning Ok(()) after a call that logs rather than errors.
New mod common; in three benches — full bench build under RUSTFLAGS="-D warnings" is clean; common/mod.rs carries #![allow(dead_code)].
The affinity table was reproduced independently on a 16-physical/32-logical host: budget 4 → [0, 2, 4, 6], budget 16 → the 16 physical cores.

Re-validation

Latest main merged (4e34e84f2), which now includes #1764. All 12 benches build (debug + release); cargo fmt --all --check clean; cargo clippy -p onnx-runtime-ep-cpu --all-targets -D warnings clean under default / --features mlas / --no-default-features / aarch64-unknown-linux-gnu; 9 applicable Rust quality scripts pass.

) ## What `int4_decode_loop_ab` sweeps `ONNX_GENAI_CPU_DECODE_THREADS` and prints a row per width, but nothing in the row reported the width the run **actually built**. Adds `common::report_decode_width()` and calls it after the measured phases. ``` steady 20.053 20.625 48.6 0.0 decode_width requested=2 realized=2 path=spmd-pool as_requested ``` ## Why this is not hypothetical At least four paths silently reduce realized width below the request — the pre-clamp in `resolve_persistent_decode_threads_with_override`, both headroom reservations, and the single-CPU-cpuset branch that drops decode to the flat path. Only one is even `NXRT_CALIB_DEBUG`-visible. `ONNX_GENAI_CPU_DECODE_THREADS=2` was recorded in `docs/benchmarks/2026-08-21-int4-acc4-execution-regime.md` as producing timings *identical* to `=1`. The row was dropped rather than explained. **It was an unbounded-topology artifact, and it is already fixed.** Measured at `9747b4971` (current main minus #1766), same binary otherwise, qwen/acc0/block32/s=1: | t | ms/token | CPU% | vs t=1 | |---|---|---|---| | 1 | 40.298 | 100% | 1.00x | | 2 | 37.166 | 267% | **1.08x for 2.67 cores** | Before #1766 the benches never called `initialize()`, so the process was unbounded and global Rayon ran full width at *every* budget. The "t=1" arm was not 1-wide. On current main, same sweep, both directions (<2% between passes): | t | ms/token ↑ | ↓ | speedup | CPU% | path | |---|---|---|---|---|---| | 1 | 39.649 | 39.532 | 1.00x | 99% | flat | | 2 | 20.053 | 20.053 | **1.98x** | 187% | spmd-pool | | 4 | 10.133 | 10.138 | 3.91x | 326% | spmd-pool | | 8 | 5.230 | 5.279 | 7.55x | 549% | spmd-pool | | 16 | 2.664 | 2.609 | **15.0x** | 886% | spmd-pool | A/A null control at t=4: 10.179 vs 10.077 (1.0%). ## Design notes - **Reports, does not assert.** A reduced width is legitimate when the host cannot honour the request (8 lanes in a 2-CPU cpuset); aborting would leave a constrained container unable to benchmark at all. `WIDTH-MISMATCH` is for the caller to discard or mark the row, as a contended cell is already marked UNTRUSTED. - **Called after the phases, never before.** The pool is built lazily at first decode and `decode_width()` is deliberately non-forcing; reading it earlier reports `path=unresolved`, and a forcing read would build the pool and change the topology being measured. - **Scoped to `int4_decode_loop_ab`.** `gqa_decode` also reads a decode width but pins the ambient Rayon pool by design, so a width line there would report `path=flat` and mislead rather than verify. ## Validation - `cargo fmt --check` clean; `clippy --benches --all-targets -D warnings` clean - All 12 benches build (release) - Width line verified **by execution** at t=1/2/4/8/16 — `realized` tracks the request at every width, `path` flips `flat`→`spmd-pool` at t=2 Bench-only; no shipped code changes. Refs #1763, #1766 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…erseded (#1772) #1771 found that every `t=N` decode row published before #1766 (`11cb8e5f3`) was taken on a process that never called `CpuExecutionProvider::initialize()`. The decode budget sized the SPMD pool but did not *bound the process*: affinity stayed at all logical CPUs and the global Rayon pool ran full width at every budget. The `t=1` arm was never 1-wide. This marks the affected rows. It does not delete them, and it tries to be exact about the blast radius rather than blanket-invalidating work that is still sound. ## The distinction that matters Each acc4 speedup is a **paired A/B whose two arms shared the same broken topology**. A ratio measured that way is still a ratio: 1.686x at t=1 and 1.380x at block 64 remain the measured effect of removing per-block bookkeeping, and the recommendation built on them stands. What does not survive is any claim that attributes an effect **to a width**, because the width on the label was not the width that ran. Those are called out individually: the "wash at t=8", the t=16 rows, "the one place bandwidth does bind is full width", and the cross-width comparisons in *The regime*. ## §27's `sys` claim is mine and was worse than imprecise The section records "`sys` time rising ~20x from `t<=2` to `t>=4`" as the evidence handed to the runtime owner. The pre-#1766 column had the **opposite shape** — `sys` was highest at `t=2` (2.19s) and *decreased* with width. On bounded topology the onset is `t=16` and an order of magnitude smaller (1.14s). I wrote that characterisation from my own contaminated column, so it is corrected in my voice rather than attributed to the runtime owner. What stands: the `yield_now`-per-iteration ramp is in the source, and the blocktime `500us -> 0` A/B (`sys` 28.1s -> 5.1s, latency-neutral) is a genuine within-configuration result. What does not: its magnitude, since it was measured with the pool oversubscribed against a full-width Rayon pool, which inflates barrier time by itself. Recorded as unquantified pending a re-run, not as refuted. ## Also - The withdrawn "`=2` is inert" claim was previously explained as contention. #1771 supplies the mechanical cause, which is better: the unbounded process made the bottom of the sweep flat. Contention was a real confound in that window but is not what produced the identity. The independent falsifier against `9747b4971` reproduces the correction from the other direction — **1.98x at t=2, 187% CPU**. - §27 recommends verifying a knob *structurally* rather than by timings. That guard now exists in CI (#1747, `b9d9c48`) and is mutation-proved against production, so the note points at it. Ticks the first box of #1771's suggested actions. The second (re-deriving the `sys`/`worker_wait` onset) is runtime-owned and I have handed the correction over with the evidence. Docs-only; `ci.yml` skips the Rust jobs for `docs/*|*.md` by design. Refs #1771, #1766, #1747 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…=8 3.08x Review of the previous commit made the right objection: the ORT arm reproducing to +4.4% shows the *ORT* ruler did not move and says nothing about the native one, which changed repeatedly over the same window (#1722 is literally titled "make the acc0 native and ORT arms measure one quantity"). So the inference is replaced with a measurement. `e9754e7ef`'s bench is rebuilt in a second worktree and run beside current main's -- same host, same environment, `PROBE_REPS=1` on both so neither gets a rep loop the other lacks, arms interleaved and the order alternated: | width | kernel-only, measured | published pair implies | verdict | |------:|----------------------:|-----------------------:|---------| | 1 | 1.64x [1.61-1.88], 12 cells | 1.59x | movement is kernel | | 8 | 1.82x [1.78-1.89], 6 cells | 3.08x | 3.08x RETRACTED | Both old figures reproduce to within 0.4% -- but only **unpinned**. The old bench never called `EpFactory::initialize`, so it never ran `bound_process_to_decode_budget()`; that function, physical-core selection included, already existed at `e9754e7ef` and production always called it. The old t=8 row therefore measured eight decode workers scattered over 32 logical CPUs onto SMT siblings -- a topology no served session ever ran in. #1766 added the call. Pinned to eight physical cores the same old binary gives 8.430 ms against its unpinned 14.115, and forced onto `0-7`, 16.121. 1.67x of the claimed 3.08x was placement, not kernel work. This is the effect 2026-08-21-decode-worker-cpu-placement.md (#1680) already recorded, landing on a number I quoted two days later. Two corrections that look right and are not, recorded so they are not re-applied: the ~11% warmup/spawn handicap of §27 is in `tokens_s_total`, and both published figures are `ms_token` -- the old ORT harness docstring names "the native harness's `steady` column-2 median" as its comparand and the reproductions land on it. Deducting 11% yields a number neither tree produces. The asymmetry that *is* real, old ORT `min` over reps against old native single-shot, biases in ORT's favour. The gap conclusion is unchanged and is measured on today's tree, both arms, matched pins -- but the headline table is rebuilt to fix three defects: - it mixed statistics (native median latency over ORT throughput-equivalent) so its columns did not yield its own gap figure. Both sides are now `tokens_s_total`, with the mixed variant shown and labelled; - `t=4` was quoted as 1.148x when its A/A null spans 0.868-1.150, so the gap is inside its own noise floor there. Now "~1.15x, does not resolve"; - "three independent launches per width" was false (3/5/3 cells across two invocations) and the `t=1` 1.4% spread depended on an undisclosed post-hoc discard. Retained-cell figures are published beside the headline (1.112x [0.927-1.128], n=3) and the discard rule is stated prospectively. `acc0_gap_matrix.py` gains `--launches`, a per-width `--tokens 1:64,4:192` map and a `gap` column in ORT/native orientation beside `ratio`, so the Reproduce block names a command that produces the published table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…t=1/4/8 (#1852) ## The 1.84x acc0 gap is stale. Re-measured, it is ~1.12x. The published acc0 (`accuracy_level = 0`, the production default) int4 decode gap against ORT — **1.84x at t=1** — is what made acc0 the top remaining CPU MatMulNBits target in the ledger. It dates from `e9754e7ef` (#1628) and **eight merges have landed since**, three of them direct acc0 kernel work. On current main the gap measures **~1.12x**. | width | native tok/s | ORT tok/s | **gap** | gap range | cells (trusted/taken) | A/A range | |---:|---:|---:|---:|---:|---:|---:| | 1 | 27.9 | 31.2 | **1.120x** | 1.112–1.128 | 3/3, **2 retained** | 1.025–1.036 | | 4 | 107.2 | 122.3 | ~1.15x | 1.087–1.284 | 4/6 | **0.868–1.150** | | 8 | 211.0 | 238.0 | **1.120x** | 1.089–1.145 | 3/3 | 0.997–1.028 | | 16 | — | — | ~1.64x, **does not resolve** | 1.456–1.831 | 2/3 | **0.969–1.295** | Both arms are `tokens_s_total`, paired within each launch and then medianed. *Trusted/taken* is the harness's verdict; *retained* is editorial — all three `t=1` cells passed the guard and one was dropped afterwards by me, disclosed below. Only `t=1` and `t=8` resolve. `t=4` sits inside its own A/A null (0.868–1.150). **`t=16` reads ~1.64x and is the open row** — see below; a second revision of this PR corrects an earlier claim that it was wholly contaminated. **acc0 is no longer the top CPU target — conditional on `t=16`.** At the two widths that resolve, the remaining ~12% is a kernel efficiency difference that sits below several other open items. `t=16` is the width closest to an unconfined production process, and a confirmed 1.64x there would reverse that. ## Why the movement is kernel — measured, not inferred The first revision of this PR argued from a control: ORT re-measures at 31.99 ms, within 4.4% of its published 30.632, therefore the harness is comparable and *"the movement is entirely on our side."* **Review objected that this does not follow, and review was right.** The ORT arm reproducing shows the *ORT* ruler did not move. It says nothing about the native ruler, which sits in a different binary and changed repeatedly over the same window — `81e611c03` (#1722) is literally titled *"make the acc0 native and ORT arms measure one quantity"*. So the inference was replaced with a measurement. `e9754e7ef`'s tree is checked out in a second worktree, its `int4_decode_loop_ab` rebuilt, and run **beside** current main's on the same host, same environment, `PROBE_REPS=1` on both so neither gets a rep loop the other lacks, arms interleaved and the launch order alternated: | width | kernel-only, measured | published pair implies | verdict | |---:|---:|---:|---| | 1 | **1.64x** [1.61–1.88], 12 paired cells | 1.59x | apparent movement **is** kernel | | 8 | **1.82x** [1.78–1.89], 6 paired cells | 3.08x | **3.08x retracted** | ### Retracting the t=8 3.08x Both old figures reproduce today to within 0.4% — but only **unpinned**: | published | rebuilt `e9754e7ef` today, unpinned | delta | |---|---:|---:| | `56.307 ms` (t=1) | 56.519 (56.402 / 56.519 / 56.878) | +0.4% | | `14.091 ms` (t=8) | 14.115 (14.105 / 14.115 / 14.196) | +0.2% | The old bench never called `EpFactory::initialize`, so it never ran `bound_process_to_decode_budget()` and its process was never confined. That function — physical-core `select_budget_cpus` included — **already existed at `e9754e7ef`**, and production always called it; only the bench was missing the call, which #1766 `11cb8e5f3` added. The old `t=8` row therefore measured eight decode workers scattered over 32 logical CPUs onto SMT siblings: **a topology no served session ever ran in.** | binary at t=8 | 8 physical cores | 4 cores + SMT siblings | unpinned | |---|---:|---:|---:| | `e9754e7ef` | 8.430 ms | 16.121 ms | **14.115 ms** | | current main | 4.664 ms | 7.988 ms | **4.619 ms** | **1.67x of the claimed 3.08x was placement, not kernel work.** Today's binary is pin-insensitive (0.99x) because it confines itself. This is exactly the effect `docs/benchmarks/2026-08-21-decode-worker-cpu-placement.md` (#1680, ledger §24) already recorded — landing on a number I quoted two days later. ### Two corrections that look right and are not Recorded so they are not re-applied at this site: - **The ~11% warmup/spawn handicap of §27 is in `tokens_s_total`.** Both published figures are **`ms_token`** — the old `ort_matmulnbits_baseline.py` docstring names *"the native harness's `steady` column-2 median"* as its comparand, and the reproductions above land on it to 0.4%. Deducting 11% from `56.307` yields a number no run of either tree produces. - **The statistic asymmetry that *is* real points the other way.** Old ORT took `min` over reps of a per-`Run` median; old native was single-shot with no rep loop. Best-of-N against single-shot flatters ORT, so it made the old gap look *worse*. Calling the two arms "the same statistic", as the first revision did, was wrong. ## Headline-table defects fixed in this revision - **Mixed statistics.** The first table printed native *median latency* beside ORT *throughput-equivalent* and called the ratio a gap, so its columns did not yield its own gap figure. Both sides are now `tokens_s_total`; the mixed variant is shown, labelled, and noted to decline (1.113 → 1.098 → 1.085) rather than be flat. - **False precision at t=4.** `1.148x` quoted against an A/A null of 0.868–1.150. Now "~1.15x, does not resolve". - **Undisclosed post-hoc discard.** "three independent launches per width" was false (3 / 6 / 3 / 3 cells across two invocations), and the `t=1` 1.4% spread depended on discarding a cell after seeing it. Retained-cell figures are now published beside the headline (**1.112x [0.927–1.128], 18.1%, n=3** — the median barely moves, the *precision* does not survive), and the discard rule is stated prospectively for next time. - **Reproducibility.** `acc0_gap_matrix.py` gains `--launches`, a per-width `--tokens 1:64,4:192,8:384` map, and a `gap` column in ORT÷native orientation beside `ratio`, so the Reproduce block names a command that produces the published table. ## Method preconditions added 1. **The two arms were not getting the same machine.** `ONNX_GENAI_CPU_DECODE_THREADS=w` confines the *whole native process* to `w` CPUs; the script pinned ORT to all 16 even CPUs at every width. Measured effect on a quiet host: **1–2%** — real, small, and now data rather than argument. 2. **The realized width is read back and checked** (`decode_width requested=4 realized=4 as_requested`). Timings cannot detect a vacuous sweep. 3. **`LoadWatch` samples the runnable count *during* every arm**, refusing above `width + slack`. A pre-check cannot see a competitor that arrives mid-cell — one did, and four cells were discarded because of it. 4. **The wide-pin arm turned out to be a contention detector.** One `t=1` cell passed every host-level guard while CPU 0 alone was busy: both matched-pin arms ~2x slow, the roaming arm normal. A single-CPU pin is the most fragile cell in any width sweep, and it is what every speedup is quoted against. ## Still unresolved — and a correction to the first revision **`t=16`, and it is the row that matters.** The first revision of this PR wrote the width off as "every cell contaminated". **That was wrong, and wrong in the direction that flattered the conclusion.** Two of the three `t=16` cells passed the load guard cleanly (runnable 6, no competitor recorded): | `t=16` | gap | A/A | native spread | ORT spread | |---|---:|---:|---:|---:| | launch 1 | 1.831 | 1.295 | 17.8% | **55.4%** | | launch 2 | 1.456 | 0.969 | 27.7% | **19.6%** | | **median** | **1.643** | — | — | — | So it is **~1.64x from two accepted cells**, not "no data". It still does not resolve, but on the correct ground: the A/A null spans 0.969–1.295 (±30%, against 3.6% at `t=1` and 2.8% at `t=8`) and both arms are unstable at this width. The cell the guard *did* refuse reads 1.585 — between the two retained — so the discard is not load-bearing either way. This is the one open cell that could reverse the re-ranking, and it needs a dedicated quiet-host study with launch distributions and a pre-registered A/A acceptance threshold. ## Second-review fixes (this revision) An adversarial review returned MERGE AFTER FIXES with all six core claims surviving falsification and seven defects. All are fixed: 1. **`t=16` mischaracterised** — the headline fix above, propagated to all four sites that carried the re-ranking claim. 2. **Cell counts** — `t=4` is 4 trusted of **6** taken; the published table came from **two** script invocations, not three. 3. **Column semantics** — harness-trust and editorial retention were conflated in one column; now separated, which makes the `t=1` discard visible in the table rather than only in prose. 4. **`t=1` placement probe samples published**, including a `114.94 ms` outlier on one pinned rep of three (the old binary's bimodality — it is why the `t=1` A/B range reaches 1.88x). Placement is worth 1.9% at this width, against 1.67x at `t=8`, as expected for a single-threaded process with no SMT sibling to hit. 5. **ORT-side spread disclosed at `t=16`** (55.4%) — the denominator is no better behaved than the numerator there. 6. **Stale checksum constants** in `int4_decode_loop_ab`'s module doc corrected, with a note that they drift under reduction reassociation (#1667, #1783) and that the *pattern* — block 16 moves under `ONNX_GENAI_CPU_MM_INT4_GEBP=0`, block 32 does not — is the route evidence, not the digits. 7. **`--tokens` map footgun** — a map missing a `--threads` width died with a bare `KeyError` after the first cell had already waited out the load guard. It now refuses at parse time, naming the missing widths. ## Validation Docs plus one benchmark harness; no library code. The harness was stub-validated end to end (launch loop, per-width token map, `gap` column, paired per-width summary) and both binaries were run for the numbers above. Required CI (`Fast (Linux x86_64)`, `Rust quality`) must be green before merge — no admin bypass. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Roy <roy@squad.local>

justinchuby and others added 2 commits August 22, 2026 18:36

Merge remote-tracking branch 'origin/main' into seb/bench-decode-topo…

4e34e84

…logy

justinchuby marked this pull request as ready for review August 22, 2026 18:37

justinchuby enabled auto-merge (squash) August 22, 2026 18:37

justinchuby mentioned this pull request Aug 22, 2026

test(bench): make the acc0 native and ORT arms measure one quantity (retracts the 0.436x headline) #1722

Merged

justinchuby merged commit 11cb8e5 into main Aug 22, 2026
18 checks passed

justinchuby deleted the seb/bench-decode-topology branch August 22, 2026 18:52

This was referenced Aug 22, 2026

bench(cpu): print the realized decode width next to every t=N row #1770

Merged

Published decode t=N benchmark rows predate #1766 and measured an unbounded process #1771

Open

justinchuby mentioned this pull request Aug 22, 2026

docs(cpu): mark the width labels taken on an unbounded process as superseded #1772

Merged

justinchuby mentioned this pull request Aug 23, 2026

perf(cpu): place decode workers one per physical core and keep the dispatcher a core #1729

Merged

justinchuby mentioned this pull request Aug 23, 2026

docs(perf): the 1.84x acc0 gap is stale — re-measured it is 1.12x at t=1/4/8 #1852

Merged

Navigation Menu

Repository navigation

bench(cpu): put benchmark processes in the production decode topology - #1766

bench(cpu): put benchmark processes in the production decode topology#1766
justinchuby merged 3 commits into
mainfrom
seb/bench-decode-topology

justinchuby commented Aug 22, 2026

Uh oh!

codecov Bot commented Aug 22, 2026 •

edited

Loading

Uh oh!

justinchuby commented Aug 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

justinchuby commented Aug 22, 2026

Measured, int4_decode_loop_ab, 16-physical / 32-logical host

Why a helper rather than inlining the current behaviour

Scope

Validation

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

justinchuby commented Aug 22, 2026

Adversarial review (Opus) — verdict

BLOCKING — kernels was never wired up

SHOULD-FIX — coverage rode on criterion_group! target order

NIT — EpFactory::initialize names a method that does not exist

Independently confirmed, no change needed

Re-validation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Measured, `int4_decode_loop_ab`, 16-physical / 32-logical host

codecov Bot commented Aug 22, 2026 •

edited

Loading

BLOCKING — `kernels` was never wired up

SHOULD-FIX — coverage rode on `criterion_group!` target order

NIT — `EpFactory::initialize` names a method that does not exist