Repository navigation
Back weight offload pages with stable VMM slots - #733
justinchuby wants to merge 1 commit into
Conversation
Stage 1 for #716: add an opt-in CUDA VMM-backed weight residency path that preserves each weight key's virtual address across eviction and page-in without relaxing graph capture yet. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a41b279 to
34ecb06
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #733 +/- ##
==========================================
- Coverage 80.85% 80.13% -0.73%
==========================================
Files 331 331
Lines 132743 132800 +57
Branches 132743 132800 +57
==========================================
- Hits 107330 106414 -916
- Misses 21168 22143 +975
+ Partials 4245 4243 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
✅ Benchmarks — No RegressionComparison 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
|
|
Follow-up on Option A vs B: I tried the contained shared-reservation version locally: keep the existing VMM arena as the one reservation, allocate each stable weight slot at packed offsets instead of rounding each slot to 2 MiB, and fix range decommit so a decommit releases any overlapping granule only when all touching slots are gone. That removes the per-weight padding shape, but the real decode loop still fails under the one-ledger constraint before producing counters. The reason is the #723 policy's zero-eviction behavior: once the stable resident subset fills the 6 GB residency budget, misses outside that subset are transient bypass pages. The non-VMM path can Evicting resident weights to make room makes the run complete, but it is exactly the regression we measured: hit rate drops and evictions become non-zero. Padding is not the only problem; transient bypass headroom also needs a unified reservation/budget owner that can coordinate weights with KV/other zones. So my judgement is Option B: keep #733 as evidence/do-not-merge and let #736 subsume it. |
|
Correction to my previous Option-A framing: the VMM arena already has the shared reservation, byte-granularity I tried the contained fix locally (packed stable weight slots inside the existing arena). With extra headroom ( But under the original tight limit it still fails before counters: VMM needs to lease a 2 MiB committed granule when only ~742 KB is left in the device ledger. So the remaining blocker is not padding; it is transient/overhead headroom under the one-ledger constraint. The non-VMM path can temporarily So I am keeping #733 draft/do-not-merge and treating this as Option B for the original acceptance bar. The useful conclusion for #736 is sharper now: reuse the existing shared suballocator, but make the unified budget owner reserve/coordinate transient headroom across zones instead of letting weights/KV independently fill their local budgets. |
|
Committed-byte admission probe result: I added the contained arena preflight query ( Packed/shared with extra headroom proves the design is neutral at the policy level: Conclusion: the preflight query is contained, but the needed budget/headroom policy is not. #733 stays draft/do-not-merge; #736 should own the unified committed-byte budget decision across weights/KV/scratch. |
|
Closing this draft as superseded by #736, preserving the branch and measurements. What it proved:
The conclusion is not that stable VA is wrong. It is that it becomes a speed win only with the shared physical authority/pool and dynamic zone lending in #736. Landing a separate opt-in path now would duplicate ownership and preserve the wrong budget unit. The stable-pointer mechanism is retained in this branch and #727 proves graph replay survives remap. #716 remains open for capture+offload after #736's pool/committed-byte admission lands. |
…capture (#716) (#796) ## Summary Weight offload previously forced whole-step CUDA graph capture **OFF**: the pager returned a different device pointer on every page-in (`alloc_raw`/`free_raw`), and a captured graph bakes pointers into its recorded nodes, so the two were mutually exclusive (`resolve_graph_capture_enabled` returned `false` whenever offload was on). The models that most need offload — those that do not fit in VRAM — were exactly the ones that lost graph capture and all of the #708/#728 capture-fragmentation wins (35B-A3B decode 154 -> 34 segments). This PR gives each retained weight `key` a **reserved-once device virtual address** and remaps physical granules underneath on page-in/eviction, so a captured graph replays correctly across evict->repage. It then removes the offload/capture mutual exclusion, **gated on the paging path actually being stable-VA** (the managed no-spill authority). Hard prerequisite for #755 (managed no-spill VMM as the default with automatic weight streaming). ## What changed, by step **Step 1 - prove the mechanism in isolation first** (`crates/onnx-runtime-cuda-memory/tests/vmm_stable_va_weight_slot_gpu.rs`, **passes on this HW**): a captured graph that reads from a stable VA still produces correct results after the pages under that VA are swapped for different physical granules holding **different weight values**. Extends the #727 remap-at-same-VA proof to the weight-value-swap case. **Step 2 - stable-VA slot conversion** (`crates/onnx-runtime-ep-cuda/src/weight_paging.rs`): - Per-key `StableWeightSlot { va, len }` reserved once through the **existing #740 authority-scoped pool** via `allocate_committed`/`carve()` suballocation. No second allocator, no per-weight reservation (the mistake that made #733 net-negative). Charging stays on the global granule-ref **0->1 / 1->0** attribution. - `CudaWeightPage::Drop` decommits granules but **keeps** the VA for stable slots (`deallocate_span` only for transient bypass pages). Never asserts in `Drop`. - Respects committed-granule admission (#745): reserving VA is free; only committed physical bytes are charged. 2 MiB granule sizing per #776. - **Enforceable ordering rule (not a comment):** page-ins - and thus eviction-driven `decommit` of any stable slot - run entirely under the residency mutex; capture is capture-once/replay-many with no page-ins during replay; the engine additionally declines capture when a step reports a bypass/eviction (`GLOBAL_BYPASSED_PAGE_INS`). So no `decommit` of a baked VA can occur while a replay is in flight - the case #727 explicitly did **not** prove safe. No `cuMemMap`/grow inside a captured region. - Residency GPU test `vmm_retained_weight_key_keeps_a_stable_virtual_address_across_repage` proves the VA is identical across evict->repage with one reused granule and **no accounting regression**. The existing `vmm_weight_admission_reuses_owned_granules_without_runtime_alloc_free` and `weight_offload_gpu` suite still pass (no regression). **Step 3 - remove the exclusion, gated on stable-VA** (`native_decode/{cuda,mod,load}.rs`): `resolve_graph_capture_enabled` now takes `weight_offload_stable_va` and keeps capture OFF only when `weight_offload_enabled && !weight_offload_stable_va`. The signal is `policy.enabled && policy.managed_no_spill` (the explicit byte `--vram-limit` authority that installs the VMM arena + physical pool). The pointer-unstable `alloc_raw`/`free_raw` compatibility path keeps the old capture-off exclusion. **#723's `StableResident` eviction policy is unchanged** - this changes only the addressing mechanism underneath it. **Step 4 - measure** (`crates/onnx-genai-engine/tests/stable_va_capture_offload_e2e.rs`): drives the managed no-spill VMM stable-VA authority and verifies **token-identical** greedy decode with capture OFF and ON in one process, dumping the deterministic offload counters (`page_ins`, `hits`, `evictions`, `bypassed_page_ins`, `vram_alloc_ns`/`vram_free_ns`). ## Honest measurement caveat The full sustained-paging throughput/counter comparison the issue asks for could **not** be completed in this environment: - The genuinely-offloaded model (`qwen14b-zp`, 15.5 GB) is **not native-loadable** - it has no `inference_metadata.yaml` (`model.io` ports), which is the separate #384 load-chain gap. The native decode path (where this change lives) cannot load it. - On the native-loadable 0.5B model, the managed no-spill authority is **pin-or-fail**: either the ~381 MB weight set fits fully resident (0 page-ins) or a step cannot commit its working set (admission/headroom failure after heavy eviction, ~504 evictions observed). There is no clean budget window that both pages sustainedly *and* completes a full decode - that is exactly what #755's automatic weight streaming is for. What **is** verified end-to-end here: under the managed no-spill VMM stable-VA authority, decode with capture OFF and capture ON is **byte-identical** (32/32 greedy tokens), i.e. capture is transparent and no longer force-disabled. The addressing mechanism itself is proven **deterministically** by the two GPU tests in steps 1-2. A negative/partial result was an acceptable deliverable per the task; this is a positive mechanism result with a partial end-to-end result. ## Validation (local; CI not waited on) - `cargo fmt -p onnx-runtime-ep-cuda -p onnx-genai-engine -p onnx-runtime-cuda-memory -- --check` PASS - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` PASS (no `--all-targets`, the CI gate) - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` PASS (also linted the new cuda-gated test under `--features cuda,native-backend`) - `cargo test -p onnx-runtime-session` PASS (full suite) - GPU tests single-threaded PASS: `vmm_stable_va_weight_slot_gpu`, `vmm_retained_weight_key_keeps_a_stable_virtual_address_across_repage`, `vmm_weight_admission_reuses_owned_granules_without_runtime_alloc_free`, `weight_offload_gpu` (5/5) - engine `native_decode` unit tests PASS incl. new `weight_offload_on_stable_va_path_keeps_graph_capture` and updated `weight_offload_forces_graph_capture_off` Refs #716, #755, #727, #740, #723. **Do not merge; do not close #716.** Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
## Land the `commit_shared_prefix` allocator entry point (#777) Implements the smallest next increment named by the #777 probe (PR #793): the explicit pinned-prefix API on the CUDA VMM arena. This is the **allocator primitive only** — not a kernel change, not copy-on-write, and not wired into the KV path or engine. Refs #777, #793, #727, #740, #745. ### What this adds An explicit pinned-prefix API on `CudaVmmAllocator` in `crates/onnx-runtime-cuda-memory`: - **`create_shared_prefix(bytes)`** — reserves a private window and acquires N physical granules from the existing #740 authority-scoped pool, maps them read/write for the owner to fill, and charges them **once** on the owned axis (`governor.used(Tier::Device)`). Returns a `SharedPrefix` holding one owner-reference (the registry-entry lifetime model, so the prefix survives momentarily-zero sharers). - **`commit_shared_prefix(&prefix, ptr, allocation_bytes, byte_offset)`** — maps those already-owned handles into a *different* sequence's reservation at a granule-aligned `byte_offset`, **`PROT_READ`** (the Q3 non-sticky write-protection posture). Goes through the existing #740 pool and `carve()` suballocation on the existing global granule-ref **0→1 / 1→0** attribution. **No second allocator. No per-sequence physical reservation** (what made #733 net-negative). - **`incremental_owned_bytes_for_shared_prefix(&prefix)` → 0** — the Nth sharer bills only its private bytes, Q2's admission arithmetic made explicit on the entry point (#745). ### How the invariants hold in code (not comments) - **Union lifetime, structurally.** A pool-level `shared[handle]` refcount composes with the arena's per-granule allocation refcount; the physical handle returns to the pool only when the **last** mapping (owner or any sharer) unmaps. Freeing N−1 sharers does not release it; freeing the last one does. - **Charged once.** Shared granules are charged on `create_shared_prefix`; each `commit_shared_prefix` adds **0** owned bytes. - **Never map inside a captured region.** `enter_graph_capture()` raises a depth counter that `commit_shared_prefix` refuses under — the ordering rule is enforced, not documented. - **Loud write-protection.** Shared granules are `CU_MEM_ACCESS_FLAGS_PROT_READ`, so a stray write faults rather than silently corrupting another request's KV; Q3 proved this is non-sticky and this preserves that property. - **Physical bytes only** (`committed_physical_bytes`, `mapped_bytes`), never nominal content bytes. **No `assert!` in any `Drop`** (STATUS_STACK_BUFFER_OVERRUN on this platform). - **Errors, never mis-maps.** Misaligned target VA, granularity mismatch, over-long prefix, out-of-arena pointer, non-live allocation, a target granule that is already committed, and any attempt under an open capture all return `Err` and roll back cleanly (unmap + pool return), leaving the region uncommitted. ### Tests (`vmm_commit_shared_prefix_gpu.rs`) Against the **real** `CudaVmmAllocator` + `LedgerGovernor` (no mock), one concern per test: - N sequences share one pinned prefix; the ledger charges it **once**; freeing N−1 sharers does not release it, freeing the last one does; teardown returns `total_owned_bytes`, `governor.used`, and `creates − releases` all to zero. - Admitting an additional sharer costs only its private bytes (asserted against the real ledger). - A write into the shared region faults and the context survives (reuses the Q3 pattern; sync and async). - An unsupported request errors rather than mis-mapping. Per the #797 harness warning, all device ops route through synchronous copies or a single created stream (avoiding the legacy-default-stream vs non-blocking-stream silent partial-fill race), and each test builds its own allocator so no alphabetically-earlier sibling warms the context. ### Validation (local; not waiting for CI) - `cargo fmt -p onnx-runtime-cuda-memory -- --check` — clean. - `cargo clippy -p onnx-runtime-cuda-memory --features gpu-tests --all-targets -- -D warnings` — clean. - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` — clean. - GPU suites, single-threaded, all pass: the new `vmm_commit_shared_prefix_gpu` (4/4) plus the required regression set — the four `vmm_prefix_share_*`, `vmm_graph_remap_gpu`, `vmm_kv_token_major_floor_gpu`, `vmm_kv_layout_residency_gpu`, `vmm_stable_va_weight_slot_gpu`. ### What is now possible vs. what still needs KV integration The allocator can now physically share a pinned prefix across independent sequence reservations — charged once, read-only, correct union lifetime and admission arithmetic. The capacity mechanism (roughly doubled admissible concurrency for a large shared system prompt) now exists at the allocator layer. It does **not** yet: construct a sequence's KV as `shared prefix + private tail` in the engine, detect that two requests share a prefix, or handle divergence past the shared region. The immediate next increment named from evidence is **KV-path integration** (validated on the output-level parity oracle), which depends on the token-major KV shape/stride work (#783/#787) since only token-major reduces the prefix to a single multi-map per sequence. Then hash-based detection (layer 2), then boundary copy-on-write (layer 3, cost already characterized in Q5). `docs/PREFIX_SHARE_INVESTIGATION.md` is updated to move `commit_shared_prefix` from "next increment" to a "Landed this round" section and to name the following increment. > Not merging and not closing #777 per the task. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
Summary
ONNX_GENAI_WEIGHT_OFFLOAD_VMM=1).onnx-runtime-cuda-memoryVMM arena for reserve/create/map/set-access/granularity instead of adding new CUDA driver wrappers.Why VMM rather than application slots
The original stable-slot design would swap contents through fixed application slots and needs deterministic scheduling plus RAW/WAR ordering. This stage uses CUDA VMM to get the same captured-pointer stability from hardware: kernels see a stable virtual address while residency maps physical pages behind it outside captured regions. That is smaller and composes with #727's verified remap behavior.
WDDM behavior
This is opt-in because CUDA VMM physical memory (
cuMemCreate) cannot oversubscribe into WDDM shared memory the waycudaMalloccan. EnablingONNX_GENAI_WEIGHT_OFFLOAD_VMM=1can therefore hard-OOM on Windows where the current non-VMM weight path survives by spilling.Counters / evidence
GPU unit test proves:
cuMemAlloc/cuMemFreecounts are unchanged across page-ins.14B end-to-end, same rebuilt binary,
ONNX_GENAI_WEIGHT_OFFLOAD=1,ONNX_GENAI_WEIGHT_OFFLOAD_DEVICE_BYTES=6000000000,max-new-tokens=8:VMM granule floor on 14B: peak committed
6,050,283,520 B; peak content resident5,132,642,304 B; granule overhead917,641,216 B(~17.9% of content). The allocator churn result is good, but the residency counters are not neutral: accounting in 2 MiB granules forces evictions/transient bypasses under the 6 GB budget. This is the important follow-up result before merge.This PR still does not relax
resolve_graph_capture_enabled.Validation
cargo fmt --all -- --checkcargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warningscargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warningscargo clippy -p onnx-genai-cli --features native-cuda -- -D warningscargo test -p onnx-runtime-ep-cuda --features cuda stable_vmm_weight_slots_keep_their_virtual_address_across_eviction -- --nocapture(1 passed)Refs #716