Conversation
Refactor free_swa method to handle SWA indices more effectively and ensure proper freeing of allocated slots.
LingZ315
requested review from
alphabetc1,
hnyls2002 and
ispobock
as code owners
August 24, 2026 04:01
5 tasks
|
Anecdotal evidence: seems to have corrected the issue on my end. DSV4 Flash, 4xB200, DP=TP=EP=4 with dp attention. |
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.
Motivation
Fixes #36081.
SGLang v0.5.18 can report a one-page SWA pool leak after grouped cache frees.
SWATokenToKVPoolAllocator.free_swa()previously deferred Full-pool indices and resolved their Full-to-SWA mapping only infree_group_end(). Tombstone recovery can replace that mapping in the same free group, so the flush frees the replacement SWA page while the old physical page becomes unreachable.I minimized this on an RTX PRO 5000 using the exact v0.5.18 Docker source revision. The unpatched allocator ended with 2 physical SWA pages in use but only 1 mapped page, reproducing the one-page discrepancy from the issue. The patched allocator preserves the invariant.
Modifications
free_group_end()), snapshot and detach the queued ownership so a tombstone recovery cannot replace it.free_group_end(), preserving one batched lookup/clear on the steady-state path.Accuracy Tests
This change does not affect model outputs.
Validated on an NVIDIA RTX PRO 5000:
main: 2 passed, 17 skipped.test_swa_unittest.py: 13 passed.The host does not provide
/usr/local/cuda/bin/nvcc, so optional HiCache JIT kernels used their existing fallback in the broader unified-cache run.Speed Tests and Profiling
Allocator-bookkeeping microbenchmark on an NVIDIA RTX PRO 5000 72GB Blackwell (PyTorch 2.11.0+cu130), with 50 warmups and 500 interleaved A/B samples. It mirrors grouped
free_swa()bookkeeping and excludes the identical physical allocator release. These steady-state cases contain no mapping replacement, so the baseline is correct and directly comparable. Values are medians per free group; wall time includes Python launch and synchronization overhead.The page-size-1 cases (1, 8, and 64 calls/group) had wall-time deltas of +0.5%, -0.1%, and +0.4%. Across all seven cases, the final implementation stayed within -0.1% to +0.5%, i.e. measurement noise. It achieves this by inserting a snapshot barrier only before a mapping update or group end instead of performing a gather/clear on every
free_swa()call.Checklist
CI States
Latest PR Test (Base): ⏳ Run #32713782397
Latest PR Test (Extra): ⏳ Run #32713782164
Latest PR Test (AMD ROCm 7.2): ⏳ Run #32713782533