Repository navigation
Conversation
…tch through the workspace manager _merge_dcp_topk_global() allocated (1 + 2 * dcp) * 8 * T * K bytes per call from the plain allocator: the packed candidates, the all-gather output and the copy that moves the rank dimension inside. The profile run never calls the merge, so the KV cache was sized as if it cost nothing, and with T = 8192, K = 2048, dcp = 4 the first long prefill needed 1.13 GiB per rank that was not there (OOM in the all-gather). Merge DCP_TOPK_MERGE_ROWS rows at a time through buffers taken from the workspace manager, gather in place on the group's communicator, and add the buffers to the indexer's profiling specs so the profile run reserves them. The scratch is bounded at 144 MiB per rank for the numbers above, shares the prefill workspace allocation with the gathered K it must not overlap, and the KV budget sees it. Fixes vllm-project#59317 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Mikhail Kostryukov <mike@triptrack.net>
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Resolves the conflict with vllm-project#54951 in sparse_attn_indexer.py: keep both the merge workspace split and the TP row shard, and pass the narrowed cu_seqlen_ks as row_starts. Row sharding requires dcp_world_size == 1, so it never runs together with the DCP top-k merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Mikhail Kostryukov <mike@triptrack.net>
This branch has not been deployed
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.
Purpose
Fixes #59317.
With DCP > 1 the sparse indexer merges each rank's local top-k into the global top-k in
_merge_dcp_topk_global. The merge allocated three transients from the plain allocator per call:packed(T, K, 2)fp32, the all-gather output(dcp * T, K, 2)and the copy the communicator'sall_gathermakes to move the rank dimension inside. Peak(1 + 2 * dcp) * 8 * T * Kbytes per rank, 1.13 GiB forT = 8192, K = 2048, dcp = 4, on top of the logits. The profile run takes the indexer's profiling branch, which never calls the merge, so the KV cache was sized as if the merge cost nothing and the engine found out on the first long prefill (torch.OutOfMemoryError ... in _merge_dcp_topk_global -> all_gather, 512 MiB is the gather output for those numbers).This PR takes both routes proposed in the issue:
DCP_TOPK_MERGE_ROWS(1024) at a time, so the scratch is bounded at(1 + 2 * dcp) * 8 * 1024 * Kbytes (144 MiB for dcp 4, K 2048) whatever the chunk size; the pack kernel takesrow_startsper row and the selector is row-independent, so the result is unchanged;_dcp_topk_merge_specs): the prefill path takes it in the sameget_simultaneouscall as the gathered K, which stays live across chunks and must not be overlapped; the decode path takes its own. The all-gather goes in place into the reserved rank-major buffer on the group's communicator (pynccl when enabled,dist.all_gather_into_tensorotherwise), and the rank dimension is moved with one stridedcopy_into the row-major buffer the CuteDSL selector reads;profile_specswhendcp_world_size > 1, so the profile run reserves the scratch and the KV budget sees it.Related: #59318 is the CUDA graph memory over-estimate that happened to cover this transient on our host (about 1 GiB per rank); its fix is #59368. Either fix alone changes the failure boundary, so they were validated together.
Not a duplicate: #47348 optimizes the CuteDSL merge kernels themselves and leaves the buffers as they are; #59211 is DCP for the kpool indexer, a different code path; #55132 is about the indexer reserving too much for the decode logits on ROCm, this is about a buffer the profiling run did not reserve at all.
Test Plan
CPU unit tests (no GPU), on
vllm-openai:nightlyaf7f948 with the PR'ssparse_attn_indexer.pymounted over the package:test_dcp_topk_merge_specs_bound_the_scratchchecks the reservation shapes and the(1 + 2 * dcp) * 8 * rows * Kbound;test_dcp_topk_merge_walks_rows_through_the_workspaceruns the merge with faked kernels and all-gather over2 * DCP_TOPK_MERGE_ROWS + 5rows and checks the row slices, that every buffer is a contiguous view of the given workspace, the rank-major to row-major layout the selector receives, and the output. The existing GPU harness_merge_local_topks_global_with_fake_dcp(CUDA + CuteDSL) is updated to fake_dcp_all_gather_intochunk by chunk; those tests need a free GPU and were not run in this round.Serving: GLM-5.3-NVFP4, 4x H200 NVL, TP4 DCP4 EP, MTP 3,
fp8_ds_mla,max-model-len 786432,max-num-batched-tokens 8192,max-num-seqs 32,gpu-memory-utilization 0.945, on nightly af7f948 with this PR and the fix for #59318.Test Result
Unit tests: 2 failed on the base image (no
DCP_TOPK_MERGE_ROWS), 2 passed with the PR.Serving, both fixes: the CUDA graph estimate is exact (0.91 GiB pool estimated and captured) and the KV cache is 13.25 GiB / 1,039,616 tokens per rank (13.32 GiB / 1,045,248 before, with the 1 GiB over-estimate that used to cover the merge). Fresh prefill 8x32768: 65.7 s, 5092 tok/s (5129 tok/s on the unpatched production image the same night); 2x131072: 69.7 s, 4941 tok/s (4988 before). Decode N=1 110.7, N=8 329.8, N=32 761.1 tok/s. The decode figures are single samples; across seven boots of the unpatched image that night N=8 ranged 263 to 317 tok/s and N=32 701 to 838 tok/s, so no decode change is claimed either way. Stress 3x600k: cold 3/3 in 373.8 s (370.8 s before), replay from CPU offload 3/3. Needle 200k at depth 0.1: retrieved, so the chunked merge selects the same tokens. The needle ran concurrently with the stress, so the replay wall time (47.3 s against 2.4 s before) is contention with the needle prefill, not a cost of the fix. GPU memory in use while serving: 141.0 GiB of 143.77 per GPU, against 142.4 GiB before (the merge transient no longer lives in the free memory).
Second boot,
VLLM_MEMORY_PROFILER_ESTIMATE_CUDAGRAPHS=0with this fix only, the combination that OOMed on the first 8x32k prefill before: KV cache 15.34 GiB / 1,203,712 tokens, fresh prefill 8x32768 in 63.0 s (5103 tok/s), 2x131072 in 72.6 s, decode N=1 119.1, N=8 279.2, N=32 823.9 tok/s, no errors. 0.8 GiB free per GPU while serving, so that is the proof that the merge no longer needs the slack, not a recommended setting.AI assistance
AI assistance (Claude Code) was used for the analysis, the patch, the tests and this description.