feat(unified-memory): dense KV views for uniform-row MHA/SWA models - #34602
Conversation
5268948 to
945f2f8
Compare
|
/tag-and-rerun-ci |
945f2f8 to
192dc4b
Compare
| self._translate_kv_loc = getattr( | ||
| self.token_to_kv_pool_allocator, "translate_kv_loc_dense", None | ||
| ) or getattr(self.token_to_kv_pool_allocator, "translate_kv_loc", None) | ||
| # kernels need the kernel-facing id space. `translate_kv_loc_dense` is |
There was a problem hiding this comment.
Done — the added comment blocks across the series are trimmed to short mechanism notes, and the rationale moved into the commit messages.
| ) | ||
| return self.cuda_graph_swa_out_cache_loc[:n] | ||
|
|
||
| def _zero_cuda_graph_kv_translate_region( |
There was a problem hiding this comment.
why only the triton backend needs this?
There was a problem hiding this comment.
The helper is gone entirely. The canonical page-table builder introduced in #35247 is prefix-only per row — the v2p gather runs inside the live-prefix mask — so the hazard it guarded (translating an unwritten region) is structurally impossible for every backend, rather than patched in one.
| if self._translate_kv_loc is None: | ||
| return kv_indices | ||
| if filled_len is None: | ||
| filled_len = int(kv_indptr[bs].item()) |
There was a problem hiding this comment.
filled_len can never be None in your code. remove this sync.
There was a problem hiding this comment.
Gone with the helper — the whole _translate_kv_indices_ragged path was dropped. Reads are now translated inside the table build itself (#35247), so there is no post-fill translate to bound and no sync.
| ) | ||
| return kv_indptr | ||
|
|
||
| def _translate_kv_indices_ragged( |
There was a problem hiding this comment.
Let's revert changes for MTP because its implementation is complicated and will be reverted eventually. No one is going to use triton + mtp + unified memory. It only creates complexity.
There was a problem hiding this comment.
Done — both speculative-decoding fix commits are dropped and the series now carries zero speculative-decoding changes (git diff base..tip -- python/sglang/srt/speculative/ is empty on every PR in the stack). Spec x unified work moves to a follow-up PR.
|
/tag-and-rerun-ci |
2331e31 to
0bee02c
Compare
Add a builder that exposes a uniform-row MHA sub-pool as kernel-facing per-layer K/V views, generalizing the existing MLA per-layer-view mechanism to a page envelope of 2L row-blocks. Nothing calls it yet; this is a pure addition. - python/sglang/srt/mem_cache/layout/page_major.py: add `build_mha_views`. When K and V rows are equally wide, the page envelope `[L0K*ps | L0V*ps | ...]` is a uniform array of 2L row-blocks and is therefore itself a valid kernel-facing paged pool under `kernel_id(t) = (t // ps) * (ps * 2L) + t % ps`; folding each block's byte offset into the view's storage_offset makes every per-layer view a contiguous `(n_dense, head_num, head_dim)` tensor — the stock `MHATokenToKVPool` shape — so one shared block table serves all layers. Overlap safety and the tail-pad requirement are asserted at construction. - python/sglang/srt/mem_cache/unified_memory_pool.py: add `MHASubPoolSpec.is_uniform_row()` (K/V row bytes equal) and `blocks_per_page()` (2L, refusing asymmetric specs) — the geometry precondition the builder needs, stated on the spec that owns the layout. - test/registered/unit/mem_cache/test_unified_mha_views.py: new. Pins that the strided and kernel-facing builders describe the same physical envelope (cross-readback both directions), that one kernel-facing id addresses 2L cells without aliasing, that the spec's byte offsets equal the builder's block origins, and that tail-pad/asymmetric misuse fails at construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 024a1ae7db67ed0b3c7c132f6e7d2e2eaacea86f)
…osite `UnifiedSWATokenToKVPoolAllocator` gains the kernel-facing (kernel-facing) id surface the mamba composite already has — the last piece of allocator plumbing needed before per-layer views can be turned on. Both multipliers default to 1, where every new entry point collapses byte-identically to the physical translate, so the kernel-facing scaling is inert until a factory passes real multipliers. Three things fall out of routing the SWA rail through that surface. ONE IMPLEMENTATION. The swa translation was already written twice — the composite allocator's `translate_loc_from_full_to_swa` and `UnifiedSWAKVPool`'s method of the same name, each doing its own page math over the same v2p table — and scaling both by the multiplier would have made that a third copy of one formula. Both ENTRY POINTS stay, because their callers hold different objects: the attention backends reach it through `token_to_kv_pool`, the radix cache and disaggregation through the allocator. Only the arithmetic moves. THE CLAMP BECOMES SINGLE-SOURCED. That duplication already bit once: the pool-level copy shipped without the tombstone clamp the allocator copy had — a freed (-1) `v2p_swa` entry produced a NEGATIVE id, which a captured graph then read out of bounds — and the clamp had to be patched into the copy separately (sgl-project#35773). Delegating deletes the copy, so the clamp (to the reserved padding sink, 0) can never drift out of one of the two entry points again. INT64, LIKE EVERY OTHER ID. Delegating to `translate_kv_loc_for_kernel` adopts its dtype, dropping the `.to(torch.int32)` both copies carried. Nothing asks for that narrowing: every `swa_out_cache_loc` buffer the backends allocate is int64 (triton, fa3, flashinfer, trtllm_mha, aiter, xpu, ascend), so it was undone by the very next `copy_`; the non-shared pool's equivalent returns int64 (its `full_to_swa_index_mapping` is `torch.arange(..., int64)`); and every backend that genuinely needs int32 already calls `.to(torch.int32)` where it fills its own buffer. One rule now holds across the unified pool: virtual ids stay int32 only where upstream stores them (`req_to_token`), every id the allocator computes is int64, and narrowing happens at the kernel-ABI boundary that requires it. It also drops a silent truncation — the int32 form was safe only because `_assert_dense_id_bound` caps the kernel-facing space at 2^31, a bound the `2 * layer_num` multiplier eats into. - python/sglang/srt/mem_cache/multi_ended_allocator.py: accept `full_kernel_page_multiplier` / `swa_kernel_page_multiplier` and thread them into the sub-allocators; expose `{kernel_page_multiplier, full_v2p_page_table, translate_kv_loc_for_kernel}` mirroring the mamba composite (their presence is what selects a backend's dense-first path), plus a swa-side `{swa_kernel_page_multiplier, swa_v2p_page_table}` so the swa read table is built DIRECTLY from virtual ids rather than chained through full-physical. `translate_kv_loc_for_kernel` gathers with `torch.take`, which returns the shape of its INDEX, so the one call serves both a flat write loc and a whole 2-D page table — the swa rail translates whole tables, not just id runs. It writes into a caller's buffer when given one (cuda-graph capture needs a fixed address) and allocates otherwise; the only extra case is a caller passing one tensor as both index and `out`, which a gather forbids. `translate_loc_from_full_to_swa` becomes a one-line delegation. - python/sglang/srt/mem_cache/unified_memory_pool.py: the pool-level `translate_loc_from_full_to_swa` becomes the same one-line delegation, so it can no longer drift from the allocator's — the drift sgl-project#35773 had to patch. - test/registered/unit/mem_cache/test_multi_ended_allocator.py: pin the multiplier-1 byte-identical collapse (the arm every existing SWA model runs), the kernel-facing translate against `v2p[t // ps] * (ps * mult) + t % ps` with the physical translate held unscaled (compaction depends on it), the swa page_stride scale, the tombstone clamp at scaled stride, and that slot 0 stays the sink in both maps across alloc/free churn. int64 dtype pins, with the wrong-dtype regression guarding that an int32 `out=` raises, so a narrowed rail cannot come back unnoticed. - test/registered/unit/mem_cache/test_full_loc_fast_path.py: new `TestUnifiedSWATombstoneClamp` — a tombstoned entry must translate to 0, not a negative id, at page sizes 1 and 4 and at a scaled multiplier, over a real swa sub-allocator rather than a duck-typed stand-in. The scaled-multiplier case exists only on this surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Make the unified pool's MHA and SWA sub-pools expose per-layer views —
the same layout the MLA sub-pool already uses — so every layer's K and V read
through one contiguous `(n_dense, head, dim)` tensor and one shared block
table. The switch is atomic: the views, the ids that address them and the
sizing they need all have to move together, or an intermediate state writes
physical ids into view rows.
The row-block array exists only when K and V rows are equally wide, so
asymmetric-K/V models (MiMoV2: 192 / 128) can no longer run
`--enable-unified-memory` and are rejected at startup. MLA is unaffected: one
latent row per layer, no K/V width to reconcile.
- python/sglang/srt/mem_cache/unified_memory_pool.py: `MHASubPoolSpec` asserts
uniform rows at construction, gains the block count and the tail-pad
the views need; `_build_mha_views` calls `build_mha_views`;
`UnifiedMHATokenToKVPool` reshapes onto the kernel-facing row space (`size` in dense
rows, inherited stock read/write, envelope-copy relocation) and
`UnifiedKVPool` derives `view_tail_pad_bytes` from its specs instead of
taking it as a ctor arg, so no construction site can under-allocate it. Both
factories drop the pad argument and pass the kernel-facing `kernel_page_multiplier`.
New `unified_memory_supported_for_model` states the geometry precondition
for the server_args screen.
- python/sglang/srt/server_args.py: reject `--enable-unified-memory` for an
asymmetric-K/V MHA model up front, naming the four head dims, rather than
letting it fail later inside pool construction.
- python/sglang/srt/mem_cache/multi_ended_allocator.py: drop the
now-untrue "falls back at multiplier 1" wording from the dense-translate
docstrings.
- python/sglang/srt/mem_cache/layout/page_major.py: correct the asymmetric-K/V
branch's message — such models no longer fall back to the strided
page-major layout, they are screened out at startup by the check above.
- test/registered/unit/mem_cache/test_unified_mha_views.py: pool-level
cover — every MHA sub-pool comes back 3-D, the stock inherited write lands
on the same bytes as writes through independently built strided views (page
sizes 1 and 4), envelope-move parity, the view row count bound, the derived tail
pad, and construction-time refusal of an asymmetric spec. The store-path
case builds its pool and k/v/loc tensors on the platform's device: the
stock `set_kv_buffer` dispatch is decided by the platform
(`memory_pool._is_cuda`, resolved at import), not by the tensors it is
handed, so on a CUDA box CPU tensors would hit the CUDA-only
`sglang::store_cache` op — and running on-device there exercises the real
store kernel instead of the naive fallback. Every other case is byte-layout
arithmetic and stays on CPU.
- test/registered/unit/mem_cache/test_layout_compat.py: this file's subject is
the ENVELOPE, so it now builds its 4-D description from
`build_page_major_mha_views` rather than reading it off the unified pool,
which no longer exposes that shape.
- test/registered/unit/mem_cache/{test_unified_mla_views,
test_unified_mla_gpu_parity,test_unified_handout_zeroing}.py: drop the
now-removed `view_tail_pad_bytes` argument.
- test/registered/unit/server_args/test_page_major_backend_allowlist.py: pin
the startup screen — every backend rejected for an asymmetric-K/V MHA model,
and MLA exempt (Kimi-Linear reports head_dim 72 / v_head_dim 128 and runs
the unified pool today).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pass an explicit `kv_cache_layout` when constructing the unified MHA pool so the parent's env-driven layout selectors cannot claim it. `MHATokenToKVPool` reads `SGLANG_USE_HND_KVCACHE` / the vectorized-5d selector to pick both a layout label and a buffer SHAPE; this pool overrides the buffers itself, so under `SGLANG_USE_HND_KVCACHE=1` the parent would take the HND store path (4-D indexing) against 3-D per-layer views and raise `IndexError` at the first write. - python/sglang/srt/mem_cache/unified_memory_pool.py: pass `kv_cache_layout="page_major"` with the reason inline. - test/registered/unit/mem_cache/test_unified_mha_views.py: pin that the pinned label survives `SGLANG_USE_HND_KVCACHE=1` and `use_hnd` stays False. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… MHA pool Make the KV-transfer and CPU-offload entry points raise on the unified MHA pool. They all assume per-layer buffers indexed by TOKEN id (or read the `_kv_buffer_descs` table this pool does not build); the per-layer views are indexed by kernel-facing id, so each would silently mis-index instead of failing. Mirrors what `PageMajorMHATokenToKVPool` already does for its own layout. - python/sglang/srt/mem_cache/unified_memory_pool.py: raise NotImplementedError from `get_contiguous_buf_infos`, `get_cpu_copy`, `load_cpu_copy` and `set_kv_buffer_prefix_valid`, each naming the layout and what assumption it breaks. - test/registered/unit/mem_cache/test_unified_mha_views.py: pin that all four raise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The strided MHA K/V views are being removed (next commit): until the static-pool page-major layout is reimplemented over the per-layer views, --enable-page-major-kv-layout without --enable-unified-memory fails at startup with a clear message instead of building a pool it cannot serve. The flag, its docs, and the rest of the feature surface stay — the arm is temporarily broken, not removed. The static e2e cells convert to their unified-triton equivalents (the unified pool keeps the page-major layout, per-layer views). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The strided 4-D envelope K/V views have no remaining producer the project supports: the unified pool reads/writes its page-major envelope through per-layer views, and the static page-major arm is rejected at boot (previous commit). Deleted: the build_page_major_mha_views builder (a trimmed copy survives in test_unified_mha_dense_views as the byte oracle), the store_cache_4d Triton kernel + registry entry, move_kv_cache_native's 4-D branch, and _extract_kv_strides' 4-D branch (non-3-D buffers fail loud on the ValueError). PageMajorMHATokenToKVPool stays as a non-constructible stub so the feature surface (flag, docs, configurator wiring, comments) is untouched until the per-layer-view reimplementation. The page-major envelope Mamba state views stay — the unified pool stores its Mamba/KDA state through them. test_store_cache_4d.py is deleted outright: its kernel cases die with the kernel, and TestStoreCache4DThroughSetKVBuffer had been latently broken since the kernel-facing conversion (CUDA-only, 4-D indexing against the now-3-D per-layer views) — its claim is pinned by test_stock_write_lands_on_envelope_truth. The byte-identity claim from test_layout_compat moves into the kernel-facing suite as a direct per-layer-view-vs-envelope-formula pin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "how many row-blocks does one page hold in the kernel-facing id space"
number was written down three times and could disagree: `MHASubPoolSpec`
computed 2L, `_build_mla_views` open-coded `spec.layer_num`, and the
allocators took it as a ctor kwarg defaulting to 1. That default is the
dangerous one -- every MHA/SWA view is kernel-facing now, so a construction site that
does not pass the kwarg emits PHYSICAL ids into view rows, which is exactly
the intermediate state the per-layer-view switch must not have. Production
`init_unified_{mamba,swa}_pools` passed the right value, but the two sides
could drift independently and the tests already had.
Put the number on the spec that owns the layout and derive everything from
it, the same way `view_tail_pad_bytes` is already derived:
- `SubPoolSpec.blocks_per_page()` -> 1 (views are not dense; kernels
take real physical ids), `MHASubPoolSpec` -> 2L (a K block and a V block
per layer), `MLASubPoolSpec` -> L (one latent row per layer). This also
answers `_build_mla_views`'s open-coded `spec.layer_num` at the dense-id
bound: the bound is over BLOCKS, and for MLA the block count IS the layer
count.
- `MultiEndedAllocator` reads `spec.blocks_per_page()` instead of a
kwarg. `kernel_page_multiplier=` survives only as an explicit override, for
tests that pin the multiplier-1 collapse.
- `Unified{Mamba,SWA}TokenToKVPoolAllocator` drop their pass-through kwargs,
and the two factories stop computing the value. Behaviour-preserving: the
MLA arm passed `len(full_attention_layer_ids)`, which is `MLASubPoolSpec.
layer_num`; the MHA arms already called `blocks_per_page()`.
`test_multi_ended_allocator.py`: the composite tests stop injecting
multipliers and now pin the derivation itself. One of them was passing only
because of the default this commit removes --
`test_paged_pool_translate_helper_returns_physical_tokens` asserted that
`translate_loc_from_full_to_swa` equals the PHYSICAL helper
`_virt_tokens_to_phys_tokens`, which holds only at multiplier 1 and is false
for every real kernel-facing sub-pool. It now asserts the kernel-facing formula, and says why
the two id spaces must not be pinned together.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… needs it Uniform K/V rows were asserted three times for one property: the ServerArgs screen, `MHASubPoolSpec.__post_init__`, and `build_dense_mha_views`. Only two of those are reachable by anything, and only one of them is a boundary a caller can actually violate. Keep the ServerArgs screen -- it is the user-facing message, and it names the four head dims and the flag to drop. Keep the builder's assert -- its addressing arithmetic is what breaks, so it is where a caller that reaches it directly must be stopped. Drop the spec's, which sat between the two and could only fire for a model ServerArgs had already refused; `is_uniform_row()` goes with it, having had no other caller. The ServerArgs screen also stops re-testing `enable_unified_memory`: the assert immediately above it already established the flag, so the `if` was unconditional and read as though the check were one of two arms. test_unified_mha_dense_views.py: the refusal case moves to the builder, where it now covers a direct caller rather than a constructor. Drops `test_dense_blocks_is_k_and_v_per_layer`, whose assertion (`dense_blocks_per_page() == 2 * _L`) restated the one-line body of the method under test -- only editing the test and the method together turns it red, and the addressing / spec-offset cases already pin the 2L law against an independent derivation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ies on `UnifiedMHATokenToKVPool.move_kv_cache` builds its page-envelope view as `_raw[: num_pages * page_bytes]`, which is only this sub-pool's region because `UnifiedKVPool` happens to anchor every sub-pool at byte 0. The MLA twin makes the same assumption and states it; this one did not. State it here too. It is the one invariant whose violation is silent: the loc values stay in range, the move completes, and only the bytes are another sub-pool's -- so it would surface as an accuracy regression after a compaction, not as a crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Dense" only ever meant "not the strided envelope views". Those are gone, so the word now contrasts with nothing and just makes every name longer -- and worse, it was doing double duty: half the uses described a LAYOUT (per-layer contiguous views) and half described an ID SPACE (the ids those views are indexed by). Two different axes under one word. Layout: the word simply goes. `build_dense_mha_views` -> `build_mha_views`, `build_dense_mla_views` -> `build_mla_views`, `dense_blocks_per_page` -> `blocks_per_page`, `_dense_size` -> `_view_rows`, `n_dense` -> `n_rows`, and the `kv_cache_layout` label `page_major_dense` -> `page_major`. Id space: use the word the translator module already established for it -- kernel-facing. `translate_kv_loc_dense` -> `translate_kv_loc_for_kernel` (the pair now reads `translate_kv_loc` for physical, `_for_kernel` for what a kernel indexes with), `_assert_dense_id_bound` -> `_assert_kernel_id_bound`, `loc_is_dense` -> `loc_is_kernel_facing`, `_decode_dense_loc` -> `_decode_kernel_loc`, `_swa_write_loc_from_dense` -> `_swa_write_loc_unified`, and `dense(t)` -> `kernel_id(t)` in the addressing law. Two test files lose it from their names as well. Deliberately NOT touched: `build_page_major_mamba_views` and the prose around it. The Mamba conv/SSM state IS still a strided envelope view -- it is the KV side that stopped having two layouts -- and "strided" stays the right word there. Likewise every unrelated `dense` in the tree (dense vs MoE, dense vs sparse attention, dense vs ragged mamba layout, `first_k_dense_replace`); the rename matches whole identifiers over an explicit KV-layout file scope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Several blocks this series added were written for a reviewer, not for someone reading the code: why the design works, what it replaced, which failure it was a response to. That audience is gone the moment the PR merges, and the prose stays behind to go stale. Keep the fact, drop the argument, per .claude/rules/comment-style.md: - `build_mha_views` / `UnifiedMHATokenToKVPool`: the addressing law, the block assignment, the view overlap and its tail pad, and "compaction passes REAL physical ids" all stay -- a reader cannot recover any of them from the code. The paragraphs explaining why no inherited method needed overriding go. - `MultiEndedAllocator`: keep what the override is for, drop the derivation rationale. - `UnifiedMHATokenToKVPool.move_kv_cache`: keep the zero-anchor invariant and that violating it is silent; drop the retelling of the mechanism. - `PageMajorMHATokenToKVPool`: the docstring still described the 4-D strided views and the paths that used to fail loudly on them, both removed. A stale docstring is worse than none; what is left is that the class is currently non-constructible and why it still exists. - The two accuracy thresholds and the platform-dispatch trap in the tests keep their measured numbers and the trap; the incident history around them goes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Leftovers from the rename: the series calls its per-forward write target a "write loc" everywhere else, but a few comments still said "rail". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0bee02c to
6c88bee
Compare
…gl-project#34602) Co-authored-by: Caihua Li <caihua.li@bytedance.com> Co-authored-by: Cheng Wan <cheng.wan@radixark.ai>
…gl-project#34602) Co-authored-by: Caihua Li <caihua.li@bytedance.com> Co-authored-by: Cheng Wan <cheng.wan@radixark.ai>
…gl-project#34602) Co-authored-by: Caihua Li <caihua.li@bytedance.com> Co-authored-by: Cheng Wan <cheng.wan@radixark.ai>
…gl-project#34602) Co-authored-by: Caihua Li <caihua.li@bytedance.com> Co-authored-by: Cheng Wan <cheng.wan@radixark.ai>
Motivation
The unified memory pool stores MHA and SWA KV in a page-major envelope whose per-layer slices
are strided. Every attention backend therefore has to reconstruct a 4-D view of that envelope, and
in practice only Triton does — which is why the unified pool is Triton-only for these model families
today. The MLA sub-pool already stores dense per-layer views; this brings MHA and SWA to the
same layout, so every layer's K and V read through one contiguous tensor addressed by one shared
block table. That is the precondition for any other backend to read the pool at all.
The correctness fixes that used to lead this branch are gone from it: the SWA tombstone fix was
superseded upstream by #35773 (same bug, fixed via
clear_full_to_swa_mapping), and the twoTriton translate fixes were speculative-decoding-only in effect and moved out of this series
entirely (the series carries zero
speculative/changes; spec × unified work follows separately).Modifications
Makes the unified pool's MHA and SWA sub-pools expose dense per-layer views, matching the
layout the MLA sub-pool already uses, and moves the ids, the sizing and the pool doors onto that
layout together — the switch is atomic, because an intermediate state would write one id space
into the other's rows. The view tail-pad is derived from the sub-pool specs rather than passed in,
so no construction site can under-allocate it. Because a dense block array only exists when K and
V rows are equally wide, asymmetric-K/V MHA models are now rejected at startup with a message
naming the four head dims, instead of failing later inside pool construction; MLA is unaffected.
Accuracy Tests
GSM8K, unified pool vs. baseline.
Speed Tests and Profiling
Serving benchmark at matched batch, production configuration: radix
cache + overlap scheduler + cuda graphs +
page_size256 when possible.Two workloads:
heavy-decode(512-token prompts, 128 concurrent) andradix-retract(4k shared-prefix prompts, 192 concurrent, forcing eviction and retraction).
Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ✅ Run #33333034298
Latest PR Test (Extra): ❌ Run #33333034168
Latest PR Test (AMD ROCm 7.2): ⏳ Run #33333034282