test(cpu): pin the fixed point every benchmark pin relies on, and scope #1729

[justinchuby]

Why

A cross-agent note asked for every t>=8 row published before #1729 (6e8c31ebd) to be re-taken, on the grounds that the default decode pool pinned 16 workers to cpus 0–15 — 8 physical cores with both SMT siblings loaded.

The premise is correct, and it is the same defect this repo found independently and filed as #1680 (§24 of CPU_MATMUL_ASSIGNMENT.md). The blanket conclusion is too broad for the rows in that file, and — the point of this PR — which rows survive is decidable from the source, without spending host time re-measuring.

The argument

Every multi-thread timing in §23/§25 is taskset-pinned to the even CPUs (§24's closing note). On this host SMT siblings are adjacent pairs, so that mask is 16 CPUs that are already 16 distinct physical cores.

The spread half of #1729 is the identity on such a mask. Pre-#1729 the SPMD shard builder used allowed_cpus() in raw ascending order (confirmed at 6e8c31ebd^); post-#1729 the same list goes through order_pin_targets. Its Spread arm is leaders_within(cpus) plus the non-leader remainder — and on an all-even mask each core group contributes its single allowed member, the remainder is empty, and leaders_within ends sort_unstable(); dedup(). Same ascending list. build_decode_pool pins worker i to cpus[i % len] either way, so #1729 cannot move a number taken under that pin.

The reserve half is the identity on that mask too. This is a correction to the first version of this PR, which claimed t=16 went 16 workers -> 15. It does not. #1729 did not introduce the dispatcher reserve; it changed it from a logical-CPU rule to a physical-core rule:

pre  (6e8c31ebd^):  total < allowed ? total : allowed - 1
post (core budget): min(total, cores - 1).max(1)

When the mask holds one CPU per physical core, allowed == cores, and the two agree at every width: below saturation min(total, allowed-1) == total because total <= allowed-1; at and above saturation both give allowed-1. They diverge only when allowed > cores — a mask holding both SMT siblings, which is exactly the unpinned case #1729 was written to fix and exactly the case these benchmarks are not run in.

I got this wrong in the characteristic way: I read the post-#1729 formula carefully and assumed the pre-#1729 one, then published a delta with only one side measured. An adversarial review accepted the wrong table; fetching 6e8c31ebd^ is what caught it. It is in the ledger under its own heading because it is the same error class the rest of this file is about.

#1794 does not apply. It fixed default_persistent_threads returning available / 2. These sweeps set ONNX_GENAI_CPU_DECODE_THREADS explicitly, and an explicit count bypasses the default entirely. The rows that defect corrupted are pinned runs that left the width to the default — of which this file has none, because the pin and the explicit width were adopted together.

Disposition: no row taken under the even-CPU pin moves at any width — both halves of #1729 are the identity there, and #1794's defect lived in a default these sweeps never used. t=16 stays withheld for its own unrelated reason (its A/A null spans 0.969–1.295, ±30%, against 3.6% at t=1). The placement question and the instrument question are independent, and only the second ever disqualified that row.

This is a stronger claim than the one asked for, so it carries a stronger obligation: it holds because of the pin, and says nothing about an unpinned row. Unpinned numbers remain governed by §24's standing rule and are not rehabilitated by anything here.

The tests, and why they are the real content

Both arguments were load-bearing and untested. cargo test order_pin_targets matched zero tests, and nothing asserted the reserve invariant either.

1. a_one_cpu_per_core_mask_is_unchanged_by_either_placement_policy — a cpuset already holding one CPU per physical core is a fixed point of both placement policies. This is the guarantee every pinned benchmark in this repository rests on: the house rule for a clean multi-thread number is taskset to one CPU per core, and it is worth nothing unless the pool then pins workers to the CPUs that were reserved, in the order reserved. Also asserts the policies still disagree on a full mask, so it is a property of the mask rather than a policy that never reorders.

Live: reversing the leader order inside Spread fails it with `spread` reordered a mask that was already one CPU per core.

2. the_core_reserve_matches_the_logical_reserve_on_a_one_cpu_per_core_mask — carries the pre-#1729 logical rule as a reference implementation and asserts agreement across masks 1..32 at every total, plus the contrasting SMT-mask case where the rules genuinely differ ((16, 32, 16) -> 15 vs logical 16). A future change to the reserve that breaks this would silently invalidate a file full of measurements; this makes it break in CI instead.

Live: reserving two CPUs instead of one fails it at total=3 on a 4-CPU mask.

Also recorded

t=2 is closed, by two methods sharing no apparatus: 1.96x measured here on a quiet host (20.447 vs 40.039 ms/token, 0.6% A/A null, both workers 99% busy by per-thread attribution) and 1.94x / 97% efficiency reported independently by the runtime owner on a post-#1729 baseline. ~1% apart. The withdrawn "71% of one core" is now over-determined.

One standing caveat is reinforced rather than revised: t=1 runs path=flat and t>=2 runs path=spmd-pool, so a t=1 vs t=2 comparison crosses routes as well as widths. §20 already reads that row as "vs serial" rather than "vs a one-worker pool".

Validation

• cargo test -p onnx-runtime-ep-cpu --lib — 1829 passed, 0 failed (two new tests), under scripts/hostlock.sh
• cargo clippy -p onnx-runtime-ep-cpu --all-targets --all-features -- -D warnings — clean
• cargo fmt --all --check — clean

No production code changed: two tests plus a documentation section.

[codecov]

Codecov Report

❌ Patch coverage is 92.10526% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.55%. Comparing base (f55520d) to head (0b0f6a6).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/onnx-runtime-ep-cpu/src/decode_spmd.rs	91.30%	2 Missing ⚠️
crates/onnx-runtime-ep-cpu/src/decode_affinity.rs	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           main    #2150       +/-   ##
=========================================
+ Coverage      0   80.55%   +80.55%     
=========================================
  Files         0      431      +431     
  Lines         0   217593   +217593     
  Branches      0   217593   +217593     
=========================================
+ Hits          0   175291   +175291     
- Misses        0    36473    +36473     
- Partials      0     5829     +5829     

Flag	Coverage Δ
cli-ort-linux	72.51% <ø> (?)
cli-ort-windows	72.01% <ø> (?)
mlas	85.90% <ø> (?)
offline	80.67% <92.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_affinity.rs	94.63% <93.33%> (ø)
crates/onnx-runtime-ep-cpu/src/decode_spmd.rs	90.50% <91.30%> (ø)

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

[justinchuby]

Review accepted my table, and the table was wrong. Recording that, because it matters more than the parts that were right.

Chasing your Low finding (the "workers" column vs effective lanes under budget narrowing) sent me back into the reserve, and there I found something neither of us had checked: the pre-#1729 formula. I had read the post-#1729 one carefully and assumed the old one reserved nothing at saturation. It did:

pre  (6e8c31ebd^):  total < allowed ? total : allowed - 1
post (core budget): min(total, cores - 1).max(1)

#1729 did not introduce the dispatcher reserve. It changed it from a logical-CPU rule to a physical-core rule. On a one-CPU-per-core mask allowed == cores, and the two agree at every width — below saturation min(total, allowed-1) == total because total <= allowed-1; at and above saturation both give allowed-1. They diverge only when allowed > cores, i.e. a mask holding both SMT siblings: exactly the unpinned case #1729 was written to fix, and exactly the case these benchmarks are not run in.

So t=16 does not go 16 -> 15 under this pin. It was 15 both before and after. My claimed disposition was "t<=8 survives, t=16 does not"; the correct one is no row taken under the even-CPU pin moves at any width.

I published a delta with only one side measured. That is the exact error class this PR's ledger section is about, committed in the section itself, which is why it is now written up there under its own heading rather than quietly fixed.

Your Low finding was right and is what led me there, so it is worth being precise about where it lands. Budget narrowing does apply — with explicit THREADS=N and no DECODE_AFFINITY, bound_process_to_decode_budget confines the process to N CPUs, so allowed == N. Then reserve_single_group_headroom(N, N, N) = N-1, and dispatcher_owns_a_shard(N-1 < N) is true, so the structure is N-1 pinned workers + 1 dispatcher shard = N lanes. Your correction to the column header stands. What changes is that this is true identically on both sides of #1729, so it does not disturb the conclusion — it sharpens what the conclusion is about.

New in this revision:

the_core_reserve_matches_the_logical_reserve_on_a_one_cpu_per_core_mask — carries the pre-#1729 logical rule as a reference implementation and asserts agreement across masks 1..32 at every total, plus the contrasting SMT-mask case where the rules genuinely differ ((16, 32, 16) -> 15 against logical 16). Without that second half the test would also pass on a reserve that had simply stopped distinguishing the two masks.

Live: reserving two CPUs instead of one fails it at total=3 on a 4-CPU mask.

Re-validated: 1829/0, clippy -D warnings clean, fmt --check clean, full suite under scripts/hostlock.sh.

Two follow-ups on your other items, both agreed and neither changed:

• The redundancy you flagged between my assert_ne! on a full mask and compact_and_spread_are_permutations_that_differ_on_an_smt_host is real but load-bearing in situ — without it the fixed-point assertion would also pass on a policy that never reorders anything. Kept, with the reason in the comment.
• Item 7 came back clean from you and I have nothing to add to it.

Requesting a second pass, specifically on the corrected reserve argument and the new test — the first pass validated the version of this that was wrong.