Skip to content

[Fix] Allocate SWA KV pools inside the memory-saver region - #40979

Merged
ch-wan merged 3 commits into
mainfrom
jiajun/swa-kv-pool-memory-saver
Sep 29, 2026
Merged

ch-wan merged 3 commits into
mainfrom
jiajun/swa-kv-pool-memory-saver

Conversation

@guapisolo

@guapisolo guapisolo commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

release_memory_occupation now frees the KV cache of MHA, MLA and DSA sliding-window models.

Symptom & Reproduction

  • Symptom: with --enable-memory-saver, releasing kv_cache leaves the KV cache of SWA models resident, and every DSA model keeps its index-K buffers.
  • Engine measurement: an H200 TP2 engine of a hybrid SWA model used 85.3 GiB after load, 32.9 GiB after releasing everything, and 85.3 GiB after resuming only the weights, so only the weights were pausable.
  • Reproduction: build a pool through its KVCacheConfigurator builder with the flag on, pause the torch_memory_saver kv_cache tag, and read torch.cuda.mem_get_info; expected: the pool's bytes are freed; observed on the merge base: 0 GiB freed for all three SWA pool kinds.

Root Cause

  1. pause(GPU_MEMORY_TYPE_KV_CACHE) unmaps only buffers allocated inside a memory-saver region.
  2. _build_hybrid_swa_kv_pool never passes the flag, so SWAKVPool gives both sub-pools False.
  3. _build_hybrid_mla_swa_kv_pool hard-codes False in the explicit sub-pool kwargs it hands SWAKVPool.
  4. IndexKeyCache.__init__ allocates index-K outside memory_saver_adapter.region, in every DSA pool.

Fix

Each pool builder now passes get_exec().features.enable_memory_saver, as every other pool builder already does. The MHA builder uses a new SWAKVPool argument that is forwarded to both sub-pools and defaults to False. The hybrid MLA/DSA builder sets the flag in its explicit common kwargs, because explicit sub-pool kwargs bypass that argument. IndexKeyCache allocates index-K inside the GPU_MEMORY_TYPE_KV_CACHE region, like the MLA buffers of the same pool. With the flag off the adapter is a no-op, so allocation is unchanged. Unchanged: the NPU SWA path, LayerSplitIndexKeyCache.remote_buffer and the DSA compress-tail buffers.

Verification

  • pre-commit run --files on the three changed files at this head: passed.
  • Pause/resume on one H200 with torch_memory_saver 0.0.9.post1: pools built through the real builders (48 layers, 12 full and 36 SWA), freed GiB per tree. Run before the rebase at 29bc585196, whose three-file diff equals this head's except one deleted comment:
tree MHA SWA (21.0) hybrid MLA (5.9) hybrid DSA (6.3, index-K 0.39)
base 0 0 0
first commit only 21.2 0 0
builder change only 21.2 6.0 6.0
full change 21.2 6.0 6.4
  • After resume every paused buffer kept its data_ptr, read back zero, and accepted writes.
  • CI at 34ba83fa52 (this head minus the comment-only commit): PR Test Base passed; /rerun-test of the SWA and DSA pool unit tests passed at 29bc585196.
  • Not run: a full engine with a hybrid MLA/DSA model, or the NPU path.

Review Focus

  • index_key_cache.py: index-K now joins the kv_cache tag for every DSA model; resume zero-fills it, and flush_cache() runs right after the pause, so stale index-K is never read.
  • _build_hybrid_mla_swa_kv_pool: its explicit sub-pool kwargs bypass the SWAKVPool argument, so the flag must stay in common.

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ❌ Run #36200923877
Latest PR Test (Extra): ❌ Run #36200923762
Latest PR Test (AMD ROCm 10): ❌ Run #36200923733

@guapisolo

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Sep 24, 2026
@guapisolo guapisolo removed the run-ci CI: run the baseline test suite on this PR label Sep 24, 2026
@guapisolo

Copy link
Copy Markdown
Collaborator Author

/rerun-test test_dsa_pool_host_unit.py test_dsa_layer_split_broadcast.py test_swa_unittest.py registered/attention/unittests/dsa/test_dsa.py test_swa_pool_v_head_dim.py

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Results for /rerun-test test_dsa_pool_host_unit.py test_dsa_layer_split_broadcast.py test_swa_unittest.py registered/attention/unittests/dsa/test_dsa.py test_swa_pool_v_head_dim.py:

