Unify full→SWA index translation in init_forward_metadata; drop pool caches - #27091
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1363d59a24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/tag-and-rerun-ci |
2d85528 to
866b5a0
Compare
4ef5862 to
5deade1
Compare
|
/rerun-test test_deepseek_v4_flash_fp4_b200.py |
|
Results for 🚀 |
|
/rerun-test test_deepseek_v4_flash_fp4_b200.py |
This reverts commit 3ea4b29861ee15c9da0b1ecc77e85a98efed7860.
Deferring the eager draft-extend init to after prepare_mlp_sync_batch
changed the init-time batch state for EVERY backend, and NSA/DSA cannot
consume the padded batch: core fields are padded (batch_size reassigned,
seq_lens padded) while spec_info / extend_seq_lens_cpu are not, so the
deep_gemm fp8_paged_mqa_logits schedule metadata disagrees with its
companion tensors ("_batch_size == batch_size" assert on the DeepSeek-V3.2
--dp 8 --enable-dp-attention EAGLE test).
Restore main's pre-pad init timing. The DSV4 store crash this commit
originally addressed is fully handled by get_swa_out_cache_loc: the
pre-pad cached value fails the length check against the re-padded
out_cache_loc and the store falls back to store-time translation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR sgl-project#27091 (Unify full->SWA index translation; drop pool caches) drops all `token_to_kv_pool.invalidate_loc_cache()` calls because the SWA loc-cache itself was removed from the memory pools. cg-refactor inherited those call sites from the pre-split runner files; reapplied the deletion to our split files: - model_runner.py: the decode-replay call site - runner/decode_cuda_graph_runner.py: capture run_once - speculative/eagle_draft_extend_cuda_graph_runner.py: capture run_once - speculative/multi_layer_eagle_draft_extend_cuda_graph_runner.py: capture run_once The modify/delete conflicts on legacy `breakable_cuda_graph_runner.py` / `piecewise_cuda_graph_runner.py` were resolved by keeping the deletion (cg-refactor split those into runner/ + runner_backend/). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI (base-c-test-8-gpu-h200 / test_dsa_glm5_dp_mtp: GLM-5 dsa backend, --dp 8, EAGLE NextN) crashed in the draft-extend forward with deep_gemm '_batch_size == batch_size' from the DSA indexer's fp8_paged_mqa_logits schedule_meta. This is exactly the failure #27091 documented and reverted: re-planning a DP-padded batch feeds NSA/DSA a padded state its indexer schedule_meta cannot consume. The two replan_equivalent=True opt-in sites (eagle_info_v2 draft-extend, multi_layer_eagle_worker per-step) did precisely that post-pad re-plan. Drop the opt-in: draft-extend keeps its marked pre-pad metadata (no re-plan), matching the proven skip_attn_backend_init=True behavior; trtllm-MLA's defensive backstop continues to cover pre-pad-then-padded staleness. The plan-record + replan_equivalent mechanism stays but now has no production caller (dormant until a backend can tolerate a post-pad re-plan). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Same DSA fix as the eagle_info_v2 / mlv1 sites: a DP-padded re-plan of the draft-extend breaks the DSA indexer's schedule_meta (#27091). Keep the marked pre-pad metadata. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Incoming changes from origin/main include: - PR sgl-project#27192 retired DecodeInputBuffers/PrefillInputBuffers in favor of CudaGraphBufferRegistry. cg-refactor still uses both: the typed DecodeInputBuffers wrapper backs the registry's source=... adoption. Resolved the runner/decode_cuda_graph_runner.py conflict to keep cg-refactor's DecodeInputBuffers.create(...) + share_buffers() pattern while pulling in upstream's new _allocate_decode_buffers helper and its NgramEmbeddingInfo / envs references. - PR sgl-project#26676 moved SWATokenToKVPoolAllocator to mem_cache.allocator.swa; updated kv_canary/api.py and flashinfer_backend.py imports accordingly while keeping cg-refactor's cuda_graph_config imports. - PR sgl-project#27091 SWA loc-cache deletions in model_runner.py already merged in the previous merge round; the modify/delete conflicts on the legacy breakable_/piecewise_cuda_graph_runner.py files were resolved by keeping cg-refactor's deletion (split into runner / runner_backend). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sync unified-KV attention branch with main (109 commits). Conflicts resolved in: - deepseek_v4_backend_hip_radix.py: keep unified methods (_attach_unified_kv_decode_streams/_forward_unified_kv) AND main's new get_swa_out_cache_loc (sgl-project#27091 unified full->SWA translation). - deepseek_v4.py: non-unified SWA-store now uses backend.get_swa_out_cache_loc (main sgl-project#27091) instead of the removed pool get_cached_swa_loc; unified path keeps its own ring addressing. - deepseek_v4_memory_pool.py: keep get_unified_kv; drop _should_cache_swa/ cached_loc pool caches (removed by main sgl-project#27091). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| # The compress kernel requires an int32 write location. | ||
| out_loc = compress_kv_pool.translate_loc_to_hisparse_device( | ||
| out_loc | ||
| ).to(torch.int32) |
There was a problem hiding this comment.
we should prefer int64 for any indices.
PR #27091 promised to compute the full->SWA out_cache_loc translation once per forward in the attention backend's metadata init and pass it to the KV store, but it only did so for the DeepSeek-V4 path. The general SWAKVPool path kept re-translating out_cache_loc inside SWAKVPool.set_kv_buffer once per SWA layer (the (data_ptr, numel) memo cache having been removed without a replacement). Finish the unification for every SWAKVPool-using backend: - SWAKVPool.set_kv_buffer takes a pre-translated swa_loc and uses it directly for SWA layers; it asserts swa_loc is provided (no internal translate). - Each backend translates out_cache_loc once per forward and stores it on its forward metadata as swa_out_cache_loc, mirroring how swa_page_table is cached. Eager paths compute it in init_forward_metadata; cuda-graph paths bind a pre-allocated buffer that is refilled from the live out_cache_loc before each replay (so replay re-reads live slots), never a reassigned tensor. This includes FA's topk>1 target-verify branch. - Covered backends: FlashAttention, Triton, FlashInfer, trtllm-gen, aiter, xpu (consistent by construction, no separate cuda-graph metadata path), and musa (inherits FlashAttention). Adds CPU unit coverage for SWAKVPool.set_kv_buffer (swa_loc passthrough + assert). Validated e2e on gpt-oss-20b (FA3 + SWAKVPool + cuda graph) and via the swa/{triton,flashinfer,torch_native} attention unittests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #27091 promised to compute the full->SWA out_cache_loc translation once per forward in the attention backend's metadata init and pass it to the KV store, but it only did so for the DeepSeek-V4 path. The general SWAKVPool path kept re-translating out_cache_loc inside SWAKVPool.set_kv_buffer once per SWA layer (the (data_ptr, numel) memo cache having been removed without a replacement). Finish the unification for every SWAKVPool-using backend: - SWAKVPool.set_kv_buffer takes a pre-translated swa_loc and uses it directly for SWA layers; it asserts swa_loc is provided (no internal translate). - Each backend translates out_cache_loc once per forward and stores it on its forward metadata as swa_out_cache_loc, mirroring how swa_page_table is cached. Eager paths compute it in init_forward_metadata; cuda-graph paths bind a pre-allocated buffer that is refilled from the live out_cache_loc before each replay (so replay re-reads live slots), never a reassigned tensor. This includes FA's topk>1 target-verify branch. - Covered backends: FlashAttention, Triton, FlashInfer, trtllm-gen, aiter, xpu (consistent by construction, no separate cuda-graph metadata path), and musa (inherits FlashAttention). Adds CPU unit coverage for SWAKVPool.set_kv_buffer (swa_loc passthrough + assert). Validated e2e on gpt-oss-20b (FA3 + SWAKVPool + cuda graph) and via the swa/{triton,flashinfer,torch_native} attention unittests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…caches (sgl-project#27091) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Motivation
The full→SWA index translation (
translate_loc_from_full_to_swa) was triggered in two inconsistent styles:out_cache_loclazily at the first SWA layer viaDeepSeekV4TokenToKVPool.get_cached_swa_loc(env-gated bySGLANG_OPT_CACHE_SWA_TRANSLATION), with the result cached across layers.SWAKVPooltranslated on demand and memoized with a(data_ptr, numel)cache, kept consistent with ainvalidate_loc_cache()call sprinkled through allocators, cuda-graph runners, spec draft runners, andmodel_runner.This unifies the design: compute the translation once per forward in the attention backend's metadata init and store it on the backend metadata, then drop the lazy/per-layer caches and the
invalidate_loc_cachemachinery that existed to keep them coherent across cuda-graph capture/replay.Modifications
init_forward_metadata_in_graph, cached on metadata. The SWA-translated KV-store write target is computed in the in-graph init op and stored onDSV4AttnMetadata.swa_out_cache_loc. Because the compute is recorded inside the cuda graph, replay re-reads the liveout_cache_locbuffer (spec-v2 and DP padding rebindout_cache_locafter the out-graph prep). Multi-step draft decode mirrors the eager init's per-step slice. The field lives inassign_fields(recomputed every forward, not copied across replays) and_CP_GLOBAL_FIELDS. Applied to bothdeepseek_v4_backend.pyanddeepseek_v4_backend_hip_radix.py.get_swa_out_cache_loc(forward_batch). All five KV-store sites (model fused stores ×2,_compute_kv_to_cache, backendstore_cache×2 twins) call one resolver: it returns the cached value only when provably current (non-idle and length matchesout_cache_loc), and otherwise falls back to store-time translation — exactly the pre-cache behavior. This keeps every path correct even when the in-graph init never runs: eager IDLE (forward_idleskips attn init but the nextn model still stores dummy tokens), the draft-extend graph runners (EAGLEDraftExtendCudaGraphRunner/ multi-layer / frozen-kv only run the out-graph prep), and any batch re-padded after init.prepare_mlp_sync_batch, but that fed every backend's init a padded batch state NSA/DSA cannot consume (deep_gemm schedule-meta batch mismatch on the DeepSeek-V3.2--dp 8 --enable-dp-attentionEAGLE test), so it was reverted. The pre-pad staleness that motivated it (the deepep 4×B200 DSV4-Flash "expected 64 but got 62" crash) is instead handled by the resolver's length check, which falls back to store-time translation when DP padding rebindsout_cache_locafter init.translate_*.translate_loc_from_full_to_swa/translate_loc_to_hisparse_devicereturn plain int64 lookups. Backends whose kernels require int32 convert at the metadata-build site: flash_mla (swa_out_cache_loc,swa_page_indices), FA3 (swa_page_table— eager paths previously passed int64 and crashedflash_attn_with_kvcache), trtllm-gen (_maybe_translate_swa— int64 was silently misread as int32, collapsing gpt-oss GPQA accuracy), and dsa/indexer hisparse page tables. int64-tolerant backends (triton, flashinfer, torch-native) take the mapping dtype as-is.get_cached_swa_loc,SGLANG_OPT_CACHE_SWA_TRANSLATION, theSWAKVPool(data_ptr, numel)cache, andinvalidate_loc_cache— including all call sites incuda_graph_runner,piecewise_cuda_graph_runner,breakable_cuda_graph_runner, the eagle/mtp draft runners,model_runner, and triton'supdate_sliding_window_buffer. The per-layer gather is captured correctly by cuda graph, so the invalidate-before-capture dance is no longer needed.test/manual/coretests that asserted the removed cache machinery; updatedtest_swa_alloc_extend_page_estimation.pyand thedsv4_attentiontest kit for the new helper signatures; added direct unit coverage for the four resolver branches (no-metadata, current-cached, stale-shape, idle).Accuracy Tests
Run against this branch (
PYTHONPATHpointed at the worktree so the changes are actually exercised):test/registered/attention/unittests/— 185 passed, 21 skipped, 548 subtests passed (includes cuda-graph SWA decode/verify, DSV4 capture/replay, C4/C128, dsa, dense, mla, production eagle draft / draft-extend runners, and the new resolver tests).test/registered/unit/mem_cache/(swa) — 614 passed.Speed Tests and Profiling
No dedicated benchmark. The change removes per-iteration cache bookkeeping and the
invalidate_loc_cachecalls; DSV4 now does the SWA write-target translation once per forward in the metadata init rather than lazily per first-SWA-layer. Paths that fall back to store-time translation (eager idle, draft-extend graph runners) match the pre-cache behavior.Checklist
🤖 Generated with Claude Code
CI States
Latest PR Test (Base): 🚫 Run #26912813528
Latest PR Test (Extra): ❌ Run #26912814327