[AMD] Qwen3.5-FP8 MI355X SGLang disagg perf tuning: image bump to v0.5.16, TP4P+TP8D baseline, add MTP / Qwen3.5-FP8 MI355X SGLang disagg 性能调优：镜像升级至v0.5.16，TP4P+TP8D基线优化，新增MTP配置 - #2557
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
1 similar comment
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
…e, add MTP config - Bump qwen3.5-fp8-mi355x-sglang-disagg image from v0.5.14 to v0.5.16 (3-5% throughput improvement across all concurrency levels) - Switch baseline from TP8P+TP8D (16 GPU) to TP4P+TP8D (12 GPU): TP4 prefill delivers identical decode interactivity with 24-31% better throughput/GPU - Add qwen3.5-fp8-mi355x-sglang-disagg-mtp: EAGLE speculative decoding (MTP depth 3) on TP4P+TP8D, conc-list [8,16,32,64,128] - models.yaml: add mtp_flags for Qwen3.5-397B-A17B-FP8 (--speculative-algorithm EAGLE --speculative-eagle-topk 1) - Infra: add docker_sg.sh wrapper and update job.slurm DOCKER_CMD_DETECT for nodes where Slurm doesn't activate the docker supplementary group Co-authored-by: Cursor <cursoragent@cursor.com>
989c2ff to
d0fb017
Compare
ChangLiu0709
changed the title
At conc=128, TP8 decode bottlenecks MTP causing throughput collapse. Use TP4P+TP4D (8 GPU) for conc=128 while keeping TP4P+TP8D (12 GPU) for conc 8-64, matching the validated local benchmark results. Co-authored-by: Cursor <cursoragent@cursor.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - spec-decoding: "mtp" | ||
| conc-list: [ 8, 16, 32, 64, 128 ] | ||
| prefill: | ||
| num-worker: 1 | ||
| tp: 4 | ||
| ep: 1 | ||
| dp-attn: false | ||
| additional-settings: | ||
| - "PREFILL_NODES=1" | ||
| decode: | ||
| num-worker: 1 | ||
| tp: 8 | ||
| ep: 1 | ||
| dp-attn: false | ||
| additional-settings: | ||
| - "DECODE_NODES=1" | ||
| - "DECODE_MTP_SIZE=3" | ||
|
|
||
| qwen3.5-fp4-mi355x-sglang: | ||
| image: lmsysorg/sglang-rocm:v0.5.16-rocm720-mi35x-20260730 |
There was a problem hiding this comment.
🟡 The new qwen3.5-fp8-mi355x-sglang-disagg-mtp entry (configs/amd-master.yaml:332-351) runs decode tp: 8 for the entire conc-list [8,16,32,64,128], but the PR's own results table and perf-changelog.yaml state that at conc=128 TP8 decode causes 'MTP collapse' and that the documented numbers (interactivity 39.5, tput/GPU 5147) were actually measured on TP4P+TP4D. A sweep of this config will therefore reproduce the known-collapse topology at conc=128 instead of the validated one. Consider splitting conc=128 into its own arm with decode tp: 4, matching the pattern used elsewhere in this file (e.g. dsr1-fp8-mi355x-sglang-disagg-mtp).
Extended reasoning...
The committed qwen3.5-fp8-mi355x-sglang-disagg-mtp search-space arm (configs/amd-master.yaml:332-351) is a single block covering conc-list: [8, 16, 32, 64, 128] with decode: { tp: 8, ep: 1, ... } applied uniformly to every concurrency point. There is no second arm carving out conc=128 with a different decode topology.
However, the PR description's own MTP benchmark table daggers the conc=128 row explicitly: 'conc=128 MTP uses TP4P+TP4D (8 GPU) — TP8 decode at conc=128 causes MTP collapse; TP4D resolves it with +83% tput/GPU.' The accompanying perf-changelog.yaml addition echoes this independently: 'at conc=128 the decode TP8 bottleneck causes MTP collapse (-38% tput/GPU), which is resolved by switching to TP4D.' Both artifacts agree the author already discovered and worked around this collapse during local validation, but that workaround (switching decode to TP4 at conc=128) was never encoded back into the committed YAML.
The result is a config/documentation mismatch: the recipe as written will always exercise the TP8 decode topology at conc=128, which is the exact case the author's own text says degrades throughput by ~38%. A sweep run against this config cannot reproduce the row of numbers (interactivity 39.5, tput/GPU 5147) that the PR presents as the MTP conc=128 result, because those numbers came from a TP4D run this config doesn't perform. This isn't a case of stale prose describing correct code — the YAML and the changelog/description are making mutually exclusive factual claims about which topology conc=128 runs on.
This pattern (splitting a conc range into separate arms with different decode tp/DECODE_MTP_SIZE per topology) is already established elsewhere in this same file — see dsr1-fp8-mi355x-sglang-disagg-mtp and dsr1-fp4-mi355x-sglang-disagg-8k1k-mtp, both of which use multiple per-conc arms specifically to match validated topologies. That makes the fix straightforward and consistent with existing conventions: drop conc=128 from the current TP8D arm and add a second arm with conc-list: [128], decode: { tp: 4, ... }, mirroring the existing qwen3.5-fp8-mi355x-sglang-disagg baseline entry's TP4P+TP8D pattern but with decode tp=4.
Concrete walkthrough: (1) a maintainer runs the full sweep on qwen3.5-fp8-mi355x-sglang-disagg-mtp; (2) at conc=128 the recipe launches decode with tp=8 per the single committed arm; (3) per the PR's own analysis this triggers MTP collapse, so the sweep records materially worse throughput (~38% lower tput/GPU per the changelog) than the TP4D figures published in the PR description's table; (4) anyone comparing the recorded sweep data against the PR's stated 5147 tput/GPU for conc=128 will find it doesn't match, because the committed recipe never runs the TP4D topology that produced that number.
Impact is data-quality/reproducibility rather than a hard failure: the run will complete and produce a valid (if degraded) data point rather than crashing, since the sibling spec-none baseline arm also runs conc=128 on TP8D deliberately for apples-to-apples comparison. For that reason this is best treated as a fix-before-merge nit rather than a blocking correctness bug.
| - "Add qwen3.5-fp8-mi355x-sglang-disagg-mtp: EAGLE speculative decoding (MTP depth 3) on the TP4P+TP8D topology. MTP improves TPOT by 3-14% at conc≤64; at conc=128 the decode TP8 bottleneck causes MTP collapse (-38% tput/GPU), which is resolved by switching to TP4D (see Section 13 of the Qwen3.5 DI perf tuning analysis)." | ||
| - "models.yaml: add mtp_flags (--speculative-algorithm EAGLE --speculative-eagle-topk 1) to Qwen3.5-397B-A17B-FP8." | ||
| - "Infra: add docker_sg.sh wrapper and update job.slurm DOCKER_CMD_DETECT to use sg-docker fallback for nodes where Slurm doesn't activate the docker supplementary group." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX |
There was a problem hiding this comment.
🟡 The new perf-changelog.yaml entry (line 5782) ends with pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX — an unresolved placeholder rather than the real PR number. Per docs/configuration-procedures.md, the sanctioned pre-PR placeholder is pr-link: TBD, to be replaced with the real URL right after the PR is created; since this is PR #2557, it should read pull/2557.
Extended reasoning...
The newly-added perf-changelog.yaml entry for this PR (the qwen3.5-fp8-mi355x-sglang-disagg / -mtp config-keys block) ends with:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXXXXXX is a literal, unresolved template placeholder — it is not a real PR number, and it is also not the documented placeholder convention.
docs/configuration-procedures.md (around lines 251-254) specifies the changelog convention explicitly: the pr-link field should resolve to https://github.com/SemiAnalysisAI/InferenceX/pull/<number>, and before the PR exists, the sanctioned interim value is pr-link: TBD, which must be "replace[d] with the real URL immediately after creating the PR." The doc further calls out (around line 272) that a changelog still carrying an unresolved placeholder is a stop-condition / not-ready-for-sweep signal when the PR is otherwise ready to run — and this PR already carries the full-sweep-fail-fast label, i.e. it is being treated as ready.
Every other entry in perf-changelog.yaml follows the convention with a resolved numeric PR link (e.g. pull/2206, pull/2238, pull/2309 for the immediately preceding entries), confirming XXXX here is simply a forgotten fill-in rather than an intentional convention. Since this change is PR #2557 (per the PR metadata), the line should read:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2557Why this slips through: the documented automated check (line 272 of configuration-procedures.md) appears to look for the literal string TBD as the "not yet filled in" marker. Because the author used XXXX instead of TBD, it likely doesn't match that check's leftover-placeholder detection, making it more likely for this entry to merge unresolved than if the author had used the sanctioned TBD value.
Step-by-step proof:
- Open
perf-changelog.yamland note every other entry'spr-linkresolves to a real PR number (e.g..../pull/2309for the DSv4 AgentX entry immediately above this one). - Open
docs/configuration-procedures.mdand find the changelog template section — it documentspr-link: TBDas the only sanctioned pre-merge placeholder, to be swapped for the real URL right after PR creation. - Look at the new entry at the end of
perf-changelog.yaml(line 5782):pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXX. This is neither a real number norTBD. - Cross-reference the PR metadata: this change is submitted as PR [AMD] Qwen3.5-FP8 MI355X SGLang disagg perf tuning: image bump to v0.5.16, TP4P+TP8D baseline, add MTP / Qwen3.5-FP8 MI355X SGLang disagg 性能调优：镜像升级至v0.5.16，TP4P+TP8D基线优化，新增MTP配置 #2557.
- Conclusion: the line should read
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2557.
Impact is limited to changelog metadata/traceability — it does not affect the benchmark configs, scripts, or results in this PR — so this doesn't block correctness of the actual benchmark work, but it is a concrete, easily-fixed convention violation a maintainer would want corrected before merge.
When a GPU hang leaves a container in an unkillable state ("did not
receive an exit event"), docker rm -f is insufficient. Add a fallback
that retrieves the container's init PID and kills it directly so the
daemon can release GPU VRAM and the cgroup.
Applied to pre-run cleanup, trap handler, and post-run cleanup.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31502632978 |
The container creation barrier (300s timeout) races against Docker image downloads when one node has the image cached and another doesn't. A multi-GB SGLang image pull can easily exceed 300s, causing the barrier to timeout and the job to fail with "No logs directory found". Add a best-effort srun pre-pull step before the main benchmark srun so all nodes have the image cached before the barrier starts. Co-authored-by: Cursor <cursoragent@cursor.com>
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31502632978 |
1 participant
Changes
Baseline (
qwen3.5-fp8-mi355x-sglang-disagg)lmsysorg/sglang:v0.5.14-rocm720-mi35x→lmsysorg/sglang:v0.5.16-rocm720-mi35x(ROCm 7.2, SGLang v0.5.14 → v0.5.16, 3-5% throughput improvement across all concurrency levels).MTP (
qwen3.5-fp8-mi355x-sglang-disagg-mtp) — new configspec-decoding: mtp,DECODE_MTP_SIZE=3(EAGLE, num-steps 3, draft-tokens 4, topk 1).[8, 16, 32, 64, 128], ISL=8K / OSL=1K.mtp_flags: "--speculative-algorithm EAGLE --speculative-eagle-topk 1"toQwen3.5-397B-A17B-FP8.Infra
dockersupplementary group viasg docker -cfor Slurm job steps where group credentials are not inherited.DOCKER_CMD_DETECTto trysg dockerfallback beforesudo docker.Local benchmark results (v0.5.16, ISL=8K/OSL=1K)
Baseline (TP4P+TP8D, 12 GPU):
MTP (TP4P+TP8D, 12 GPU):
†conc=128 MTP uses TP4P+TP4D (8 GPU) — TP8 decode at conc=128 causes MTP collapse; TP4D resolves it with +83% tput/GPU.
Authors
@ChangLiu0709
@xiaohuguo2023
@chunfangamd
中文说明
Qwen3.5-FP8 MI355X SGLang 分离式推理优化：
Made with Cursor
Made with Cursor