Repository navigation
Build KV growth byte geometry under a BSNH fixed stride (seq-major) - #801
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #801 +/- ##
==========================================
- Coverage 79.43% 79.43% -0.01%
==========================================
Files 331 331
Lines 137820 137820
Branches 137820 137820
==========================================
- Hits 109483 109480 -3
- Misses 23991 23993 +2
- Partials 4346 4347 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Make native CUDA KV state grow along a fixed-stride BSNH byte layout when the resolved layout is seq-major, closing the last engine-side gap to a real end-to-end seq-major generation (the documented next step after #797). The precise gap turned out not to be the *shape build* but the *growth byte geometry*. The CUDA GQA node validates past_key/past_value as BNSH [batch, kv_heads, seq, head_dim] and reads present_capacity from axis 2 regardless of the kv_layout attribute (only the kernel stride arithmetic re-specializes), and DeviceIoBinding requires logical/physical shapes to share axis order. So persistent_state_shapes must stay BNSH for both layouts; a permuted binding shape cannot express seq-major. Seq-major is realized purely in the growth/commit byte geometry. Changes (confined to native_decode/cuda.rs growth functions, to merge cleanly with the concurrent managed-VMM and GEMM-workspace work): - kv_growth_byte_layout(): map the BNSH binding shape to the physical byte layout + grow axis the resolved KvCommitLayout stores it in. Head-major = declared shape, grow axis 2 (one stripe per head, byte-identical to before). Seq-major = [batch, capacity, kv_heads, head_dim], grow axis 1, with a capacity-independent kv_heads*head_dim per-token stride. - apply_vmm_growth / build_grown_buffers: drive the existing device copy/zero primitives over that byte layout instead of a hard-coded axis-2 scatter. - ensure_capacity: in-place VMM growth accounts 0 moved KV bytes for seq-major (fixed stride keeps the prefix in place) vs valid_len*bytes_per_token for head-major. - persistent_state_shapes: documented the BNSH-metadata contract. head-major is byte-identical to today (default() == HeadMajor and absent metadata unchanged), asserted explicitly in a unit test. Validation: - 2 new engine unit tests: head-major stays BNSH / seq-major permutes to BSNH; seq-major growth is fixed-stride and moves no bytes while head-major re-strides. - New real-model e2e (mobius_seqmajor_growth_parity_native_cuda, #[ignore]): patches qwen2.5-0.5b-q4_0-mobius GQA nodes with kv_layout=1, forces growth with ONNX_GENAI_KV_MIN_BUCKET=8, generates 48 greedy tokens over 3 growth events, and asserts byte-identical head-major vs seq-major output, capture requested OFF and ON, under both growth mechanisms. Measured end to end: in-place VMM seq-major growth moved 0 KV bytes vs head-major 688,576. - gqa_seqmajor_parity_gpu (independent CPU oracles), group_query_attention_gpu (25), vmm_kv_layout_residency_gpu, vmm_kv_token_major_floor_gpu, vmm_stable_va_weight_slot_gpu all green single-threaded; full onnx-runtime-session green; engine + ep-cuda clippy gates green. Honest caveats (orthogonal, out of scope): committed physical bytes remain equal head vs seq (flat bucket/granule commit; the kv_heads-x residency win is still driver-level only, #797); this decoder's dynamic logical shape declines whole-step graph capture (captures=0) so the boundary re-capture cost could not be reproduced end to end (the obstacle #794 also hit); wall-clock not reported (box contended, 7 s-212 s spread this session). Refs #797, #794, #782, #750, #777. Does not flip any default; no issue closed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
force-pushed
the
squad/bsnh-fixed-stride
branch
from
August 12, 2026 04:35
258fa79 to
8794982
Compare
Owner
Author
|
Coordinator pre-merge validation on the merged branch: The structural finding corrects my task framing, and that is the valuable partI asked for
That is a better answer than the one I asked for. It also explains cleanly why The measured win, and its interaction with #798Because #798 made managed VMM the default, the default growth path is now Real seq-major generation is now validated end to end using the #794 approach: a Caveats reported honestly rather than smoothed overTwo, both stated plainly in the PR rather than omitted:
That second one is worth a follow-up in its own right: if the decoder declines Wall-clock was correctly not reported: the box was contended and the spread was Merging. |
🔴 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 12, 2026
) ## Headline result `qwen2.5-0.5b-q4_0-mobius` does **not** structurally decline native CUDA graph capture on current `main`. The `captures=0` result in #801 was a policy artifact in its test harness: the capture-OFF phase set `ONNX_GENAI_CUDA_GRAPH=0` before the process-wide `RuntimeConfig` `OnceLock` was initialized, then the later capture-ON phase mutated the environment in the same process. That mutation cannot change the cached runtime policy, so predicate `ONNX_GENAI_CUDA_GRAPH` continued to decline capture. With each policy phase isolated in its own process, the real model captures successfully. The forced-growth run measured deterministic counters: - capture ON, head-major: `growth_events=3`, `captures=4`, `invalidations=4`, `replays=39`, no decline reason - capture ON, seq-major: `growth_events=3`, `captures=4`, `invalidations=4`, `replays=39`, no decline reason - capture OFF: `captures=0` now reports `predicate ONNX_GENAI_CUDA_GRAPH declined capture` This also supplies the missing end-to-end reproduction discussed in #801/#782/#797: growth currently triggers re-capture for both layouts. Fixed-stride seq-major removes VMM KV-copy bytes (`688,576 -> 0`) but does not remove the decoder's current growth invalidation. Changing that invalidation is correctness-sensitive because captured launches may bake the capacity geometry; this PR does not change eligibility or invalidation semantics. ## Reporting change - Replaces the decode-level boolean resolution with an invariant-carrying decision: every disabled decision owns a named predicate and reason. - Covers policy, structural, binding-shape, attention-mask, auxiliary-output, and runtime capture-audit declines. - Emits one warning per decoder session and exposes `decline_reason` through CUDA debug stats. - Prints `cuda_graph_decline_reason` beside the existing `cuda_graph` counters in `profile_native`. - Updates the #801 Mobius regression to run ON/OFF phases in separate processes and assert capture ON actually captures. - Adds an anti-regression test that every decode-level decline names its predicate. #796/#798 are reflected in the decision precedence: stable-VA managed no-spill offload does not decline capture; only `weight_offload_enabled && !weight_offload_stable_va` does. ## Validation - `cargo fmt -p onnx-genai-engine -p onnx-genai-bench -- --check` - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` - full `cargo test -p onnx-runtime-session` (153 primary tests; all secondary binaries passed) - targeted engine capture-decision tests (7 passed) - real-model Mobius seq-major growth regression (all three isolated phases passed) Wall-clock is intentionally not compared: the post-rebase real-model run was heavily contended (individual model runs ranged roughly 69-259 seconds). Only deterministic counters are used above. Refs #801, #782, #797, #796, #798. Does not close any issue. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
This was referenced Aug 12, 2026
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
…ures (#805) ## Headline: fixed-stride seq-major still has 4 invalidations, equal to head-major The #782 prediction is **not closed by the current end-to-end engine path**. On the default managed in-place VMM allocator, process-isolated and interleaved capture-ON runs measured: | Layout | Growth events | Captures | Replays | Invalidations | Physical committed bytes | KV bytes moved | |---|---:|---:|---:|---:|---:|---:| | head-major | 3 | 4 | 39 | 4 | 100,663,296 | 688,576 | | seq-major | 3 | 4 | 39 | 4 | 100,663,296 | 0 | The 48 generated tokens and text were byte-identical. Every individual configuration ran in its own process, avoiding #804's cached-policy failure. Runs were interleaved head-major then seq-major. Wall-clock is intentionally omitted because deterministic counters answer the question and this shared box has high timing variance. A no-growth control (`min_bucket=64`) measured `1 capture, 45 replays, 1 invalidation` for both layouts. Thus three forced growths add exactly three invalidations and three captures in both layouts. Code attribution agrees: `DecodeCudaState::ensure_capacity` calls `invalidate_graph` unconditionally after VMM growth, before the layout-specific moved-byte accounting. Fixed stride removes KV movement but the engine does not yet retain the graph based on that fact. The harness now process-isolates every layout/configuration, emits structured measurements, asserts output parity, and keeps the legacy realloc check (`786,432` physical bytes; `688,576` moved for both). Documentation records the actual invalidator rather than claiming end-to-end zero re-captures. ## Validation - `cargo fmt -p onnx-genai-engine -- --check` - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` - full `cargo test -p onnx-runtime-session` (153 primary tests plus all secondary binaries) - real-model isolated/interleaved Mobius measurement (8 child processes; passed) Refs #782, #797, #801, #804. Does not close or merge any issue. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
…804) (#807) ## Why This class of defect produced **two wrong conclusions this week**, both of which cost real work before being caught. Both are the same family: **state that is frozen or warmed once per process, making results depend on test order.** On a measurement-driven subsystem a wrong number does not just fail — it redirects the design. - **#797** — a GPU residency test failed ~50% of the time on a cold context and looked exactly like a correctness bug in growth-under-capture. Root cause: a readback on the **legacy default stream** raced a memset on a created **non-blocking stream** (the two are mutually exempt from implicit synchronization), so the readback returned partial fills **with no CUDA error**. `cuCtxSynchronize` did *not* fix it, which refuted the obvious race theory. It only "passed" because an alphabetically-earlier sibling subtest warmed the context. - **#801 / #804** — `captures=0` was reported as the decoder structurally declining CUDA graph capture, and that attribution reached a merged PR body. Root cause (#804): the capture-OFF phase set `ONNX_GENAI_CUDA_GRAPH=0` before the process-wide `RuntimeConfig` `OnceLock` was first read, and that config is frozen on first read — so every later phase in the same process inherited it. #794 and #801 failed to reproduce an end-to-end measurement and misattributed the reason. This is the same telemetry-that-misleads lineage as #719 / #715 (documented in `MEMORY_ARCHITECTURE.md`'s "third failure mode"). After the second instance in a week, the response is a **guard**, not more vigilance. This PR is **test infrastructure + documentation only**. No production behavior changes. ## What ### 1. A guard for process-frozen configuration (`onnx-genai-runtime-config`) `runtime_config()` records the environment it froze the `RuntimeConfig` snapshot from (captured through the same lookup `from_env` uses, so the set can never drift from the parsed fields). On every later call it re-reads those variables and **panics loudly** — with the frozen and current values, and a pointer to #804 — if any now differs. A test can no longer silently believe it set a knob that was already frozen. - **Gated on `debug_assertions`, not `cfg(test)`.** The #804 mutation lives in an *integration* test of a downstream crate, where `cfg(test)` for this crate is never set. `debug_assertions` is enabled for dependencies under the `dev`/`test` profile, so the guard is live for every `cargo test` in the workspace, and it is compiled out of `--release` — **the production path is byte-for-byte unchanged and pays nothing** (a single `OnceLock` read). - The comparison is a pure `find_changed()` helper with unit tests for the unchanged, changed, newly-set (the #804 shape), and cleared cases. ### 2. A helper for stream-correct device test code (`onnx-runtime-cuda-memory::test_support::TestStream`) A shared, test-only (`#[cfg(any(test, feature = "gpu-tests"))]`) RAII wrapper around **one** non-blocking stream. Every device op a test needs — `fill`, `write`, `read`, plus `raw()` for graph launches/memsets — flows through that stream, so a single `cuStreamSynchronize` is a total order. The module doc explains the legacy-default-vs-non-blocking exemption **at the call site** so the next author does not rediscover #797 the hard way. `vmm_kv_layout_residency_gpu.rs` (the exact #797 site) is converted to it and its four local stream helpers removed; **8/8 reliable in isolation** and green in the full single-threaded suite. ### 3. The pattern, documented where it will be read (`docs/MEMORY_ARCHITECTURE.md`) A tight "**a fourth failure mode: order-dependent test state**" subsection next to the existing third-failure-mode material: both case studies, the two mechanisms (frozen process config; context warmed by an earlier sibling), the two remedies (process isolation for anything that reads process-frozen config; single-stream discipline for device tests), and the one-line discipline that names the defect — *run any test you touch in isolation as well as in the full suite*. Cites #797/#801/#804/#794/#719/#715 and links rather than duplicating (#791 consolidated this doc to stop drift). ## Inventory of other order-dependent tests found (not fixed here) Per scope discipline, these are **listed, not converted**. The `TestStream` helper and the config guard make converting them mechanical follow-ups. ### Device tests that mix a created stream with default-stream copies (#797 hazard) Each creates a non-blocking stream **and** issues default-stream `cuMemcpyHtoD_v2`/`cuMemcpyDtoH_v2` copies. Where those touch the same buffers as stream-ordered work on a cold context, they are exposed to the exact #797 race. They should migrate to `test_support::TestStream`: - `crates/onnx-runtime-cuda-memory/tests/vmm_commit_shared_prefix_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_dummy_page_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_dummy_write_protect_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_granularity_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_graph_remap_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_kv_contiguous_tail_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_prefix_share_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_prefix_share_write_protect_gpu.rs` - `crates/onnx-runtime-cuda-memory/tests/vmm_stable_va_weight_slot_gpu.rs` (Non-vmm siblings `virtual_memory_gpu.rs` and `physical_pool_production_gpu.rs` also use default-stream copies; they pass but are the same latent shape.) ### Tests that mutate a `RuntimeConfig`-feeding env var within a process These change a variable that feeds the frozen snapshot (`ONNX_GENAI_CUDA_GRAPH`, `ONNX_GENAI_EP`) across phases. The correct pattern is **child-process-per-phase**; the new guard will now fire loudly on any that mutate in-process and then read `runtime_config()`: - `crates/onnx-genai-engine/tests/mobius_seqmajor_growth_parity_native_cuda.rs` — **already correct** (spawns a child per mode; its own doc comment states `ONNX_GENAI_CUDA_GRAPH` is parsed once). This is the reference pattern. - `crates/onnx-genai-engine/tests/stable_va_capture_offload_e2e.rs` — sets `ONNX_GENAI_CUDA_GRAPH` per capture phase. - `crates/onnx-genai-engine/tests/gemma4_e2b_native_cuda_pipeline_bench.rs` — sets `ONNX_GENAI_CUDA_GRAPH` to `1` then `0`. - `crates/onnx-genai-engine/tests/qwen2_5_1_5b_divergence.rs` — switches `ONNX_GENAI_EP` between `cpu` and `cuda`. - `crates/onnx-genai-engine/tests/qwen3_0_6b_capture_step_inputs_decline.rs` and `gemma3n_native_cuda_capture_realmodel.rs` — toggle the native-decoder capture env per phase. Session-crate tests that toggle env (`ONNX_GENAI_DECODE_MEMO`, `ONNX_GENAI_SCAN_INLINE_SINGLE_TRIP`, `ONNX_RT_SIBLING_MERGE`, the EP-CPU weight-offload knob) read those variables **directly at their call sites**, not through `RuntimeConfig`, so they are *not* affected by the freeze guard and the full `onnx-runtime-session` suite stays green. ## Validation (all local; not waiting for CI) - `cargo fmt -p onnx-genai-runtime-config -p onnx-runtime-cuda-memory -- --check` ✅ (`--all` hits the Windows `os error 206` long-command artifact, not a formatting issue) - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` ✅ - `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 test -p onnx-runtime-session` ✅ (full suite) - `cargo test -p onnx-runtime-cuda-memory --features gpu-tests -- --test-threads=1` ✅ (all vmm_*/gpu suites, RTX 4060) - The converted `seq_major_fixed_stride_grows_under_captured_replay_without_recapture` run **8× in isolation**: 8/8 pass — the property this PR is about. ## Notes - Kept strictly to test infra + the docs subsection so the two other active workstreams (GQA pooled QKV staging in `group_query_attention.rs`; head-major vs seq-major re-capture measurement) merge cleanly — no overlap. - Does **not** merge and does **not** close any issue. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
This was referenced Aug 12, 2026
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
…ajor-only KV consumers (#777 #801 #797 #794 #809) (#812) # Remove the last blocker to seq-major KV being usable in production: gate the fourth place the layout lives **Outcome reached: #3 — "it works only under a condition."** Seq-major decode works end-to-end and is byte-identical to head-major, but only on the **pure decode path where our own GQA kernel is the sole consumer of the device KV buffer**. This PR makes the runtime **refuse rather than mis-map** when that condition does not hold, and pins down — with citations — exactly what remains between "growth byte geometry is seq-major" (#801) and "the `layers × 2` floor realised + prefix sharing available." Refs #777 #797 #794 #801 #809 (and #787 for token-major). Does **not** merge, does **not** close any issue. ## The question this PR answers > What remains between "growth byte geometry is seq-major" and "a real model runs seq-major end to end with the floor realised and prefix sharing available"? A fourth place the layout lives, or can the declared BNSH shape and a seq-major physical layout coexist provided only our own kernels read the buffer? **Both.** The declared BNSH shape and a seq-major physical layout *do* coexist on the native decode path (KV never leaves the device; only our GQA kernel indexes it). But there **is a fourth place the layout lives** — two engine-side *non-kernel* consumers of the device KV buffer that still stride it with hard-coded head-major (BNSH) arithmetic: 1. **`DecodeCudaState::read_present_kv`** (`native_decode/cuda.rs`) — the device present-KV **host mirror** used by prefix/paged read-out and diagnostics. It compacts per head with `capacity_head_stride = physical_shape[2] * head_dim * elem` and a per-head fragment read. 2. **`DecodeCudaState::seed_prefix`** (`native_decode/cuda.rs`) — the **paged prefix-reuse device seed** (#777/#809's device counterpart). It writes each head at `head * capacity_head_stride * elem_size`, the same head-major capacity offset. Under a seq-major (BSNH) physical buffer, heads are **interleaved per token** (`slot * kv_heads * head_dim + h * head_dim + d`), not laid out as `max_len × head_dim` capacity stripes. So both offsets index the wrong bytes: the host mirror would **silently return mis-indexed KV**, and the prefix seed would **corrupt the cache**. Neither had a seq-major guard — a real latent-corruption path. This is the fourth place the layout lives, beyond the three that are already converted: 1. kernel indexing — done (#792/#794); 2. commit geometry — done (#797); 3. growth byte geometry — done (#801); 4. **engine-side non-kernel KV consumers — gated here.** ## What this PR changes `crates/onnx-genai-engine/src/native_decode/cuda.rs`: - `read_present_kv` and `seed_prefix` now **error under `kv_layout.is_seq_major()`** with a precise message stating the condition and why (head-major byte arithmetic vs interleaved seq-major bytes). This matches the kernel-level "unsupported readers/writers fail rather than silently mis-index" gate (`kv_stride.rs::require_converted_path_support`) at the engine level. - **Head-major is untouched and byte-identical** — the gate is a pure `if is_seq_major() { bail! }` before the existing code. Absent metadata ⇒ head-major ⇒ no gate. `docs/MEMORY_ARCHITECTURE.md` (§ "KV layout and residency"): records the fourth layout location and the refuse-not-mismap gate as a bullet in the existing residency-caveat block. ## The structural constraint (#801), re-verified The BNSH binding **metadata** shape must stay for both layouts. The CUDA GQA node validates `past_key`/`past_value` as `[batch, kv_heads, seq, head_dim]` and reads `present_capacity` from **axis 2 regardless of `kv_layout`** (`group_query_attention.rs`), and `DeviceIoBinding` requires matching axis order. `persistent_state_shapes` is therefore *correctly* BNSH for seq-major too; seq-major lives in the byte geometry, not a permuted shape. Confirmed still true after merging the concurrent GQA-workspace change. ## Why the `layers × 2` floor is still not realised through the engine (and what it needs) Measured directly, **before** the merge broke the harness, with the validated #801/#805 setup — stock `qwen2.5-0.5b-q4_0-mobius` (BNSH) vs a copy whose 24 GQA nodes carry `kv_layout=1`, forcing growth with `ONNX_GENAI_KV_MIN_BUCKET=8`, 48 greedy tokens, 3 growth events (`mobius_seqmajor_growth_parity_native_cuda`, this branch @ pre-merge): | Growth path | Capture | Output | Physical committed bytes (head = seq) | d2d KV copy bytes head | d2d KV copy bytes seq | |---|---|---|---:|---:|---:| | in-place VMM (default) | OFF | **byte-identical** | 100,663,296 | 688,576 | **0** | | in-place VMM (default) | ON | **byte-identical** | 100,663,296 | 688,576 | **0** | | legacy realloc | OFF | **byte-identical** | 786,432 | 688,576 | 688,576 | Capture-ON, per-process: head-major and seq-major both `(growth 3, captures 4, replays 39, invalidations 4)`. So on this branch the seq-major decode path is **byte-identical head vs seq, capture ON and OFF, and moves 0 KV bytes on in-place growth** — but **`physical_committed_bytes` is identical between layouts**, i.e. the floor is *not* realised through the engine. Two cited reasons: 1. **`vmm_growth_requests` commits the full bucket** (`(index, 0, full_bucket_bytes)`) for every KV binding, for both layouts, instead of the dense live-prefix ranges `native_decode/kv_commit.rs::live_prefix_ranges` computes (those helpers are unit-tested and mirror the driver GPU test but are `allow(dead_code)` — not wired into the engine). 2. **The `kv_heads×` difference only becomes *measurable* under a large fixed stride.** With bucket growth the bucket ≈ 2× valid, so on 0.5b both layouts land in **1 granule/binding** (48 granules = 96 MiB). The `kv_heads=2` gap appears only when a head stripe crosses a granule boundary, i.e. `capacity × head_dim × elem ≥ granule` (`ONNX_GENAI_CUDA_KV_MAX_LEN=32768` in the driver test `vmm_kv_layout_residency_gpu`): head-major = `layers × 2 × kv_heads` granules, seq-major = `layers × 2` (the measured 192 MiB → 96 MiB, 2× = `kv_heads`; 1536 MiB → 192 MiB, 8× on qwen14b). Realising the measurable floor therefore requires **a fixed full-context stride + dense-prefix commit + per-step commit-ahead**, which eliminates growth-driven bucket recapture. That change lives in the **growth/capture-invalidation decision** — the explicit lane of the concurrent `conditional-invalidation` / `bsnh-fixed-stride` work — and outside this PR's shape/binding-construction lane. It is left as the documented next step (shared with token-major #787 and prefix sharing #777), consistent with the existing doc note: *"wiring the dense-prefix commit into the live path is the documented next step."* ## Prefix sharing (#777/#809) under seq-major It does **not** fall out naturally. #809's device consumer is `seed_prefix`, a head-major byte writer; a shared prefix is only physically contiguous under seq-major, but the seed that would install it cannot yet address the seq-major buffer. This PR makes that explicit (the seed errors under seq-major) rather than letting prefix reuse silently corrupt seq-major KV. Realising it means teaching `seed_prefix` (and `read_present_kv`) the BSNH byte geometry — same dense-prefix work as the floor. ## Validation - `cargo fmt -p onnx-genai-engine -- --check` ✓ (`--all` fails with os error 206 on this box — a command-line-length artifact, not formatting) - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` ✓ - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` ✓ (post-merge) - full `cargo test -p onnx-runtime-session` ✓ (post-merge) - `mobius_seqmajor_growth_parity_native_cuda` (byte-identical head vs seq, capture ON/OFF, 0 bytes moved) ✓ **both pre-merge and post-merge** (post-merge re-run includes the #814 governor fix, see below); the gate does not regress it. All four configs identical to the pre-merge numbers above (`physical_committed_bytes` 100,663,296 / 786,432; seq `kv_bytes_moved=0`). ### Cross-agent regression surfaced by `merge origin/main` — now fixed by #814 Merging `origin/main` (which brought the `gqa-bnsh-transpose` GQA transpose-scratch change #810) initially broke the mobius end-to-end test **during prefill workspace prep**, before any decode: ``` Error: prepare native CUDA prefill workspace Caused by: cannot reserve 2097152 bytes of device memory for Workspace { step_scoped: false }: 0 of 2097152 bytes are already leased, leaving 0 ``` #810 reserves a new **session-persistent** BNSH transpose-scratch `Workspace { step_scoped: false }` (`WorkspaceLifetime::SessionPersistent`) during prefill; the `vram-limit dynamic KV/weight lending` strategy did not provision headroom for it (GPU otherwise 100% free — 0 MiB used). This was a governor role-budgeting interaction in the **GQA/memory lane**, independent of this PR's gate (which only adds error paths in `read_present_kv`/`seed_prefix`, never on the prefill workspace path). I attributed it precisely and reported it to the `gqa-bnsh-transpose` sibling rather than fixing out of lane. **It has since been fixed forward in #814** (commit `54f89a03`: "mapped growth now claims unassigned authority capacity before holder reclaim, with rollback/overestimate accounting"). Merging #814 locally on top of this branch, the mobius test **passes all four VMM/realloc × capture configurations, byte-identical head vs seq**, confirming this PR's gate and #814's governor fix coexist cleanly. #814 is not yet on `main`, so this branch is left at its clean two-file state (the fix will arrive via `main`); the post-merge validation above was run against `this branch + #814` locally. ## Files - `crates/onnx-genai-engine/src/native_decode/cuda.rs` — seq-major refuse-not-mismap gate on the two head-major-only KV consumers. - `docs/MEMORY_ARCHITECTURE.md` — fourth-layout-location note. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
…: 4 -> 0 growth invalidations) (#811) ## Headline: seq-major KV growth no longer needlessly re-captures the decode graph Measured, process-isolated, on `qwen2.5-0.5b-q4_0-mobius` (+ the `kv_layout=1` patched copy from #801/#805), capture ON, forced growth (3 bucket growths, output byte-identical throughout): | layout (capture ON, forced growth) | captures | replays | invalidations | growth_keeps | |---|---:|---:|---:|---:| | head-major (**before = after**, moves bytes) | 4 | 39 | 4 | 0 | | seq-major **before** (#805) | 4 | 39 | 4 | 0 | | seq-major **after** (this PR) | **1** | 45 | **1** | **3** | Seq-major's forced-growth graph accounting now matches its **no-growth control exactly** (`captures=1, replays=45, invalidations=1`): all **3 growth-attributable invalidations are eliminated** and recorded as keeps. The single residual invalidation is a baseline (present in the no-growth control too), not growth-attributable. Head-major is unchanged — it genuinely re-strides every head stripe on growth (688,576 bytes moved), so it must re-capture. Output is **byte-identical** to `main` across both layouts, capture ON and OFF, VMM and legacy realloc. ## Why this is sound (the dependency set I check, and why it is complete) The CUDA GQA decode kernel bakes exactly two KV-dependent things at capture: - **device pointers** into the recorded graph nodes, and - the **physical shape** (kernel argument extents / strides), which the executor's `replay_device_graph` folds into a per-binding replay **signature** (device_ptr + physical_shape + dtype + I/O identity) and *rejects* replay on any change. Everything else the decode graph reads at replay is **data**, not baked: the key extent and launch geometry derive from the `total_lengths` **device pointer** (read at replay), never from `cache_capacity` (kernel comment `gqa_decode_fp16.rs:388`). And `cache_capacity` enters seq-major addressing (`kv_stride.rs`) only through the **batch** term of `GQA_KV_BASE` — which is `0` in native decode (batch=1, b=0). So at b=0, seq-major addressing is **capacity-independent**: token `t` always lands at `t*kv_heads*head_dim`. Therefore, if I keep the device pointers and physical shapes identical across a growth, **no captured dependency changes**. I achieve that by pinning the seq-major fixed-stride path's KV physical shape (and mask island) at the **hard-max** context length from construction, committing only the initial dense token prefix, and growing by committing more granules on demand at the **same base VA** (in-place VMM, proven at the driver level by #797 and byte-identical with 0 bytes moved by #805). Growth changes neither pointer nor shape nor addressing. **Default-to-invalidate, defense in depth:** before keeping, I snapshot every binding's `(device_ptr, physical_shape)` and re-check it after the commit; if anything moved, I **invalidate** and say so. The executor's replay signature check is a further backstop that fails safe (errors, never silently corrupts) if a pointer ever changed unexpectedly. Head-major keeps invalidating (its per-head stride *is* `cache_capacity`), and the legacy realloc path (`ONNX_GENAI_LEGACY_ALLOCATOR=1`, `commits_on_demand == false`) keeps invalidating — verified: seq-major realloc still moves 688,576 bytes and invalidates 4×, identical to head-major. ## Reporting (#804) The decision is surfaced through named reporting so an operator can see *why* a graph was kept or invalidated: - `CudaGraphDebugStats.growth_keeps` (count) and `CudaGraphDebugStats.growth_decision` (named rationale) on `CudaKvDebugStats`; - a `tracing::info!` event per decision. A silent "kept" is as dangerous as the silent `captures=0` that cost prior PRs, so every keep is counted and logged. ## Tests - **Negative oracle (the test that matters most)** — `head_major_growth_that_moves_addresses_must_invalidate_not_keep`: forces a growth that genuinely relocates KV addresses (head-major, 688,576 bytes moved) and asserts the graph is **invalidated and re-captured, never kept** (`growth_keeps == 0`, one extra invalidation + one extra capture per growth vs. the no-growth control). Fails red if a graph is ever kept when a dependency changed. - The #805 harness `mobius_seqmajor_growth_parity_native_cuda` is extended to assert the conditional-keep counts above and that seq-major forced-growth accounting equals its no-growth control, while keeping the byte-identical parity assertions. - `gqa_seqmajor_parity_gpu` (independent CPU prefill + decode oracles) stays green. ## Validation run (all green) - `cargo fmt -p onnx-genai-engine -- --check` - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` - `cargo clippy --locked --all-targets -p onnx-genai-engine --features cuda,native-backend -- -D warnings` - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` - full `cargo test -p onnx-runtime-session` — all `ok`, 0 failed (153 unit + all integration/doctests) - GPU, single-threaded: `mobius_seqmajor_growth_parity_native_cuda` (2 passed, incl. negative oracle), `gqa_seqmajor_parity_gpu` (2), `group_query_attention_gpu` (26), `vmm_kv_contiguous_tail_gpu` / `vmm_kv_layout_residency_gpu` / `vmm_kv_token_major_floor_gpu` (7 total) ## Scope / notes - Edits are confined to the capture/invalidation decision path (`native_decode/cuda.rs`) and the growth harness, to merge cleanly alongside concurrent work. - Never asserts inside a `Drop`; never maps/grows inside a captured region (commit happens between decode steps); never unmaps a live mapping (VMM keeps the base VA stable). - Per-run tok/s on this shared box is not published — identical runs range widely and other agents build concurrently; only the deterministic counters above are reported. Refs #805 #782 #797 #801 #804. Does **not** merge and does **not** close any issue. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
## What Documentation-only sync of five memory/KV design docs to the merged, executable-test-backed facts about VMM commit geometry, KV layout, weight offload, CUDA-graph capture, and plugin export. No code or tests changed. `MEMORY_ARCHITECTURE.md` is treated as the single source for KV geometry and the audit table; the other four link into it rather than restating (per #791). ## Most misleading claim found `MEMORY_ARCHITECTURE.md` — the document explicitly designated the single source of truth — still asserted the engine *"explicitly invalidates the graph on every KV bucket growth for both layouts"* and that *"the engine does not yet use that fact to retain the captured graph,"* with a table showing seq-major at `captures=4, invalidations=4`. That was falsified by **#811**: conditional invalidation has landed, seq-major fixed-stride growth now **keeps** the captured graph (growth-attributable invalidations **3 → 0**, `captures=1/invalidations=1`, matching the no-growth control), verified by `mobius_seqmajor_growth_parity_native_cuda.rs` including a negative oracle. Because this doc is the single source others link to, the stale claim had the widest blast radius. ## Changes - **MEMORY_ARCHITECTURE.md** — correct the #811 conditional-invalidation prose + table (verified against `cuda.rs` `binding_growth_signature`/`record_growth_decision` and the parity test); add the ORT-backend KV-routing + #762 plugin-export note (verified `allocator.rs:232` `for_session_device` selects by device, not implementation); thread #822 (granule queried), #812 (engine-side consumer gate), and a #800/#802 record correcting this doc's earlier fp16 sizing of Phase-2a scores (verified `attention.rs:368` uses `dtype.element_size()`). - **CUDA_GRAPH_CAPTURE.md** — replace the mutual-exclusion result with #796 (stable VA removes it), #798 (managed no-spill default), #811 (conditional invalidation), and the #804/#807 `captures=0` process-artifact correction. - **WEIGHT_OFFLOAD.md** — correct the "mutually exclusive today" claim (#796) and the north-star status for the managed no-spill default (#798). - **KV_INSERTION_DESIGN.md** (most stale, 2026-07-15) — add a synchronization banner linking to `MEMORY_ARCHITECTURE.md`; note the layout line post-dates the memo, prefix sharing is a landed primitive (#777), and block-table paged attention remains a proposal (verified: no `block_table` in any CUDA attention kernel). - **DESIGN.md** — forward-pointer in the already-superseded §39.5 noting layout (seq-major/token-major, #787/#792) is now the primary residency lever. ## Method Every retained performance claim cites the PR that measured it; measured fact / design intent / not-yet-built are distinguished; superseded designs are labelled, not deleted. The "still open" item (engine commits flat buckets, not the dense live-prefix ranges `kv_commit.rs` computes, so the `layers×2` floor is not yet realised through the engine) is described as in-flight, not delivered. ## Note on #762 The task framing had #762 (EP plugin export) as open; `gh pr view 762` shows it **merged** today. It is described accurately per that ground truth: the plugin-EP mechanism landed for the CPU EP (validated end-to-end), while the CUDA plugin shim compiles but is unvalidated on hardware — so plugin-EP-hosted VMM KV remains in-progress design intent, not a shipped capability. ## Validation Documentation only — `git diff --stat` touches exactly the five `.md` files; working tree otherwise clean. **Not merged; no issue closed.** Refs #736, #762, #776, #777, #783, #787, #791, #792, #794, #796, #797, #798, #801, #802, #804, #807, #811, #812, #822 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
…r); floor is granule-bound on qwen0.5b (#797 #801 #812 #811 #787 #777) (#827) ## Summary Realises the `layers × 2` KV floor **through the engine** by wiring the seq‑major fixed‑stride KV commit path through the shared `kv_commit::live_prefix_ranges` geometry, the last increment in the chain named by #797, #801 and #812. `DecodeCudaState::seq_major_kv_commit_requests` now *consumes* the same `kv_commit.rs` geometry the driver‑level residency test (`vmm_kv_layout_residency_gpu`) and the `kv_commit.rs` unit tests exercise, instead of duplicating the byte arithmetic inline — so the live commit geometry and the measured floor are single‑sourced and cannot drift (previously `kv_commit.rs` carried a `#![cfg_attr(not(test), allow(dead_code))]` because nothing in a non‑test build called it). Head‑major stays **byte‑identical**: on a growing packed bucket its per‑head stride *is* the bucket, so the `kv_heads` live‑prefix fragments tile the same contiguous `[0, bucket_bytes)` run — head‑major's dense ranges equal its bucket ranges. `vmm_growth_requests` is left as the flat bucket commit, now with a doc comment saying exactly that. Refs #797 #801 #812 #811 #787 #777. ## Headline: committed‑bytes comparison `physical_committed_bytes`, process‑isolated, `mobius_seqmajor_growth_parity_native_cuda`, qwen2.5‑0.5b: | Config (vmm‑inplace) | head‑major | seq‑major | |---|---:|---:| | capture ON, forced growth | 100,663,296 | 100,663,296 | | capture OFF, forced growth | 100,663,296 | 100,663,296 | | no‑growth control (`min_bucket=64`) | 100,663,296 | 100,663,296 | | realloc, forced growth | 786,432 | 786,432 | **The floor does not move on this model, and that is the correct result, not an un‑wired commit.** At `head_dim=64`/fp16 with a 512‑token hard max, a single binding's *entire reservation* is `512 × 2 × 64 × 2 = 131,072 B = 128 KiB`, far below the 2 MiB CUDA granule. So **both** layouts pin at exactly one granule per binding: `48 bindings × 2 MiB = 100,663,296 B`, which *is* the `layers × 2 = 48`‑granule floor. Seq‑major already sits on that floor; head‑major coincides because its 64‑token bucket is also sub‑granule. The `kv_heads×` separation (`vmm_kv_layout_residency_gpu`, qwen14b, 32,768‑token stride) only materialises once a **single head's live stripe crosses a granule** — `capacity × head_dim × elem ≥ 2 MiB`, i.e. ≈8,192 tokens at head_dim 128/fp16 (the measured head‑major crossover, #776). A 512‑max, 48‑token qwen0.5b run never approaches it, so #797's driver‑level `kv_heads×` win is **not reproducible end‑to‑end on this harness** — its geometry never leaves a single granule. This is the same class of precise negative result as #794 (identical bytes) and #812, stated with the same precision. ## Full per‑config counters (measured) | Config | growth_events | kv_bytes_moved | captures | replays | invalidations | growth_keeps | |---|---:|---:|---:|---:|---:|---:| | head vmm ON, growth | 3 | 688,576 | 4 | 39 | 4 | 0 | | seq vmm ON, growth | 3 | **0** | 1 | 45 | 1 | **3** | | head vmm ON, no‑growth control | 0 | 0 | 1 | 45 | 1 | 0 | | seq vmm ON, no‑growth control | 0 | 0 | 1 | 45 | 1 | 0 | | head vmm OFF, growth | 3 | 688,576 | 0 | 0 | 4 | 0 | | seq vmm OFF, growth | 3 | 0 | 0 | 0 | 1 | 3 | | head realloc OFF, growth | 3 | 688,576 | 0 | 0 | 4 | 0 | | seq realloc OFF, growth | 3 | 688,576 | 0 | 0 | 4 | 0 | - **Token IDs byte‑identical** across every configuration (`" Paris. It is the largest city…"`, 48 tokens). - Seq‑major keeps its captured graph across all 3 growths (`growth_keeps=3`, forced‑growth graph accounting `1/45/1` collapses onto the no‑growth control) — **the #811 keep decision is preserved**; the dense‑range commit changes neither device pointers nor physical shapes, so the four keep dependencies still hold. - Head‑major still moves 688,576 B and re‑captures per growth (`4/39/4`, `growth_keeps=0`) — unchanged. Wall‑clock intentionally omitted (deterministic counters answer the question; this box's identical runs vary 3.9–28 tok/s). ## Invariants held - Idempotent re‑commit; governor bills only newly mapped granules (via `seq_major_kv_commit_requests` → `commit_binding_ranges_with_mapped_growth`). - #748 `MappedGrowthGrant` growth path, #745 committed‑granule admission, #814 governor claim ordering, pre‑header 429 refusal (#743) — all unchanged. - No map/grow inside a captured region; no unmap under in‑flight replay; **no assert inside any `Drop`**. ## What this unblocks / does not - The dense‑prefix commit is now floor‑faithful and single‑sourced, so the `layers × 2` geometry is realised end‑to‑end **wherever a per‑head stripe exceeds a granule** (large `head_dim`/context). Demonstrating the `kv_heads×` win end‑to‑end needs such a model, **not** a change to this commit path. - It does **not** address the seq‑major seed/host‑mirror stride arithmetic (`read_present_kv` / `seed_prefix`), which still refuse under seq‑major (#812). Prefix sharing under seq‑major therefore still needs the BSNH seed as a separate follow‑on; the doc bullet is corrected to say so. ## Validation (all green, on an RTX 4060 Laptop, ORT 1.27, CUDA 13 wheels) - `cargo fmt -p onnx-genai-engine -- --check` ✔ (`--all-targets` full‑workspace fmt hits Windows os error 206, a command‑length artifact, not a formatting problem) - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` ✔ - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` ✔ - `cargo test -p onnx-runtime-session` ✔ (277 passed) - `cargo test -p onnx-genai-engine --features native-backend --lib` ✔ (454 passed; `kv_commit` 9/9) - **Mandatory** `mobius_seqmajor_growth_parity_native_cuda --ignored --test-threads=1` ✔ (2 passed, incl. the head‑major negative oracle) - `gqa_seqmajor_parity_gpu` ✔ (2), `gqa_shared_prefix_parity_gpu` ✔ (1), `group_query_attention_gpu` ✔ (26), `vmm_kv_contiguous_tail_gpu` ✔ (3), `vmm_kv_layout_residency_gpu` ✔ (2), `vmm_kv_token_major_floor_gpu` ✔ (2) — all `--ignored --test-threads=1`. Do not merge / do not close any issue. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
…ative) + #384 metadata generator (#834) ## Summary Demonstrates the KV-cache committed-**physical**-bytes floor end-to-end on **qwen14b** via the native CUDA decode path, comparing **head-major (BNSH)** vs **seq-major (BSNH)** layouts in the same session, swept across context capacities that bracket the 2 MiB-per-stripe granule threshold (#776). Token IDs are **byte-identical** between layouts at every point. This is the first time this line (chased since #797) has been driven through the *engine* on a model whose head stripe actually reaches a full granule (qwen14b @ 8192 ctx: `8192 x 128 x 2 = 2,097,152 B = exactly 2 MiB`), rather than the sub-granule qwen0.5b of #827. ## Headline result — honest negative (per #794 / #812 / #827 precedent) Head-major and seq-major commit **byte-identical** physical KV bytes at every swept capacity: | KV capacity (tokens) | committed bytes (both layouts) | granules | ratio seq/head | |---:|---:|---:|---:| | 1024 | 402,653,184 | 192 | 1.00x | | 2048 | 603,979,776 | 288 | 1.00x | | 4096 | 1,006,632,960 | 480 | 1.00x | | 8192 | 1,811,939,328 | 864 | 1.00x | Expected-if-it-separated: head-major `layers x 2 x kv_heads` = **768 granules (~1.5 GiB)** vs seq-major `layers x 2` = **96 granules (~192 MiB)** — the 8x that never appeared. ## Why it does not separate `growth_events == 0` and `committed_len == capacity` for **both** layouts: the engine commits the full KV bucket **eagerly** on reservation — reservation *is* the commit, so layout cannot change the total. Seq-major's fixed full-context stride *is* active and correct (`max_len == 8192` at every capacity vs head-major `max_len == capacity`, tokens match), which proves the seq-major kernel/layout resolution works — but a fixed stride only makes **growth** free (#797's 0 bytes moved) and preserves the captured graph (#811/#812); it does not shrink an eagerly-committed full bucket. The 768-vs-96 (8x) floor stays a **driver-level** property (`vmm_kv_layout_residency_gpu`) realized only by a **live-prefix commit-on-demand** path, which the engine does not do on initial reservation. The new guardrail assertions (`growth_events==0`, `committed_len==capacity`, `head==seq committed`) will **turn red** the moment a future change makes seq-major commit less — i.e. they are a tripwire for the desired win. Measurement used the in-place **VMM** allocator (`ONNX_GENAI_CUDA_VMM=1`), which reports granule-rounded **physical** bytes; the legacy/governor allocator reports nominal bytes and was deliberately avoided. Capture was declined throughout (`ONNX_GENAI_CUDA_GRAPH=0`); offload/weight-streaming (16.6 GB model vs 7.7 GB budget, #796) was active and did not perturb the KV commit counters. ## Deliverables 1. **`crates/onnx-genai-engine/tests/qwen14b_kv_floor_sweep_native_cuda.rs`** — child-process-isolated sweep harness (mirrors `mobius_seqmajor_growth_parity_native_cuda`), honoring process-frozen config (#804/#807). Self-execs per `(layout, capacity)`; asserts token parity + layout resolution + the deterministic guardrails above. `#[ignore]`, gated on `cuda,native-backend`. 2. **`scripts/gen_inference_metadata.py`** (#384) — emits `inference_metadata.yaml` from `genai_config.json` for any such export. Reproduces the hand-written qwen14b metadata **byte-for-byte**. Turns the one-off local `qwen14b-zp` fix (which unblocked native loading for this whole measurement) into a reproducible artifact; the model file itself stays local (models live outside the repo). 3. **Docs** — `MEMORY_ARCHITECTURE.md` floor-progression table + eager-full-bucket explanation; `MODEL_METADATA.md` #384 generator section. ## Method notes - Seq-major was obtained by patching a **copy** of the model (`kv_layout=1` on all 48 GQA nodes), as #801/#805 did for mobius — the original model directory was never modified. - Reported committed **physical** bytes throughout, never nominal content bytes. - Deterministic counters lead; wall-clock is omitted (this box's identical runs ranged 3.9-28 tok/s) and no number was extrapolated that was not measured. ## Unrelated pre-existing fix (disclosed) `crates/onnx-runtime-ep-cpu/src/kernels/matmul_nbits.rs`: added a `#[cfg(not(target_arch = "aarch64"))] let _ = dot_kernel;` guard in `borrowed_affine_int4_matmul` to silence a pre-existing x86 unused-variable lint that blocked `-D warnings` (matches sibling functions at 3779/4272/4448). Behavior-neutral; the mobius ignored test still passes (2 passed). ## Validation - `cargo fmt -p onnx-genai-engine -p onnx-runtime-ep-cpu -- --check` (per-package; `--all` hits Windows `os error 206`) - `cargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings` — pass - `cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warnings` — pass - `cargo test -p onnx-runtime-session` (full) — pass - `cargo test -p onnx-genai-engine --features cuda,native-backend --test mobius_seqmajor_growth_parity_native_cuda -- --ignored --test-threads=1` (#814 policy) — 2 passed - New sweep harness run twice; all guardrails passed. Refs #797 #787 #827 #384 #777. Do **not** merge; does **not** close any issue. --------- Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
justinchuby
added a commit
that referenced
this pull request
Aug 12, 2026
… baseline (#841) (#849) Follow-up to #841, correcting the single-source doc. #841 measured, end to end on qwen14b with on-demand commit **active**, that head-major and seq-major commit **byte-identical** physical KV. The mechanism is in `native_decode/cuda.rs`: head-major growth sets `new_shape[2] = new_capacity`, so its per-head stride is *the current bucket*, not a fixed full context. It re-packs dense and already sits on the `layers × 2` floor (96 granules) — **0.25× of the 768-granule figure this line of work has quoted since #797**. So the `kv_heads ×` ("8×") saving seq-major was pursued for does not exist on the path the engine actually runs, and no further layout work would have produced it. What this changes in the doc: - The floor table now distinguishes **head-major with a fixed full-context stride** (768 granules — a geometry the engine never instantiates) from **head-major as the engine actually runs it** (96 granules, measured). - The driver-level `vmm_kv_layout_residency_gpu` 8× ratio is still correct *as measured*, but is now explicitly marked as **not transferring to the engine**, since it is measured on that fixed stride. - The `layers × 2 × kv_heads` prose earlier in the section is now qualified with the stride condition it silently assumed. - Seq-major's real, measured advantage is recorded plainly: **growth cost, not residency** — 0 bytes moved and capture retained on growth, against head-major's 100,667,392 bytes moved plus invalidation. That is what the layout chain (#782/#792/#794/#797/#801/#812/#827) bought, and it is worth having; it is simply not a residency saving. Lesson recorded in the doc: **before quoting a floor, confirm the code instantiates the geometry that floor assumes.** The arithmetic was right throughout; the baseline was not. Notably `cuda.rs` has said head-major grows a *packed* bucket since #827 — the fact was in the tree before the measurement caught up 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.
Summary
Builds native CUDA KV state to grow along a fixed-stride BSNH byte layout when the resolved layout is seq-major, closing the last engine-side gap to a real end-to-end seq-major generation — the documented next step after #797, and the shared prerequisite for token-major and #777 page-level prefix sharing.
Yes — a real seq-major generation is now possible. On
qwen2.5-0.5b-q4_0-mobius(stock BNSH vs a copy withkv_layout=1on its 24 GQA nodes), forcing growth withONNX_GENAI_KV_MIN_BUCKET=8and generating 48 greedy tokens across 3 growth events, the head-major and seq-major token streams are byte-identical, capture requested OFF and ON, under both KV growth mechanisms.Rebased on current
origin/main(through #798/#799/#800). Fullcargo test -p onnx-runtime-sessionre-run green after the rebase (153 in the primary binary, 0 failures across all binaries).The precise structural finding
The gap was not the shape build — it was the growth byte geometry. The CUDA GQA node validates
past_key/past_valueas BNSH[batch, kv_heads, seq, head_dim]and readspresent_capacityfrom axis 2 (inputs[3].shape[2]/outputs[1].shape[2]) regardless of thekv_layoutattribute (only the kernel stride arithmetic re-specializes), andDeviceIoBindingrequires logical/physical shapes to share axis order. Sopersistent_state_shapesmust stay BNSH for both layouts — a permuted binding shape cannot express seq-major. Seq-major is realized purely in the growth/commit byte geometry.Changes (confined to
native_decode/cuda.rsgrowth functions)kv_growth_byte_layout()— maps the BNSH binding shape to the physical byte layout + grow axis the resolvedKvCommitLayoutstores it in. Head-major = declared shape, grow axis 2 (byte-identical to before). Seq-major =[batch, capacity, kv_heads, head_dim], grow axis 1, capacity-independentkv_heads*head_dimper-token stride.apply_vmm_growth/build_grown_buffers— drive the existing device copy/zero primitives over that byte layout instead of a hard-coded axis-2 scatter (which would corrupt BSNH).ensure_capacity— in-place VMM growth accounts 0 moved KV bytes for seq-major (fixed stride keeps the prefix in place) vsvalid_len*bytes_per_tokenfor head-major.persistent_state_shapes— documented the BNSH-metadata contract.Head-major is byte-identical to today (
default() == HeadMajor, absent metadata unchanged), asserted explicitly.Edits are intentionally confined to KV shape/stride growth geometry so they merge cleanly with the (now-landed) managed-VMM-default (#798) and GEMM-workspace-governance (#799) work.
Measurement (measured this session; interleaved, deterministic counters leading)
Since #798 made managed no-spill VMM the default, the default native CUDA growth path is now in-place VMM — so seq-major grows moving 0 KV bytes by default. The legacy reallocation allocator is reachable only via the #755 opt-out (
ONNX_GENAI_LEGACY_ALLOCATOR=1).FIXED-STRIDE RESULT: in-place VMM growth d2d KV bytes head-major=688,576 vs seq-major=0— the fixed-stride property Make native CUDA KV binding commit geometry layout-aware #797 measured at the driver level, now at the engine level on a real model, on the default path.kv_heads×residency win stays driver-level only (vmm_kv_layout_residency_gpu). Wiringkv_commit.rs::live_prefix_rangesinto the live commit path is a scoped follow-on (shared with token-major and VMM page-level prefix sharing: stop storing the same system prompt N times across concurrent requests #777).captures=0) — the same obstacle Enable stride-described seq-major flash prefill #794 hit.invalidate_graphstill runs on every growth (retained becausepresent_capacityis baked into any captured graph), so a capturing decoder would re-capture on each growth regardless of layout; the "0 re-captures" of Make native CUDA KV binding commit geometry layout-aware #797 needs a fixed full-context stable-VA reservation the bucket-growth path does not inherit.Validation
mobius_seqmajor_growth_parity_native_cuda(#[ignore], model-gated) — the honest option-1 validation (patch metadata, run both ways, compare token IDs), extended to actually exercise growth, under both allocators. All device ops route through the engine's own stream ordering; run in isolation, single-threaded.gqa_seqmajor_parity_gpu(independent CPU oracles),group_query_attention_gpu(25),vmm_kv_layout_residency_gpu,vmm_kv_token_major_floor_gpu,vmm_stable_va_weight_slot_gpu— all green single-threaded.cargo test -p onnx-runtime-session— green post-rebase (153, 0 failures).cargo clippy -p onnx-runtime-ep-cuda --features cuda -- -D warningsandcargo clippy --locked --all-targets -p onnx-genai-engine --features native-backend -- -D warnings— green.cargo fmt(changed packages) — clean.Refs #797, #794, #782, #750, #777. Does not flip any default; does not close any issue. Out of scope: token-major across layers, #777 prefix sharing, converting remaining BNSH kernels, and wiring the dense-prefix residency commit.