Repository navigation
perf(amd): cap GLM-5.2 FP4 MI355X SGLang TP8 c4 admission at 2 / perf(amd): 将 GLM-5.2 FP4 MI355X SGLang TP8 c4 的准入上限降到 2 - #4630
EricKing626 wants to merge 2 commits into
Conversation
At TP8 concurrency 4 the slowest decodes are the ones that share a decode batch, so admission above the in-flight tail only widens the batch that tail runs in. Drop max-running-requests from 8 to 2 and keep the decode graph batch at 8; requests over the cap wait for a slot. Every other point keeps 2x CONC.
EricKing626
changed the title
|
Thanks for the contribution!
中文感谢你的贡献！
|
|
Claude finished @EricKing626's task in 20s —— View job Review
🔴 One blocking issue: the new The recipe change itself looks correct. Only |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - config-keys: | ||
| - glm5.2-fp4-mi355x-sglang-agentic-mtp | ||
| scenario-type: | ||
| - agentic-coding | ||
| description: | ||
| - "TP8 concurrency 4 drops max-running-requests from 8 to 2 and keeps cuda-graph-max-bs-decode at 8. No other point moves: TP8 concurrency 1 and 2 and every TP4 HiCache point keep admission at 2x CONC." | ||
| - "At this point the slowest decodes are the ones that share a decode batch, so admission above the in-flight tail only widens the batch that tail runs in. A max-running-requests sweep at TP8 concurrency 4 measured interactivity at p90 TPOT 222.3 to 255.0 output tokens/s/user and the slowest decile's mean concurrent decode partners 1.68 to 0.71, against TTFT p90 1.07 to 1.22 s and 3911 to 3754 tokens/s/GPU; requests over the cap wait for a slot." | ||
| - "The image, EAGLE shape (5 steps, 6 draft tokens), chunked-prefill-size, mem-fraction-static, HiCache tier and the DSA triton prefill and decode backends are unchanged." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/4630 |
There was a problem hiding this comment.
🔴 BLOCKING: New inferencex-e2e/perf-changelog.yaml entries must be appended to the END of the file. This one now sits between #4571 and the later entries (#4554, #4464, #4468, ...) that came in when main was merged into this branch.
Why it matters: The file is read in chronological order, oldest at the top and newest at the bottom. An entry in the middle breaks that order and can be missed when result collection reads the file.
Fix: Move this entry, unchanged, from here to after the last entry (#4468) at the bottom of the file.
| - config-keys: | ||
| - glm5.2-fp4-mi355x-sglang-agentic-mtp | ||
| scenario-type: | ||
| - agentic-coding | ||
| description: | ||
| - "TP8 concurrency 4 drops max-running-requests from 8 to 2 and keeps cuda-graph-max-bs-decode at 8. No other point moves: TP8 concurrency 1 and 2 and every TP4 HiCache point keep admission at 2x CONC." | ||
| - "At this point the slowest decodes are the ones that share a decode batch, so admission above the in-flight tail only widens the batch that tail runs in. A max-running-requests sweep at TP8 concurrency 4 measured interactivity at p90 TPOT 222.3 to 255.0 output tokens/s/user and the slowest decile's mean concurrent decode partners 1.68 to 0.71, against TTFT p90 1.07 to 1.22 s and 3911 to 3754 tokens/s/GPU; requests over the cap wait for a slot." | ||
| - "The image, EAGLE shape (5 steps, 6 draft tokens), chunked-prefill-size, mem-fraction-static, HiCache tier and the DSA triton prefill and decode backends are unchanged." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/4630 |
There was a problem hiding this comment.
🔴 The changelog validator and PR sweep will reject this PR, because the new block sits mid-file instead of at the tail. It was inserted before the existing minimaxm31flash entries (PRs #4464 and #4468), which are the last entries on the base. Fix: restore the base file byte-for-byte, then append this one block after the final #4468 entry. The changelog conflict helper (prepare_perf_changelog_merge resolve-conflict) does this, and it keeps every main entry in its original order.
Why this was flagged
On base 98f0d6e the file is 11558 lines and ends with the minimaxm31flash GB200 (#4464) and GB300 (#4468) entries. The diff adds the glm5.2-fp4-mi355x-sglang-agentic-mtp block at line 11502, ahead of them. validate_perf_changelog.compare_entries compares base and head entries by position (validate_perf_changelog.py:129-137), so the first displaced minimaxm31flash slot now holds the new entry and fails with "entry N changed; existing entries are immutable". validate_raw_change (validate_perf_changelog.py:172) also fails because head_raw no longer starts with base_raw. run-sweep.yml builds its matrix from the appended tail entries, so the sweep would be blocked or would pick up the wrong config keys. AGENTS.md:88 requires appending only at the tail.
Verification: Trigger: merging this PR through the mandated infx.workflows.merge_with_reuse path. The diff hunk @@ -11499,6 +11499,16 @@ puts the new block (#4630) before the minimaxm31flash-fp4-b200-sglang-agentic-dspark entry. It is not at the tail. merge_with_reuse.py:688 calls canonicalize_changelog(pr, repo). canonicalize_changelog does not catch the error, so the merge tool aborts.
| # up to 64. The TP4 HiCache tier is 180 GB per rank. TP8 c4 is the exception: | ||
| # its slowest decodes are the ones sharing a batch, so admission is capped | ||
| # below CONC to trade TTFT for p90 interactivity; the graph batch stays at | ||
| # 2x CONC. |
There was a problem hiding this comment.
🟡 nit (optional): The TP8 c4 rationale was added to the section header comment rather than next to the value it explains, about 140 lines away. AGENTS.md forbids section header comments and allows only short comments on a specific line. Fix: move a one-line why comment onto max-running-requests: 2 in override_tp8_c4 (around line 193), and leave the header comment as it was.
Why this was flagged
AGENTS.md:56 forbids header comments that describe a section or override, and AGENTS.md:58 limits comments to a short technical note on a specific line or block. The diff extends the header above override_tp4_c4_hicache (agentic.yaml:51-55) with four lines about override_tp8_c4, whose max-running-requests: 2 is at about line 193. A reader of that line sees no reason for the value. The header now also describes an exception for a block elsewhere in the file.
Verification: The diff adds four lines to the section header comment at agentic.yaml:51-55. The value it explains, max-running-requests: 2 in override_tp8_c4, is at line 193 with no comment of its own. AGENTS.md:56 says "No header comments describing a file, topology, config, recipe, override, or section." It is a rule violation, not a runtime problem.
Problem
Every point in
glm5.2/sglang/mi355x-fp4-mtp/agentic.yamladmits2x CONC. At TP8 concurrency 4 that cap never binds, and the requests that decode slowest are the ones that share a decode batch: the slowest decile runs with a mean of 1.68concurrent decode partners against 0.54 for the rest. Admission headroom above the in-flight tail only widens the batch that tail runs in.
Change
override_tp8_c4dropsmax-running-requestsfrom 8 to 2.cuda-graph-max-bs-decodestays at 8. Nothing else moves: TP8 concurrency 1 and 2 and every TP4 HiCache point keep2x CONC.The FP8 sibling on B300 (
glm5.2/sglang/b300-fp8-mtp/agentic.yaml) already caps the same way at concurrency 4, 12 and 16, and keeps2x CONCat the throughput end, so this follows the shape that recipe already establishes.Measured
A local max-running-requests sweep on MI355X at this point, same image and recipe, one server per point:
Risk
Intvty@P90 TPOTis1 / p90(ITL)and excludes TTFT, so part of this gain is time moved from decode into queueing rather than time saved. The AgentX North Star is E2E normalized interactivity, which includes TTFT (inferencex-e2e/docs/MODELS.md), and TTFT P90 regresses 14% here. The sweep run reports that metric; if it does not improve, this change should not land.Throughput per GPU is also expected to fall about 4% at this point, which is the intended move along the latency-throughput curve, not a regression to absorb elsewhere.
Validation
Pending: the sweep above is a local run, not an ingested e2e run. A green
e2e-tests.ymlrun for this branch and an accepted/use <run_id>are required before merge, and the E2E normalized interactivity column of that run is thegate described under Risk.