🚀 1-gpu-5090 (1 test): ✅ View workflow run

cd test/ && python3 registered/unit/mem_cache/test_dsa_pool_host_unit.py

🚀 4-gpu-b200 (2 tests): ✅ View workflow run

cd test/ && python3 registered/unit/mem_cache/test_dsa_layer_split_broadcast.py
cd test/ && python3 registered/attention/unittests/dsa/test_dsa.py

🚀 1-gpu-h100 (2 tests): ✅ View workflow run

cd test/ && python3 registered/unit/mem_cache/test_swa_unittest.py
cd test/ && python3 registered/attention/unittests/dsa/test_dsa.py

🚀 ubuntu-latest (1 test): ✅ View workflow run

cd test/ && python3 registered/unit/mem_cache/test_swa_pool_v_head_dim.py

@guapisolo

Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Sep 24, 2026
@guapisolo
guapisolo force-pushed the jiajun/swa-kv-pool-memory-saver branch from 29bc585 to 8d6a9ee Compare September 24, 2026 19:59
guapisolo and others added 2 commits September 24, 2026 15:08
SWAKVPool hard-coded enable_memory_saver=False for its full and SWA sub-pools and the
MHA sliding-window configurator never passed the flag, so for SWA models the KV cache
was allocated outside torch_memory_saver's region and
release_memory_occupation(kv_cache) was a no-op. Measured on an H200 TP2 engine of a
hybrid SWA model: 85.3 GiB after load, 32.9 GiB after release(all), 85.3 GiB after
resume(weights), i.e. only the weights were pausable and a colocated Miles trainer had
no memory left for its own init. Thread the flag through SWAKVPool and pass the server
setting from the configurator, like every other KV pool class does.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…he memory-saver region

The hybrid MLA SWA builder passes explicit sub-pool kwargs to SWAKVPool
with enable_memory_saver hard-coded to False, so neither MLA/DSA
sub-pool was allocated inside the torch_memory_saver region and
release_memory_occupation(kv_cache) left their KV cache resident. Pass
the server setting there, as the MHA SWA builder now does.

IndexKeyCache allocated the DSA index-K buffers outside
memory_saver_adapter.region(), so every DSA pool, hybrid or not, kept
its index-K resident after a release even with the flag on. Allocate
them inside the KV_CACHE region, like the MLA buffers of the same pool.

Measured by pausing the kv_cache tag on pools built through the
configurator builders on one H200 (48 layers, 12 full / 36 SWA): hybrid
MLA freed 0 -> 6.0 GiB of 5.9 GiB; hybrid DSA freed 0 -> 6.4 GiB of
6.3 GiB, 0.39 GiB of which is index-K.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@guapisolo
guapisolo force-pushed the jiajun/swa-kv-pool-memory-saver branch from 8d6a9ee to 34ba83f Compare September 24, 2026 22:08
@guapisolo guapisolo removed the run-ci CI: run the baseline test suite on this PR label Sep 24, 2026
The comment restated why the change was made and claimed an unconditional
invariant that the parameter's False default does not enforce; the PR body
carries that rationale. No behavior change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@guapisolo

Copy link
Copy Markdown
Collaborator Author

cc cmt

CI record for this PR:

The current head c7db69c only removes a code comment on top of 34ba83f; its CI run stopped at the maintenance-mode rebase check and ran no tests.

guapisolo added a commit that referenced this pull request Sep 29, 2026
@ch-wan
ch-wan merged commit f0e4001 into main Sep 29, 2026
93 of 110 checks passed
@ch-wan
ch-wan deleted the jiajun/swa-kv-pool-memory-saver branch September 29, 2026 23:29
ziang-and pushed a commit to zianglih/sglang that referenced this pull request Oct 2, 2026
… graph registration (sgl-project#40648, sgl-project#40978, sgl-project#40979, sgl-project#40980, sgl-project#40977, sgl-project#41972)

Retain colocated training fixes above the release MiMo implementation, including address-stable Marlin reload and mandatory TopK layer IDs. Disable NCCL graph registration for TP LM-head all-to-all and pausable graph pools.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants