Repository navigation
feat(cuda-weights): byte-weighted residency hit rate; record the KV/weight budget split, the 2x weight-bytes error, and that no-spill is accounting-only on WDDM - #869
Merged
Conversation
The count-based hit rate weights a 10 KiB norm the same as an 11 MiB projection, so it can improve while the bytes actually streamed get worse. Measured on qwen14b-zp (#857): raising the weight budget moved the count rate 57.09% -> 81.31% while the gap to the streaming floor WIDENED from 1.78x to 2.30x. htod_bytes is what decode pays for, so residency-policy work (#837 item 3) has to be judged on the byte-weighted rate -- which did not exist until now. Adds GLOBAL_HIT_BYTES, accumulated inside record_hit from the resident page's own length, so no call site changes. GlobalOffloadStats gains hit_bytes plus byte_hit_rate(), and profile_native reports both rates side by side. Unit test pins the divergence: 90% by count, under 1% by bytes. Refs #837 #857 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
…error, and the open WDDM question Three facts measured on 2026-08-12 that the single-source doc did not carry: - The weight residency budget is resolved_device_budget minus the FULL declared-context KV reservation, computed once and never renegotiated (#857). On qwen14b-zp that idles 1.611 GB while weights stream 3.94 GB per step. Changing only max_sequence_length 8192->1024 cut htod_bytes_per_token 2.13x with byte-identical tokens. - total_weight_bytes measured file size; qwen14b-zp's external data is 50.0% orphaned prefix, so the runtime believed 2.00x more weights than exist and the model is over budget by 0.599 GB, not ~8.9 GB (#853/#856). Recorded under 'the instrument itself' because arithmetic that could not close is what exposed it -- measured traffic sat BELOW its own floor. - Raising the budget moved hit_rate 57.09%->81.31% while the gap to the streaming floor WIDENED 1.78x->2.30x: a rate over a population with wildly different member costs is not a cost metric. Also qualifies the WDDM section: cuMemCreate cannot fake an allocation, but whether WDDM pages our mapped granules out under demand is unestablished (#862). Until answered, 'committed under the limit' is not proof of device residency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
…ency on WDDM #863 proves single-process that WDDM pages out OUR OWN VMM granules, not just cuMemAlloc: a cuMemCreate+cuMemMap+cuMemSetAccess allocator mirroring the engine arena committed AND touched every byte of 9,984 MiB on an 8,188 MiB card; device-resident stayed capped at ~7,942 MiB while the process host working set reached 9.49 GB. cuMemCreate confers no pin. So peak_committed_physical_bytes < managed_limit_bytes and oversubscribed_bytes == 0 describe our ledger, not physical residency. Every 'no WDDM spill' claim here and in PR validations (including #836's) must be read that way -- the accounting stands, the residency implication does not. The spill is a latency effect, transparent to kernel correctness, and is not the #851 fault. Our residency is advisory on WDDM exactly as the driver's is: we choose what to keep and know the layer walk where the driver is blind, but we cannot force anything to stay. Scoped to WDDM -- under TCC cuMemCreate should fail at the physical limit rather than spill (#783 lesson). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #869 +/- ##
==========================================
+ Coverage 77.65% 77.70% +0.04%
==========================================
Files 351 353 +2
Lines 145472 147642 +2170
Branches 145472 147642 +2170
==========================================
+ Hits 112970 114726 +1756
- Misses 28021 28424 +403
- Partials 4481 4492 +11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
🔴 Benchmark Regression DetectedComparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).
Visual flags: Host infoWhat this cannot catch
|
justinchuby
added a commit
that referenced
this pull request
Aug 13, 2026
…ems (#879) `PROGRESS.md` tracked the dispatch-bound work (models that **fit** device memory) but carried nothing about the other regime — models whose weights **do not** fit — despite that line producing the largest single change today. A reader looking for "what happened to large-model support" would have found nothing, while the answers were scattered across eight issues. ## Added milestone - **#874** — stop auto-enabling managed weight streaming on Windows/WDDM. `8.09 tok/s` with `htod_bytes_per_token = 0` against `0.11` managed; `main` immediately prior measured 0.05–0.08, so **~100× end to end**, byte-identical output. Records the *structural* cause rather than the ratio: each weight is read **exactly once per decode step**, so both paths move the same bytes over the same link, but ours added a CPU memcpy, a VRAM allocation, a `cuMemMap`, an eviction and a synchronize — to buy residency discarded before it is ever re-read. Notes that explicit requests are still honoured, that the force knob is parsed so a *typo* keeps the fast default, and that **Linux is unchanged and must stay so**. - **#866** — elastic weight budget (1.68× fewer bytes, +20.3 points byte-weighted hit rate, max-context reachability tested through the production reclaim path). - **#853/#856** — `total_weight_bytes` was **2.00× too large** (50% orphaned external-data prefix). Records *how it was found*: arithmetic that could not close, with measured traffic below its own theoretical floor. - **#863** — `no-spill` is an accounting guarantee, with the later refinement that it **does** hold physically when solo and under `managed_limit`; spill is specifically the system-wide over-commit case. - **#869** — byte-weighted hit rate, and why the count-based one misleads. - **#877** — hybrid feasibility, including that it **reversed the assumed priority order**: residency is worth ~9.7×, zero-copy is not a bandwidth win. ## Added framing **Two regimes, and wins do not transfer.** Weights fit ⇒ node-dispatch bound (#870). Weights do not fit ⇒ PCIe-streaming bound. Node-count reduction is invisible in the streaming regime; the streaming work is invisible on a model that fits. This is worth stating explicitly because both lines are active and a throughput number from one says nothing about the other. ## Added open items, with measured sizes - **The #864 hybrid**, with its unresolved risk stated plainly: #877's figure is a *sequential* `cuMemcpyDtoD`, not a strided GEMV, and a real kernel could be far worse — which would kill the zero-copy half. Capture compatibility with host-mapped pointers is untested. - **#837 item 3** — 1.97× above the streaming floor, **1.158 GB/step recoverable**, ~91 ms/step at measured bandwidths; `bypassed_page_ins` still unattributed. - **#750 batching** — with the condition stage 1 established: it amortizes **only** as one fused forward with `M = N`. - **#851** — parked with the three eliminated hypotheses and what unblocks it, so nobody repeats the search. Docs only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
justinchuby
added a commit
that referenced
this pull request
Aug 14, 2026
…o retract (#911) Several confident performance claims on this project have been retracted, **including two by me on the same day**. Every one failed the same way: a number was interpreted before the conditions that produced it were established. The corrections were each worth more than the original claims, but they cost real time, and they are avoidable. There are skills for test discipline and error recovery, but nothing about making a *measurement* trustworthy — which is where the actual damage has been. ## What it records Eight failure modes, each with the incident behind it, because the abstract rule is forgettable and the incident is not: 1. **A knob you set produced the number you then interpreted.** #834 concluded the engine commits KV eagerly from `committed_len == capacity`, when the harness had set `min_bucket = capacity` and `kv_capacity_bucket` is `len.next_power_of_two().max(min_bucket).min(hard_max)`. The knob forced the observation; at the default bucket the engine commits on demand — the opposite conclusion. 2. **The instrument measured a real thing that was not the thing named.** `h2d_enqueue_copy_ms` reported 1.7% and retired a line of work; it bracketed an async enqueue returning, not a transfer completing. Properly measured: 18.8%. 3. **Arithmetic that cannot close.** `total_weight_bytes` was 2.00× too large (#853), exposed because measured traffic sat *below* its own theoretical floor. 4. **A rate over a population with wildly different member costs.** `hit_rate` rose 57.09% → 81.31% while the gap to the floor *widened* 1.78× → 2.30× (#869). 5. **Contention you did not know was there.** I filed a flaky-gate issue and retracted it — two contending loops were mine (#851). The strict re-run was 8/8. 6. **Wall-clock on a box that pages your own memory** — 3.9–28 tok/s across identical configurations (#863). 7. **A proxy standing in for the real access pattern** — 11.41 GB/s sequential proxy vs ~5.6 GB/s real strided GEMV (#877 → #880). 8. **A precondition observed rather than established** — a test that hoped the allocator would reuse an address, green on glibc and vacuous on Windows (#906). Each carries the check that catches it, and the corollary I want most: **build a control that could falsify you.** #891 measured `1/N` amortization *and* showed it identical at `past_len` 0/512/2048, ruling out a KV-pressure artifact — a result that survives a control it could have failed is worth far more than one that merely looks good. ## Also recorded **The reporting rules that made the good results good** — state the conditions with the number; report a ceiling, not just a gain (#891's `N_max ≈ 19 @ 2048 ctx`, #901's ~10% bound on eviction-order tuning, both of which stopped someone spending a week for a tenth of the prize); treat a truthful negative as first-class; and say when a favourable result rests on one measurement — #901 measured MRU as better and still recommended keeping LRU, which is the harder and more useful call. **The correctness constraints that outrank performance**, led by byte-identical token IDs — the check that caught a residency policy silently collapsing generation 16 → 3 tokens with no error at all (#886). Every issue and PR number cited was verified to exist before committing, which felt like the minimum for a document about not asserting things you have not checked. Docs only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
justinchuby
added a commit
that referenced
this pull request
Aug 14, 2026
…he host-mapped cold recommendation (#915) Review pass on #907 before it goes to the wider ORT stack, with the disagreements applied directly rather than left as comments. The structure holds — contracts/invariants/flows is the right shape, the responsibility boundaries are clean, and the appendix's evidence table is the single best thing in the document because it lets a reader tell measurement from intent. My changes are where the document is either **missing a decision** or **recommending something we have since measured to fail**. ## The one I disagree with most The appendix told readers to prefer *"a resident hot set plus host-mapped cold, single-touch weights over copy-map-evict churn."* We built exactly that this week, and it lost twice (#912): - **Correctness.** Above ~0.44–0.65 GB of *distinct* host-mapped bytes read per decode step, the device silently returns stale data — generation collapsed 16 tokens to 3 with no error raised. A single host-mapped read is bit-identical at 1/8/16/32 cold weights, and a copy-instead A/B isolates the fault to the read, so it is an aggregate aperture ceiling, not a per-read bug. - **Throughput.** Even capped at a provably safe budget, 0.73 tok/s against WDDM demand paging's 7.84 on the same model in the same session. The safe aperture (0.26 GB/step) is smaller than the traffic it must displace (~0.6 GB/step) — too short by construction, not by tuning. Shipping a design doc that recommends this to other teams would have cost each of them the same week. Replaced with the narrower claim the evidence supports, and I kept what survived: the mechanism works and capture replays bit-identically, so this is a capacity verdict about one class of GPU, not a dead technique. Datacenter parts should be re-measured, not assumed to inherit it in either direction. ## The question the document never asked Every contract describes *how* to manage memory. None asks **whether managing a given class of bytes beats leaving it to the OS** — and the honest answer is sometimes it does not. That is not a caveat; it decides when the rest of the design pays for itself. The rule: **copying into device memory only pays when the data is re-read from device memory before eviction.** A weight read exactly once per decode step has no reuse to amortize, so we pay the same PCIe bytes as the OS *plus* a host copy, an allocation, a mapping, an eviction and a sync. Measured 30× slower than WDDM demand paging (#864); deferring recovered ~100× end-to-end (#874). We were not losing to a better algorithm — we were losing to not being in the path. Added as a decision table plus two obligations that are contracts, not advice: **a governor must be able to decline and report it** (that is what I3 exists for), and **the unmanaged path must stay runnable as a control**, since an A/B arm is the only thing that can tell you the control plane costs more than it saves. It also connects batching to memory management properly — batch-N is what *creates* the reuse that makes managing pay, which is why they are one lever and not two features (#884/#891, ceiling `N_max ≈ 19 @ 2048 ctx`). ## Corrections from measurements this design would otherwise repeat - **C7** must report host-mapped read capacity and resident-vs-host bandwidth — a tiering decision needs both and can infer neither. A platform that cannot report the first is treated as **zero**, not unbounded. - **I8b (new): an authority accounts charge, not residency.** On WDDM the two diverge with no notification (#863). A design that treats the ledger as physical truth is correct on TCC and quietly wrong on every consumer Windows machine. C8's numbers are now labelled accordingly, and residency claims must name their measurement source. - **I8c (new): reservations are elastic against occupancy.** A load-time `bytes_per_token × max_context` split is charged in full from the first token; making it elastic cut streamed bytes 1.68× with the max-context guarantee tested through the production reclaim path (#857/#866). - **I8**: bytes are *referenced extents*, not file or blob size — that error overstated one model by exactly 2.00× and made every derived budget wrong (#853/#856). - **C8**: policy rates must be byte-weighted. A count-based hit rate rose 57.09% → 81.31% while the byte gap *widened* 1.78× → 2.30× (#869). - **Risk table**: do not spend the dynamic-lending fix on eviction policy. Order is ruled out as the cause (#892) and bounded at ~10% of the recoverable gap (#901); ~90% is **admission** — 11% of events but 44.6% of streamed bytes. Without this a reader would reasonably spend a week on the wrong lever. ## What I deliberately did not change The Foundry/cluster layering, the transaction ordering rules, and the PagedAttention-as-a-view position all read correctly to me and I have no measurement that contradicts them. The `ClusterCoordinator` → `ServingMemoryCoordinator` split in "Relationship to existing design" is the right call. Every issue and PR number cited was verified to exist **and** to match the figure attributed to it. Docs only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
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.
Two things, both consequences of measurements taken today.
1. A byte-weighted residency hit rate (#837 item 3)
The count-based
hit_rateweights a 10 KiB norm the same as an 11 MiB projection. Measured onqwen14b-zp(#857): raising the weight budget moved the count rate 57.09% → 81.31% while the gap to the streaming floor widened from 1.78× to 2.30×. Misses skew large (~11.9 MB average page-in), hits skew small, so the count rate can improve while the bytes actually streamed get worse.htod_bytesis what decode pays for, so residency-policy work needs a byte-weighted rate — which did not exist.GLOBAL_HIT_BYTES, accumulated insiderecord_hitfrom the resident page's ownlen, so no call site changes.GlobalOffloadStats::hit_bytesandbyte_hit_rate().profile_nativereports both rates side by side.Verified on the real model:
hit_rate=57.09% byte_hit_rate=49.90%— the divergence is real, not hypothetical.Unit test pins the failure mode it exists to catch: nine 10 KiB hits against one 12 MiB miss is 90% by count, under 1% by bytes.
2. Three corrections to the single-source doc
The weight/KV budget split (#857). The weight residency budget is
resolved_device_budget − kv_bytes_per_token × max_context, computed once at load and never renegotiated.max_contextis the model's declared maximum. Onqwen14b-zpthat idles 1.611 GB holding KV a 16-token run never commits (measured committed KV: 402,653,184 bytes) while weights stream 3.94 GB per decode step. Changing onlymax_sequence_length8192 → 1024 — same weights (hard-linked, byte-identical), same binary, same prompt — cuthtod_bytes_per_token2.13× with byte-identical tokens. Recorded with the caveat that shipping a smaller declared context is not the fix.total_weight_byteswas 2.00× too large (#853/#856). Filed under "the instrument itself", because what exposed it was arithmetic that could not close: measured traffic sat below the theoretical floor for a full per-step scan, which is impossible. Same habit as dividing bytes by time — compute the bound the number must obey, and when it is violated, suspect the instrument first.no-spillis an accounting guarantee, not physical residency (#863). Proven single-process: acuMemCreate+cuMemMap+cuMemSetAccessallocator mirroring the engine arena committed and touched every byte of 9,984 MiB on an 8,188 MiB card; device-resident stayed capped at ~7,942 MiB while the process host working set reached 9.49 GB.cuMemCreateconfers no pin — WDDM pages out our own granules, not justcuMemAlloc.So
peak_committed_physical_bytes < managed_limit_bytesandoversubscribed_bytes == 0describe our ledger, not bytes physically on the device. Every "no WDDM spill" claim in the doc and in PR validations — including my own on #836 — must be read that way. The accounting stands; the residency implication does not. It is a latency effect, transparent to kernel correctness, and it is not the #851 fault.Scoped explicitly to WDDM: under TCC,
cuMemCreateshould fail at the physical limit rather than spill, so this must not propagate to other platforms (the #783 lesson).Validation
cargo fmt -p onnx-runtime-ep-cuda -p onnx-genai-bench -- --check— passcargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings— pass (exact CI gate)cargo test -p onnx-runtime-ep-cuda --features cuda --lib byte_hit_rate— 2 passedqwen14b-zp, GPU verified idle beforehandDocs are documentation-only; the counter is additive and does not change any decision path.
Refs #837 #857 #853 #856 #863 #862.