Repository navigation
perf(bench): read decode-worker placement out of /proc instead of trusting the width label - #1937
Merged
Merged
Conversation
…sting the width label A cross-agent report had the default decode pool pinning 16 workers to cpus 0-15 -- two workers per physical core on a single 32 MiB L3, half the machine idle -- and offered #1729 as the fix. #1729 merged as `6e8c31ebd` on 2026-08-23, and the related width-halving fix #1794 (`0652fdd2e`) the same night; both are ancestors of main. The report describes a pre-#1729 build. Rather than argue from the diff, this adds the categorical instrument that settles it. `decode_placement_census.sh` reads `Cpus_allowed_list` for every `onnx-genai-spmd` thread in three configurations. It is a /proc read, not a benchmark: it does not need a quiet host and no number in it is a timing. On `0a668d54b`, reproduced identically three times: default, no taskset, no env 15 workers on 0,2,...,28 8 x L3#0 + 7 x L3#1 THREADS=16 under even mask 15 workers on 0,2,...,28 8 x L3#0 + 7 x L3#1 THREADS=8 under even mask 7 workers on 0,2,...,12 7 x L3#0 One worker per physical core in every case, with the reserved dispatcher CPU (30 at width 16, 14 at width 8) left clear -- what `order_pin_targets` and `reserve_single_group_headroom` specify. Checking it established something about the acc0 scaling result that was not written down, so both records now say it: `THREADS=8` confines the process to [0,2,4,6,8,10,12,14], entirely inside one L3 instance, while width 16 spans both. The `t=8 -> t=16` doubling on this host doubles cache and memory-controller reach as well as cores. That is not a confound -- `ort()` defaults its pin to `native_pin(threads)`, so both arms get the same CPUs at each width, and both the 1.762x and 1.319x figures post-date that fix (`4b4dacc7e`) -- but it does make the 2.0x ideal a conservative reference for both arms, which leaves the finding understated rather than overstated. Also adds `cpu_work_probe.py` and a second reason not to read `Percent of CPU` as utilisation here: on an SMT host a logical CPU whose sibling is busy is granted a full 100% share while delivering roughly half the work, and no CPU-time instrument can see that. Only a work-completed probe distinguishes them. The permanent cpu-0 competitor reported alongside the placement claim did not reproduce -- cpu 0 reads cpu_share 0.999-1.000 inside the band spanned by ten other CPUs -- so on a shared host that was load, not topology. The instrument point is the durable part and is what is recorded. Docs and bench scripts only; no compiled code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
enabled auto-merge (squash)
August 24, 2026 04:46
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1937 +/- ##
==========================================
+ Coverage 80.14% 80.96% +0.81%
==========================================
Files 413 416 +3
Lines 201044 205410 +4366
Branches 201044 205410 +4366
==========================================
+ Hits 161132 166306 +5174
+ Misses 34388 33499 -889
- Partials 5524 5605 +81
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A cross-agent placement report had the default decode pool pinning 16 workers to cpus 0–15 — two workers per physical core on a single 32 MiB L3, half the machine idle — and offered #1729 as the fix.
#1729 is merged (
6e8c31ebd, 2026-08-23T01:11:35Z), as is the related width-halving fix #1794 (0652fdd2e). Both are ancestors oforigin/main. The report describes a pre-#1729 build.Rather than argue from the diff, this adds the categorical instrument that settles it.
benches/decode_placement_census.shReads
Cpus_allowed_listfor everyonnx-genai-spmdthread in three configurations. It is a/procread, not a benchmark — it does not need a quiet host and no number in it is a timing. It still takes the hostlock as a courtesy, since it does spin the pool.Measured on
0a668d54b, reproduced identically three times:taskset, no env0,2,4,…,28THREADS=16under the even mask0,2,4,…,28THREADS=8under the even mask0,2,…,12One worker per physical core in every case, with the reserved dispatcher CPU (
30at width 16,14at width 8) left clear — exactly whatdecode_affinity::order_pin_targetsandreserve_single_group_headroomspecify.The tell was visible in the original report without any re-measurement: its two arms reported 16 and 15 shard participants, and 15 spawned workers plus an inline dispatcher is precisely what current main builds.
What it turned up in my own record
ONNX_GENAI_CPU_DECODE_THREADS=8confines the process to[0,2,4,6,8,10,12,14]— entirely inside one 32 MiB L3 instance — while width 16 spans both. Sot=8 → t=16on this host doubles cache and memory-controller reach as well as cores. That was not written down anywhere and now is, in both the benchmark record and the ledger.It is not a confound.
acc0_gap_matrix.ort()defaults its pin tonative_pin(threads), so both arms get the same CPUs at each width, and both the 1.762x (ORT) and 1.319x (native) scaling figures post-date that fix (4b4dacc7e). It does make the 2.0x ideal a conservative reference for both arms, which leaves the finding understated rather than overstated.benches/cpu_work_probe.pyA second, independent reason not to read
/usr/bin/time -v'sPercent of CPUas utilisation here, beyond its being wall-derived: on an SMT host a logical CPU whose sibling is busy is granted a full 100% share while delivering roughly half the work, and no CPU-time instrument can see it — the scheduler really is handing over the CPU; the contention is in hardware, below its view. Only a work-completed probe distinguishes them.The permanent cpu-0 competitor reported alongside the placement claim did not reproduce: cpu 0 reads
cpu_share0.999–1.000 at 9429/9482/9489 iterations, inside the 8744–9499 band spanned by ten other CPUs, with one transient outlier that two re-probes cleared. On a host shared by several agents, "permanent" was load rather than topology. The instrument point is the durable part and is what the ledger records.Scope
Docs and bench scripts only — no compiled code changes.
shellcheckclean,ruffclean.