Skip to content

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

Merged
justinchuby merged 2 commits into
mainfrom
seb/bench-report-decode-width
Aug 22, 2026
Merged

justinchuby merged 2 commits into
mainfrom
seb/bench-report-decode-width

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

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

`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.
At least four paths silently reduce it 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 entirely --
and only one is even `NXRT_CALIB_DEBUG`-visible. A `t=N` row was a label, not a
measurement of width N.

That is not hypothetical. Before #1766 the benches never called
`initialize()`, so the process was unbounded and the global Rayon pool ran full
width at every budget. Measured at 9747b49 (current main minus #1766),
qwen/acc0/block32/s=1, same binary otherwise:

    t=1  40.298 ms/token  100% CPU
    t=2  37.166 ms/token  267% CPU   <- 1.08x for 2.67 cores

That flat bottom end is what "ONNX_GENAI_CPU_DECODE_THREADS=2 does nothing"
was: not a dispatch pathology, an unbounded-topology artifact. On current main
the same sweep scales 1.98x / 3.91x / 7.55x / 15.0x at t=2/4/8/16, reproducible
within 2% sweeping in both directions.

Reports rather than asserts. A reduced width is legitimate when the host
genuinely cannot honour the request (an 8-lane budget in a 2-CPU cpuset), and
aborting there would leave a constrained container unable to benchmark at all.
The `WIDTH-MISMATCH` token is for the caller -- human or matrix script -- 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, so reading it earlier reports
`path=unresolved` -- and if it were forcing it 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.

Refs #1763, #1766

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Aug 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.58%. Comparing base (11cb8e5) to head (0471e72).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1770      +/-   ##
==========================================
- Coverage   80.67%   80.58%   -0.10%     
==========================================
  Files         411      411              
  Lines      199008   199008              
  Branches   199008   199008              
==========================================
- Hits       160551   160366     -185     
- Misses      33016    33201     +185     
  Partials     5441     5441              
Flag Coverage Δ
cli-ort-linux 72.47% <ø> (ø)
cli-ort-windows 72.06% <ø> (+0.09%) ⬆️
mlas 85.33% <ø> (+0.09%) ⬆️
offline 80.72% <ø> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
crates/onnx-runtime-ep-cpu/src/decode_spmd.rs 88.87% <ø> (ø)

... and 8 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.

…d verdict

Adversarial review caught a claim in the new doc that my own measurement in
this branch had already refuted: `reserve_single_group_headroom` does not
reduce the realized width. It reduces the *spawned thread* count, but it only
runs in the single-group case, which is exactly where `dispatcher_owns_a_shard`
(`shards.len() == 1 && shards[0].workers < requested`) is true, so
`total_workers` adds the lane straight back. Measured: a 2-lane budget on a
2-CPU cpuset spawns one thread and realizes two lanes, `as_requested`.

The genuine net reducers are three, not four: the pre-clamp to
`available_parallelism`, `reserve_split_headroom` (NUMA-split only, and
uncompensated because the dispatcher shard is single-group-only), and the
single-CPU-cpuset fallback.

Also corrects the same list in the shipped `DecodeWidth` doc from #1764, which
had the mirror-image error: it named both headroom reservations, omitted the
pre-clamp, and claimed all three report through `report_spmd_fallback`. Only
the cpuset fallback does -- neither headroom function logs anything at all
(call sites are :569, :1619, :1642, and neither :1749 nor :1763 is among them).

Splits `WIDTH-MISMATCH` into `WIDTH-MISMATCH` (both widths known, unequal --
the row's label is wrong) and `WIDTH-UNRESOLVED` (never resolved -- no decode
reached the pool). A matrix script wants to treat those differently.

Verified by execution, all three verdicts:
  t=4 plain                   -> requested=4 realized=4 spmd-pool as_requested
  t=8 under taskset -c 0,2    -> requested=8 realized=2 spmd-pool WIDTH-MISMATCH
  t=1 plain                   -> requested=1 realized=1 flat      as_requested

The mismatch arm is the pre-clamp reducer caught in the act.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby

Copy link
Copy Markdown
Owner Author

Adversarial review (Opus) — verdict and fixes

No BLOCKING findings. One NIT was a genuine defect and is fixed, plus a mirror-image error it exposed in already-shipped code.

The real finding: my doc contradicted my own measurement

The doc claimed reserve_single_group_headroom silently reduces the realized width. It does not. It reduces the spawned thread count, but it only runs in the single-group case — which is exactly where dispatcher_owns_a_shard (shards.len() == 1 && shards[0].workers < requested) is true, so total_workers = total_threads + dispatcher_shard.is_some() adds the lane straight back.

I had already measured this in this branch before writing the sentence:

CONFINE  BUDGET | allowed nodes spawned lanes requested realized path
2        2      | 2       1     1       2     2         2        spmd-pool

One spawned thread, two realized lanes, as_requested. I inherited the (2,2) → 1 framing from a handover note, verified it was wrong, and then repeated it in a doc comment anyway. Same failure mode as the two I self-caught on #1764/#1766 — a summary restated instead of re-derived.

Genuine net reducers are three: the pre-clamp to available_parallelism, reserve_split_headroom (NUMA-split only, uncompensated because the dispatcher shard is single-group-only), and the single-CPU-cpuset fallback.

Mirror-image error in shipped #1764

The review's cross-check exposed the same list wrong in the opposite direction in DecodeWidth's own doc: it named both headroom reservations, omitted the pre-clamp, and claimed all three report via report_spmd_fallback. Only the cpuset fallback does — call sites are :569, :1619, :1642; neither :1749 nor :1763 is among them. Neither headroom function logs anything at all, which is worse than the doc said. Corrected here.

Also fixed

Split the failure token, since a matrix script wants to treat these differently:

  • WIDTH-MISMATCH — both widths known and unequal; the row's label is wrong
  • WIDTH-UNRESOLVED — never resolved; no decode reached the persistent pool

Verified by execution, all three verdicts

t=4 plain                -> requested=4 realized=4 path=spmd-pool as_requested
t=8 under taskset -c 0,2 -> requested=8 realized=2 path=spmd-pool WIDTH-MISMATCH
t=1 plain                -> requested=1 realized=1 path=flat      as_requested

The mismatch arm is the pre-clamp reducer caught in the act — precisely the silent path that made a t=N row a label.

Reviewer questions answered

  • Placement — the call is the last statement of main(), after both phases. The pool is process-wide and immutable after first build, and the width sweep is external (one process per budget), so one report is representative of every row in that process.
  • PROBE_REPS=0 / PROBE_TOKENS=0 panic in median of an empty slice — pre-existing, and they abort before the report, so nothing misleading is printed.
  • Multi-session — on the persistent path realized is a fixed process-wide layout independent of the reading thread, so a main-thread read after joins is exactly correct for any PROBE_SESSIONS. Thread-dependence exists only on the flat branch, which the field doc defines and the path=flat token flags.
  • #![allow(dead_code)] in common/ — a future decode bench that forgets to call this gets no warning. Accepted: consistent with init_decode_topology, and inlining would block reuse.

Gates

fmt clean · clippy --all-targets -D warnings clean · aarch64-unknown-linux-gnu clippy clean · 1628 EP lib tests pass · all 12 benches build · rustdoc: my links resolve (pre-existing warnings in kernels/identity.rs/add.rs untouched)

@justinchuby
justinchuby marked this pull request as ready for review August 22, 2026 19:35
@justinchuby
justinchuby enabled auto-merge (squash) August 22, 2026 19:35
@justinchuby
justinchuby merged commit 4738086 into main Aug 22, 2026
20 checks passed
@justinchuby
justinchuby deleted the seb/bench-report-decode-width branch August 22, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant