[ROCm] Enable DSv4 multi-stream compressor overlap on HIP - #39314
Open
karverma-amd wants to merge 1 commit into
Open
karverma-amd wants to merge 1 commit into
karverma-amd wants to merge 1 commit into
Conversation
The multi-stream compressor overlap scaffolding exists but is inert on ROCm: model_hook force-disables SGLANG_OPT_USE_MULTI_STREAM_OVERLAP on HIP (overriding an explicit user setting), the per-layer alt_streams gate only wires _is_cuda/ _is_npu, and _forward_prepare_multi_stream_hip still writes the non-unified SWA buffer that crashes on the unified KV pool (no swa_kv_pool, see sgl-project#38662). This makes the overlap opt-in on HIP (default unchanged): - model_hook: honor an explicit env instead of clobbering it to False. - deepseek_v4: add _is_hip to the per-layer alt_streams gate. - _forward_prepare_multi_stream_hip: route Q/KV through the shared _forward_prepare_qkv helper (backend-correct unified_kv store) instead of the drifted SWA write; returns (q, kv). - SplitKBufferPool: key the split-K decode scratch per stream so concurrent side-stream attention cannot clobber it. MI355X (gfx950) DeepSeek-V4 TP8, 8k in / 1k out: -4.5 to -8.5% TPOT at conc 1-8, neutral at higher concurrency. GSM8K unchanged. The ROCm benefit requires the device-resident ordering-edge runtime (ROCm/rocm-systems#11212). Co-authored-by: Cursor <cursoragent@cursor.com>
karverma-amd
marked this pull request as ready for review
September 13, 2026 18:58
karverma-amd
requested review from
BBuf,
DarkSharpness,
HaiShaw,
HydraQYH,
celve and
yuan-luo
as code owners
September 13, 2026 18:58
Contributor
Author
|
Depends on #39282 (device-resident ordering-edge runtime, ROCm/rocm-systems#11212). That runtime is what makes this overlap a win on ROCm; on a stock nightly without it, the multi-queue dispatch contention turns the same code into a regression. |
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.
Summary
The DSv4 multi-stream compressor overlap (
SGLANG_OPT_USE_MULTI_STREAM_OVERLAP) is present but inert/broken on ROCm:arg_groups/model_hook.pyforce-disables the env on HIP, overriding an explicit user setting.alt_streamsgate indeepseek_v4.pyonly wires_is_cuda/_is_npu, so on HIP every layer getsalt_streams = Noneand the fork never engages._forward_prepare_multi_stream_hipstill writes the non-unified SWA buffer, which crashes on the unified KV pool (swa_kv_pool is None, see fix(rocm): support unified KV in DSV4 multi-stream #38662).This PR makes the overlap opt-in on HIP (default behavior unchanged — it stays off unless the env is set):
if not is_set(): set(False))._is_hipto the per-layeralt_streamsgate._forward_prepare_qkvhelper (backend-correctunified_kvstore) instead of the drifted SWA write; now returns(q, kv).Performance
MI355X (gfx950), DeepSeek-V4, TP8, 8k in / 1k out,
GPU_MAX_HW_QUEUES=4, DSPARK, fp8 KV. Baseline = overlap off; Patch = overlap on (same image,SGLANG_OPT_USE_MULTI_STREAM_OVERLAP=0/1).Low-concurrency win (−4.5 to −8.5% TPOT at conc 1‑8, matching the vLLM/ATOM 8k low-conc band); neutral at higher concurrency (the compressor is a smaller fraction of a larger step). No effect on TTFT (decode/verify-only).
Command (perf)
Accuracy (GSM8K)
benchmark/gsm8k/bench_sglang.py --num-questions 1319 --parallel 1319, greedy (no spec decode), real outputs (no acceptance simulation):Δ = 0.3% (≈4/1319) — within run-to-run nondeterminism (fp8 KV / MoE reductions), 0 invalid on both. The overlap only reorders which stream runs the compressor GEMM; the math is unchanged.
Command (accuracy)
Profile (conc 4, 8k/1k) — why it wins on the ordering-edge runtime
Enabling the overlap moves ~128 ms of compressor work off the main stream onto 4 active side streams; wall drops −6.3%, mirroring the −4.5% TPOT.
CI States
Latest PR Test (Base): ❌ Run #34775797271
Latest PR Test (Extra): ❌ Run #34775797239
Latest PR Test (AMD ROCm 10): ❌ Run #34775797382