[AMD][Qwen3.5] Use HiCache kernel IO and page_first on MI355X MXFP4 AgentX - #2972
[AMD][Qwen3.5] Use HiCache kernel IO and page_first on MI355X MXFP4 AgentX#2972yichiche wants to merge 5 commits into
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 关于重新运行失败任务的文档 |
aed1ac3 to
dc9a869
Compare
There was a problem hiding this comment.
Looks good — a mechanical config default swap that matches the existing pattern.
Reviewed the two changed defaults (HICACHE_IO_BACKEND, HICACHE_MEM_LAYOUT) in qwen3.5_fp4_mi355x_sglang_mtp.sh — env overrides are preserved via the ${VAR:-default} pattern and other hicache args (ratio, write policy) are untouched. Confirmed kernel/page_first matches the already-hardcoded values in the sibling qwen3.5_fp4_b200_sglang_mtp.sh / qwen3.5_fp4_b300_sglang_mtp.sh recipes the PR claims to align with. Checked the perf-changelog.yaml diff appends a new entry only at the tail with all prior bytes untouched, consistent with the append-only convention.
Extended reasoning...
Overview
The diff is a two-line default change in benchmarks/single_node/agentic/qwen3.5_fp4_mi355x_sglang_mtp.sh, swapping the HiCache CPU-tier defaults from --hicache-io-backend direct/--hicache-mem-layout page_first_direct to kernel/page_first, plus a matching append-only entry in perf-changelog.yaml. Ratio and write-policy defaults are untouched, and both changed values remain overridable via HICACHE_IO_BACKEND/HICACHE_MEM_LAYOUT env vars, using the existing ${VAR:-default} shell idiom already used for the sibling values on the same line.
Security risks
None. This only changes which SGLang server flag values are passed by default for the CPU KV-cache tier IO path and memory layout; no new code paths, no secrets, no auth/permission logic, no user input handling.
Level of scrutiny
Low. This is a narrow, single-value config tweak in a benchmark launcher script, not a change to shared library logic, scheduling, or benchmark invariants. I verified the specific cross-script consistency claim in the PR description by grepping other agentic MTP scripts: qwen3.5_fp4_b200_sglang_mtp.sh and qwen3.5_fp4_b300_sglang_mtp.sh do hardcode --hicache-io-backend kernel / --hicache-mem-layout page_first, so the stated rationale (aligning the ROCm recipe with the already-shipped B200/B300 recipes) checks out against the actual codebase rather than just the PR's own prose. The changelog append preserves the byte-sensitive, append-only convention (diff only adds lines after the prior last entry).
Other factors
The PR description candidly notes the local A/B benchmark used a non-CI-pinned ROCm image tag rather than the CI-pinned image, and the changelog pr-link is a placeholder (.../pull/0). These are process/documentation nits rather than functional bugs — the bug-hunting pass found no bugs, and the code change itself is simple, reversible via env override, and consistent with an established pattern elsewhere in the repo, so I don't think they warrant blocking approval or deferring to a human.
This review covers commit aed1ac3, which is no longer the latest commit on this pull request; later commits are not covered by it.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
d5fd0b9 to
5f697a8
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34610307600 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34615847734 |
ee8b4ca to
76b4956
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34615847734 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34678002084 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34678175056 |
Co-Authored-By: ChangLiu0709 <190883410+ChangLiu0709@users.noreply.github.com>
…x-bs-decode v0.5.19 splits --cuda-graph-max-bs into --cuda-graph-max-bs-decode and --cuda-graph-max-bs-prefill and removes the unsuffixed name, so argparse rejects it as ambiguous. Every agentic arm in run 34486742261 died with: sglang serve: error: ambiguous option: --cuda-graph-max-bs could match --cuda-graph-max-bs-decode, --cuda-graph-max-bs-prefill Process died before http://0.0.0.0:8894/health became ready. Use --cuda-graph-max-bs-decode, matching the Qwen3.5 RTX PRO 6000 recipe. The captured decode batch size is unchanged: min(2*CONC, 128).
Switch ROCM_QUICK_REDUCE_QUANTIZATION from INT8 to INT4 and raise the scheduler-recv-interval default from 30 to 60. Local TP2 AgentX vs the previous INT8/recv=30 defaults improved hicache conc28/32 output tok/s/gpu and cut P90 E2E/TPOT; KV-none throughput stayed flat.
0e9376b to
2fa9a88
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=34705759156 |
Summary
--hicache-io-backend direct/--hicache-mem-layout page_first_directtokernel/page_first.--hicache-ratio 1.5and--hicache-write-policy write_throughunchanged.HICACHE_IO_BACKENDandHICACHE_MEM_LAYOUTremain overridable.kernel/page_first.qwen3.5-fp4-mi355x-sglang-agentic-mtpfromlmsysorg/sglang-rocm:v0.5.18-rocm720-mi35x-20260829tolmsysorg/sglang-rocm:v0.5.19-rocm720-mi35x-20260910. Re-trigger the sweep once that tag is on Docker Hub.Details
benchmarks/single_node/agentic/qwen3.5_fp4_mi355x_sglang_mtp.shis the only Qwen3.5 MXFP4 AgentX recipe. On the dram/hicache arms it previously defaulted to SGLang's direct copy path andpage_first_directlayout, which stalls decode on host KV copies. Switching the defaults to the kernel IO backend andpage_firstlayout removes that copy bottleneck without changing HiCache sizing or write policy.Local AgentX measurement (TP2, conc=20, DURATION=1200,
qwen3.5-fp4-mi355x-sglang-agentic-mtp):direct/page_first_direct(previous default)kernel/page_first(this PR)Local software under test was
rocm/sgl-dev:v0.5.19-rocm720-mi35x-20260909. CI is pinned tolmsysorg/sglang-rocm:v0.5.19-rocm720-mi35x-20260910; that mi35x tag was not on Docker Hub at PR open (mi30x/mi45x 20260910 already were). Re-trigger after it lands. Cache hit stayed 1.0; dropped records were warmup only (40 per run). No-offload (kvnone) at the same conc is unchanged by the HiCache flag change.Accuracy
N/A — serving-path HiCache IO/layout only. No model weights, speculative-decoding, or eval-path change.
Note
Low Risk
Benchmark and serving-flag updates only; no auth, data, or model-weight changes, though results may shift versus prior sweeps.
Overview
Updates the Qwen3.5 MXFP4 MI355X AgentX MTP SGLang launch recipe and CI pin for SGLang 0.5.19 (
lmsysorg/sglang-rocm:…20260911).In
qwen3.5_fp4_mi355x_sglang_mtp.sh, the server CLI renames--cuda-graph-max-bsto--cuda-graph-max-bs-decode, raises the defaultSCHEDULER_RECV_INTERVALfrom 30 to 60, and switchesROCM_QUICK_REDUCE_QUANTIZATIONfrom INT8 to INT4 for prefill allreduce.perf-changelog.yamlrecords the same three serving tweaks plus the image bump forqwen3.5-fp4-mi355x-sglang-agentic-mtp.Reviewed by Cursor Bugbot for commit 2fa9a88. Bugbot is set up for automated code reviews on this repo. Configure here.