[AMD][MI35X] 0907 DSV4 sglang mtp agentic - #2885
Conversation
Moves dsv4-fp4-mi355x-sglang-agentic-mtp onto the stock upstream lmsysorg v0.5.19-rocm720-mi35x-20260907 image and applies the configuration a conc 64-256 sweep on MI355X converged on. Measured on that image, DP+MoE-TP: 171,732 / 246,399 / 351,252 tok/s at conc 64 / 128 / 256. v0.5.19 is what makes the image change possible: it is the first public lmsysorg tag that accepts --enable-deepseek-v4-fp4-indexer on ROCm. On v0.5.18 argument resolution raises unless is_sm100 or is_sm120, both False on a ROCm torch build, which is why this key previously needed a vendor branch image to carry the indexer. Re-running the identical configuration on a vendor image reproduces these numbers to within 2% at every concurrency, so nothing here depends on out-of-tree patches. Recipe changes, each swept with everything else held fixed: * chunked-prefill base 16384 -> 8192 at tp 8. Won conc 64 and 256 outright, lost conc 128 by 3.6%. It also avoids a cliff: 16384 with two-batch overlap at conc 256 spent 5h54m in warmup and then served 16,929 tok/s against 345,412, a 20x drop that neither knob reproduces on its own. * --enable-two-batch-overlap dropped on the DP path. Cost 15.3% throughput at conc 256 and won only at conc 128. * mem-fraction-static 0.92 on the DP path (pure TP keeps 0.86). The ladder at conc 256 measured 237,643 / 274,619 / 322,961 tok/s for 0.80 / 0.90 / 0.92 as prefix-cache chip hit went 75.75% -> 94.62%. Scoped to DP because tp 4 cannot start above ~0.89. * shared-experts fusion stays enabled under DP attention; the disable moves to the EP branch, where its +10.0% was actually measured and where it also drops the mori dispatch topk from 7 to 6. That mechanism does not exist without expert parallelism. * swa-full-tokens-ratio back to the global 0.10 on the DP path. * max-running-requests loses its 256 ceiling. It is a whole-engine budget that sglang already divides by the DP degree, so the cap silently gave conc 256 the same 32-requests-per-rank scheduler width as conc 128. The B200 and B300 siblings have no cap. * --prefill-delayer-token-usage-low-watermark 0.7 on the DP path. Upstream leaves this off entirely -- the option is Optional[float] with no fallback, so --enable-prefill-delayer alone disables the check. Worth -3.1% / +6.7% / +3.6% throughput and TTFT p50 -59% / -78% / -71% at conc 64 / 128 / 256, for a 6-18% TPOT cost. * MTP draft length by concurrency: 3 below conc 256, 1 at and above it. A depth 0/1/2/3 sweep found the optimum reverses between 128 and 256 (338,280 vs 287,516 tok/s at conc 256). num-draft-tokens is computed as num-steps + 1 rather than relying on sglang's silent correction. Search space trimmed to 7 points: tp4 conc 1/4, tp8 pure TP conc 16/32/48, tp8 DP+MoE-TP conc 128/256. Pure TP flattens out around conc 32-48 (163,203 tok/s at 48) and the DP arm carries the curve above it.
This comment was marked as duplicate.
This comment was marked as duplicate.
3 similar comments
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as spam.
This comment was marked as spam.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes performance-tuning behavior in a CODEOWNER-gated master config — DP-attention memory/scheduling knobs, shared-experts-fusion routing, and concurrency-conditional MTP draft length/golden acceptance length — a human familiar with this recipe would still be worth having take a look.
What was reviewed:
- Cross-checked the new conc-conditional golden AL values (num_steps=3 → AL 2.49, num_steps=1 → AL 1.79) against golden_al_distribution/dsv4_mtp.yaml — both match the committed curve exactly.
- Verified the perf-changelog.yaml prose ("keep swa-full-tokens-ratio at 0.10", "keep --enforce-shared-experts-fusion") against the script diff — both are accurate descriptions of the DP-branch overrides being removed, not discrepancies.
- Confirmed amd-master.yaml's new search-space rows (TP8 no-offload, TP8 hicache, TP8 DP-attention hicache) align with the script's tightened TP 4|8 guard and DP chunked-prefill formula.
- Confirmed the AGENTS.md
--use-chat-templateconvention applies torun_benchmark_serving(fixed_seq_len scripts), not this agentic script, so its absence here is not a violation.
Extended reasoning...
Overview
The PR tunes the AMD MI355X DeepSeek-V4-Pro SGLang agentic MTP recipe across three files: the benchmark script (chunked-prefill sizing formula, DP-attention branch flags, mem-fraction-static, removal of the 256-request cap, concurrency-conditional MTP draft depth/golden acceptance length), the master config's search-space rows, and a matching perf-changelog entry. No test suite covers runtime behavior of these shell scripts; correctness rests on the documented reasoning in comments/changelog and on the golden AL reference file, which I was able to independently verify.
Security risks
None. This is a benchmark configuration/shell-script change with no user input, network-facing surface, secrets, or auth/crypto paths involved.
Level of scrutiny
Medium-high. While the diff is a config/script tuning change (the kind that's often mechanical), it changes several interacting knobs at once (DP mem-fraction, shared-experts-fusion default, resource caps, spec-decoding depth selection) in a CODEOWNER-owned file (configs/amd-master.yaml, owned by a list that includes the PR author), and those knobs materially affect throughput/accuracy results that downstream consumers will treat as ground truth. That combination — cross-cutting tuning changes plus CODEOWNER-gated config plus no automated test coverage — argues for a human sanity-check even though I found nothing incorrect.
Other factors
The bug-hunting system reported no findings, and the four candidates it investigated (SWA-ratio default reverting to 0.10 under DP, shared-experts-fusion default flipping to "enforce" for DP-attention when EP_SIZE=1, the 256-request cap removal, and the flag's branch move) all turned out to be accurately described in the new perf-changelog entry rather than being undocumented side effects — i.e., they are intentional tuning decisions, not bugs. Given zero remaining findings but the config's importance and codeowner sensitivity, deferring rather than approving felt like the safer call.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34195172707 |
|
/reuse-sweep-run 34195172707 |
|
3 participants
cc @karverma-amd, @Duyi-Wang
Summary:
lmsysorg/sglang-rocm:v0.5.19-rocm720-mi35x-20260907.--enable-two-batch-overlap.mem-fraction-staticto 0.92.--disable-shared-experts-fusionto the EP branch only.--prefill-delayer-token-usage-low-watermark 0.7.speculative-num-steps 1(golden AL 1.79) for conc256.Note
Medium Risk
Benchmark and serving-flag changes affect GPU memory, scheduler limits, and speculative decoding at very high concurrency; risk is operational (OOM/instability) rather than application security.
Overview
Refreshes the MI355X DeepSeek-V4-Pro AgentX SGLang MTP recipe for the v0.5.19 ROCm image and retunes serving for higher concurrency (including conc 256).
Launch script (
dsv4_fp4_mi355x_sglang_mtp.sh): chunked prefill is 8192/16384 for TP4/TP8 without DP; with DP-attention, engine-wide size is 8192×TP so each rank keeps an 8192 token budget. DP-attention drops--enable-two-batch-overlap, raisesmem-fraction-staticto 0.92, keeps shared-experts fusion unless EP > 1, and adds--prefill-delayer-token-usage-low-watermark0.7.max-running-requestsno longer caps at 256 (still 2×CONC). MTP uses 3 steps / golden AL 2.49 below conc 256, and 1 step / 1.79 at 256+, with draft tokenssteps+1.amd-master.yaml: bumps the config image and narrows the sweep to TP8 no-offload [1, 4, 16], HiCache [32, 48], and DP-attention HiCache [128, 256] (drops TP4 and several intermediate conc arms).Reviewed by Cursor Bugbot for commit 61787fd. Bugbot is set up for automated code reviews on this repo. Configure here.