Repository navigation
Probe: VMM page-level prefix sharing — primitive sound, no kill finding (#777) - #793
Conversation
…ng (#777, #727, #740, #776, #787) Isolating GPU probe + written design for the concurrency-scaled prefix-sharing prize. No KV integration, no engine or production-kernel change this round. Five questions, each with a driver-level or real-allocator GPU test, all cleared: * Q1 (vmm_prefix_share_gpu): one physical granule mapped into N=8 sequences' reservations reads identically at every VA, including under captured-graph replay (the decode hot path). One physical page, not N copies. * Q2 (vmm_prefix_share_ledger_gpu): the real CudaVmmAllocator + LedgerGovernor charge a shared granule ONCE, keep it alive until the last sharer leaves, and cost each additional sharer 0 incremental owned bytes — admitting the Nth request costs only its private bytes. * Q3 (vmm_prefix_share_write_protect_gpu): a write into a PROT_READ shared prefix faults loudly, the fault is non-sticky (context survives), and the other sharer's copy is uncorrupted. The potential kill finding did not fire. * Q4 (vmm_prefix_share_compose_gpu): a read-only shared prefix head, a read/write private live granule, and a read-only #759 dummy tail coexist in one reservation with three cuMemSetAccess postures. * Q5 (vmm_prefix_share_gpu): copy-on-write at the boundary is a one-time burst (~5.5 ms pooled / ~9.8 ms cold on WDDM), not a per-token tax. docs/PREFIX_SHARE_INVESTIGATION.md records the measured answers, the concurrency/saving and capacity tables (512/2K/8K prefix x 2/4/8/16 concurrent, qwen14b and qwen2.5-0.5b), and the design (explicit pinned-prefix API, lifetime, eviction, layout dependency). MEMORY_ARCHITECTURE.md points to it. Recommendation: the primitive is sound enough to build KV integration on; the smallest next increment is an explicit commit_shared_prefix() entry point on the token-major reservation model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Coordinator validation on the merged branch (merged current Scope verified: probe only — 4 test files plus 2 docs, zero production code. The resultAll five questions cleared, and the one that could have killed the design did not Q3 was the kill risk. The shared prefix region is read-only by construction, Q2 is the one that makes the economics real. Charged once, alive until the Q1 confirms #727's "one handle at two VAs" generalizes to N=8 sequences, each Q4 answers the composition risk I flagged early: the read-only shared prefix, Q5 puts copy-on-write in proportion: ~5.5 ms pooled / ~9.8 ms cold, once Why this matters more than the rest of the lineEvery other lever here moves bytes by a percentage. This removes a whole It also compounds with the layout work rather than competing with it: a shared Recommendation accepted. The next increment is an explicit Merging so the primitive's guarantees become executable guards. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #793 +/- ##
==========================================
- Coverage 80.08% 79.42% -0.66%
==========================================
Files 331 331
Lines 137782 137782
Branches 137782 137782
==========================================
- Hits 110337 109437 -900
- Misses 23092 23997 +905
+ Partials 4353 4348 -5
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
|
## 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>
…809) ## What this does (#777) PR #803 landed the prefix-sharing primitive (`create_shared_prefix` / `commit_shared_prefix` / `incremental_owned_bytes_for_shared_prefix`) on the production `CudaVmmAllocator`, but **nothing in production called it** — only its definition and GPU tests. An unconsumed contract reads as working infrastructure while proving nothing; that is exactly the shape that left #721 stage 3 stranded. This PR gives the primitive a **first real, deliberately narrow consumer**. ### What is now shared in production - **An allocator-agnostic seam on the `DeviceAllocator` trait** (`onnx-runtime-memory-governor`): `create_shared_prefix`, `incremental_owned_bytes_for_shared_prefix`, `commit_shared_prefix`, plus an opaque `dyn SharedDevicePrefix` handle and `SharedPrefixCommitInfo`. A caller holding only `dyn DeviceAllocator` can now pin a token prefix once and have subsequent sequences map it. Default impls **refuse** (`InvalidRequest`) rather than mis-map, so every existing non-VMM allocator keeps compiling and refuses safely (unsupported requests error rather than mismap, per #745/#750). - **`CudaVmmAllocator` implements the seam** by delegating to the merged inherent methods (#803) and downcasting the opaque handle via `as_any()`. Granules come from the existing #740 authority-scoped pool through `carve()`; no second allocator, no per-sequence physical reservation. - **First consumer:** the real **seq-major** fused fp16 GQA decode kernel (#782/#792/#794) reads shared-prefix VMM KV through the `dyn DeviceAllocator` seam. The prefix is physically contiguous as `layers × 2` ranges (seq-major, the gate #793 requires); token-major (one multi-map/sequence) is measured (#787) but not built here. The API is **explicit** (a caller declares the shared prefix), as #793 recommended for the highest-value case (a shared system prompt), and is **restricted to prefixes that are provably read-only for the sharers' lifetime** — copy-on-write at divergence is out of scope. ### Proof + measured numbers New GPU parity test `crates/onnx-runtime-ep-cuda/tests/gqa_shared_prefix_parity_gpu.rs` drives the real seq-major GQA decode kernel over shared-prefix VMM KV and proves two sequences sharing one pinned prefix produce **byte-identical** output to two independent sequences. Measured (KV_HEADS=8, HEAD_DIM=128, f16, 1024-token prefix + 1024-token private tail per sequence; 2 MiB granule → 1024 tokens/granule): | quantity | value | | --- | --- | | independent (2 sequences) committed **physical** bytes | **16,777,216 B** (8 granules) | | shared (2 sequences) committed **physical** bytes | **12,582,912 B** (6 granules) | | removed by sharing | **4,194,304 B** (2 granules = `(C−1)×(K_prefix+V_prefix)`) | | second-sharer admission cost | **4,194,304 B** — private tails only | | prefix `incremental_owned_bytes_for_shared_prefix` | **0** (charged once) | | output | **byte-identical** for both sequences | Deterministic counters lead; wall-clock is omitted (this box's identical runs range 3.9–28 tok/s and other agents build concurrently — no number is extrapolated). ### Validation (all local; not waiting for CI, fix-forward) - `cargo fmt -p onnx-runtime-memory-governor -p onnx-runtime-cuda-memory -p onnx-runtime-ep-cuda -- --check` — clean (`--all` fails on this box with os error 206, a Windows command-line-length artifact, not formatting). - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` — 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. - Full `cargo test -p onnx-runtime-session` — pass. - Single-threaded GPU suites — pass: `vmm_commit_shared_prefix_gpu` (4), `vmm_prefix_share_gpu` (2), `vmm_prefix_share_ledger_gpu` (1), `vmm_prefix_share_write_protect_gpu` (1), `vmm_prefix_share_compose_gpu` (1), `gqa_seqmajor_parity_gpu` (2), and the new `gqa_shared_prefix_parity_gpu` (1). Harness respects the known traps: VMM allocator built on the EP's own CUDA context; all device ops route through the single EP stream (#797); pool env set before any allocator is constructed and the process is isolated (#804); no assertions inside any `Drop`; live reservations explicitly deallocated before allocators drop. ### What remains (structural / out of scope) - **Auto engine use is blocked structurally.** The engine generation loop cannot call this seam automatically until `native_decode/cuda.rs` can build a seq-major fixed-stride physical KV shape — `persistent_state_shapes` is hard-coded BNSH and no model declares a seq-major end-to-end physical shape (#794 showed seq-major changed only kernel indexing, not commit geometry). A BNSH/seq-major fixed-stride physical-shape build is the prerequisite. This PR delivers the seam + the seq-major GQA-kernel consumer + proof; the automatic decode-path wiring is the next structural increment. - Hash-based automatic prefix detection (later; explicit path proven first). - Token-major (one multi-map per sequence; measured #787, not built). - Copy-on-write at divergence (#793 measured ~5.5 ms pooled / ~9.8 ms cold, once). Docs updated: `docs/MEMORY_ARCHITECTURE.md` (concurrency-scaled prize section) and `docs/PREFIX_SHARE_INVESTIGATION.md` (smallest next increment) now record the first production consumer, the measured numbers, and the remaining structural gap. Refs #777 #803 #793 #782 #745 #750 > Please do **not** merge and do **not** close #777 from this PR. Edits are > confined to the KV/prefix integration path to merge cleanly with concurrent work. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
…T answer (#777) (#822) Makes prefix sharing **layout-general** and answers, definitively and with code citations, whether the ORT backend can use it. Refs #777, #803, #809, #793, #776, #783. ## 1. The shareability decision is now arithmetic, not a layout (core work) Prior framing (mine) said prefix sharing "requires seq-major". That is a **granule-relative** claim stated absolutely, and it was wrong. The real gate: ``` fragment_bytes = prefix_len x (contiguous bytes per fragment in that layout) shareable = fragment_bytes >= granule shareable_granules_per_fragment = floor(fragment_bytes / granule) multi_map_ops = fragments x shareable_granules_per_fragment wasted_boundary_bytes/sequence = fragments x (fragment_bytes mod granule) ``` Layout sets `fragment_bytes` and the **cost**, not the **possibility**. The genuine requirements are (a) a VMM-backed KV buffer and (b) `fragment_bytes >= granule`. New tested predicate `onnx_runtime_memory_governor::shareability::evaluate_prefix_shareability` over `ModelKvGeometry` + `KvFragmentation` (`head_major_bnsh` / `seq_major_bsnh` / `token_major`, or an arbitrary #783 stride descriptor). Its `PrefixShareability` result carries the full accounting and a `refusal_reason()` so a KV path **refuses with a reason** instead of silently making N private copies. `gqa_shared_prefix_parity_gpu` now consults it to admit the share rather than assuming seq-major. Granularity is **queried** (2 MiB CUDA #776, ~64 KiB Level Zero/Vulkan, 4 KiB CPU mmap), never hardcoded. **Head-major becomes shareable when the arithmetic says so** — exactly 8,192 tokens at 2 MiB (`granule / (head_dim x dtype)`), realistic for RAG and long system prompts. ### Correction to the issue's worked table (a term to double-check) The 2 MiB seq-major cell reads "2 granules each". That is `ceil` (granules the prefix *touches* / residency). The **shareable** count is `floor(4,096,000 / 2,097,152) = 1`, giving `96 x 1 = 96` multi-map ops — which matches the design section's own "96 multi-maps per sequence". So the correct cell is **1 granule/fragment, 96 ops**. It is a ceil-vs-floor slip in one cell, not a missing term; every other verified cell matches. The invariant `fragments x contiguous_bytes_per_token = whole-model per-token bytes` holds for all layouts (encoded as a test). Full layout x granule x prefix-length grid for **qwen14b** and **qwen2.5-0.5b** is in `docs/PREFIX_SHARE_INVESTIGATION.md`. ## 2. ORT answer — definitive, with citations The question is not "can ORT share" (the primitive is layout-agnostic) but **"does ORT KV route through an allocator we can back with the VMM arena?"** - **Dynamic decode** (`DecodeKvMode::ZeroCopyRebind`, `decode/mod.rs:66`): ORT allocates `present.*` itself; no allocator of ours is involved. **Cannot route.** - **Shared-buffer decode** (`decode/shared_batch.rs:360`): uses `Value::empty_in` -> `CreateTensorAsOrtValue` with an allocator from `for_session_device` -> `CreateAllocator` (`allocator.rs:239`), which the doc says **"wraps the session's internal EP allocator"** (`allocator.rs:232`) — selects the *device*, not the *implementation*. - **`RegisterAllocator`** (`governed_allocator.rs:738`) installs our vtable but ORT reserves the arena kind for its own arenas (`allocator.rs:65`, `governed_allocator.rs:746`) and fronts a registered device allocator with its **own BFC arena** — each KV tensor is a byte-range inside an arena chunk, not a granule-aligned VMM reservation. **So `commit_shared_prefix` cannot attach to today's automatic ORT paths — a valuable negative.** The one viable route: allocate the shared-buffer KV on **our VMM arena** and bind it as external device memory via `CreateTensorWithDataAsOrtValue` (`value.rs:1103`, "so a caller can hand ORT memory it allocated itself") + `IoBinding`. Bounded by head-major arithmetic (>= 8,192-tok prefixes at 2 MiB). **Scoped, not implemented in this PR.** ## 3. Seam honesty preserved The #809 refusing defaults stay; the predicate supplies the reason so a non-shareable configuration errors rather than mis-maps. Shared regions stay `PROT_READ`, granules charged once, `incremental_owned = 0`, no second allocator. ## Validation (all green locally; CI not awaited, fix-forward) - `cargo fmt -p onnx-runtime-memory-governor -p onnx-runtime-ep-cuda -- --check` - `cargo clippy -p onnx-runtime-cuda-memory --features gpu-tests --all-targets -- -D warnings` - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` - `cargo test -p onnx-runtime-session` (all pass), predicate unit tests (8 pass) - GPU, single-threaded: `vmm_prefix_share_*` (5), `vmm_commit_shared_prefix_gpu` (4), `gqa_shared_prefix_parity_gpu` (1) — all pass - #814 policy: `mobius_seqmajor_growth_parity_native_cuda --ignored` — pass Does **not** merge or close #777. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
VMM page-level prefix sharing — isolating GPU probe + design (#777)
Scope: an isolating GPU probe in
crates/onnx-runtime-cuda-memoryplus awritten design. No KV integration, no engine or production-kernel change.
References #777, and builds on #727 (1-handle-at-2-VAs), #740 (physical-handle
pool + 0→1/1→0 granule-ref attribution), #776 (2 MiB granule), #787
(token-major floor), #759 (dummy page).
Read
docs/PREFIX_SHARE_INVESTIGATION.mdfor the full measured answers, tables,and design;
docs/MEMORY_ARCHITECTURE.mdpoints to it.Result: the primitive is sound enough to build KV integration on — no kill finding
All five questions cleared, each with its own GPU test binary (one question per
binary so a poisoned context or a process-global counter cannot contaminate
another):
vmm_prefix_share_gpuvmm_prefix_share_ledger_gpuCudaVmmAllocator+LedgerGovernor(no mock): a shared granule is charged once, stays alive until the last sharer leaves, and each additional sharer needs 0 incremental owned bytes. Admission arithmetic: the Nth request costs only its private bytes.vmm_prefix_share_write_protect_gpuPROT_READshared prefix both fault; the fault is non-sticky (context survives, later work succeeds) and non-corrupting (the other sharer's copy is intact). The potential kill finding did not fire.vmm_prefix_share_compose_gpucuMemSetAccesspostures.vmm_prefix_share_gpuConcurrency-scaled saving (physical, granule-rounded)
Under token-major the whole-model prefix is one contiguous range. Sharing an
S-token prefix acrossCconcurrent requests removes(C−1)duplicate copies.Example — qwen14b, 2,048-token prefix, 8 concurrent: 2,688 MiB
(≈2.6 GiB) removed, matching the ~2.56 GiB figure in
MEMORY_ARCHITECTURE.md.Full 512/2K/8K × 2/4/8/16 tables for qwen14b and qwen2.5-0.5b are in the doc.
Capacity consequence (the number that matters): admissible concurrency rises
from
floor(B / (prefix + private))to1 + floor((B − prefix) / private).For a 50/50 prefix/private split this roughly doubles concurrency on a fixed
budget (e.g. 3.0 GiB KV budget: 4 → 8 requests), and the multiple grows as
the shared prefix grows — reproducing the issue's "2 vs 8 concurrent" illustration.
Design (documented, not implemented)
covers the highest-value multi-tenant system-prompt scenario); hash-based
detection is a later layer.
granule refcount, plus an owner reference for a pinned prefix.
first increment makes a pinned prefix non-evictable for its lifetime.
token-major (one multi-map) — a further argument for KV layout should be a stride descriptor negotiated per EP, not a growing enum — and the choice is not memory-neutral #783/Probe: token-major-across-all-layers KV — TLB risk cleared, floor measured (#750, #777, #783) #787.
Smallest next increment
An explicit
commit_shared_prefix(handle, offset, len)entry point on thetoken-major arena that maps a pooled physical handle into a new sequence's
reservation at offset 0 and takes a granule-ref on it (owned axis → Nth sharer
bills 0). Pinned, read-only, no detection/COW/eviction yet. Validate on the
existing output-level parity oracle. The attention kernel and the flat per-sequence
VA are unchanged.
Constraints honoured
Never unmap under a replay in flight (divergence syncs first); captures only read
already-mapped stable VAs; no assertion inside any
Drop; physical bytes reportedfrom handle counts / the governor's owned axis, never nominal content bytes.
Validation (local, this box — RTX 4060 Laptop, 8 GiB, WDDM)
cargo fmt -p onnx-runtime-cuda-memory -- --check✅cargo clippy -p onnx-runtime-cuda-memory --features gpu-tests --all-targets -- -D warnings✅vmm_graph_remap_gpu,vmm_dummy_page_gpu,vmm_kv_token_major_floor_gpu— 13 tests, all pass, single-threaded. ✅Do not merge, do not close #777.