Skip to content

[AMD] [AGENTX] Kimi-K3 Perf Tuning 2.0 - #2674

Open
ajith-sirra-amd wants to merge 2 commits into
mainfrom
amd/kimi-k3-agentic-perf-tuning-2.0
Open

[AMD] [AGENTX] Kimi-K3 Perf Tuning 2.0#2674
ajith-sirra-amd wants to merge 2 commits into
mainfrom
amd/kimi-k3-agentic-perf-tuning-2.0

Conversation

@ajith-sirra-amd

Copy link
Copy Markdown
Collaborator

Summary: [AMD] [AGENTX] KIMI-K3 Perf Tuning

Net effect

Both fixes together unlock AITER's faster MLA prefill path, DCP and restore full CUDA-graph capture for spec-decode, compounding into a substantial throughput improvement for Kimi-K3 MXFP4 on MI355X.

Signed-off-by: Sirra <asirra@amd.com>
@ajith-sirra-amd
ajith-sirra-amd requested a review from a team August 19, 2026 06:57
@ajith-sirra-amd ajith-sirra-amd added AMD agentx AgentX benchmarks, recipes, and infrastructure labels Aug 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

4 similar comments
@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@github-actions

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

Signed-off-by: Sirra <asirra@amd.com>
Comment thread configs/amd-master.yaml
Comment on lines 634 to 640
agentic-coding:
- dram-utilization: 0.50
search-space:
- { tp: 8, kv-offloading: none, conc-list: [1, 4, 8] , spec-decoding: mtp}
- { tp: 8, ep: 1, kv-offloading: dram, kv-offload-backend: { name: vllm-simple }, conc-list: [10], spec-decoding: mtp }
- { tp: 8, ep: 1, kv-offloading: dram, kv-offload-backend: { name: vllm-simple }, conc-list: [1, 4, 8, 10, 12, 16, 20, 24, 28, 32], spec-decoding: mtp }

dsr1-fp4-mi355x-sglang-disagg:
image: lmsysorg/sglang-rocm:v0.5.12-rocm720-mi35x-20260519

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 AGENTS.md's non-negotiable invariants require a new perf-changelog.yaml entry (appended at the tail) for every recipe addition or modification that can affect benchmark performance. This PR rewrites the kimik3_fp4_mi355x_mtp.sh launch script (new DCP/MLA-prefill-backend flags, new MAX_NUM_SEQS/cudagraph formula, image bump to floating :nightly) and expands the conc-list in configs/amd-master.yaml, but perf-changelog.yaml is untouched. Please append an entry documenting these changes before merging.

Extended reasoning...

AGENTS.md:21 states as a non-negotiable benchmark invariant: "Every change that can affect benchmark performance and every recipe addition or modification requires a new perf-changelog.yaml entry. The file is append-only and byte-sensitive... append only at the tail." This is explicitly called out as non-negotiable, not a style suggestion, and CLAUDE.md points to AGENTS.md as the canonical source of project instructions.

The PR's diff touches exactly two files — benchmarks/single_node/agentic/kimik3_fp4_mi355x_mtp.sh and configs/amd-master.yaml — and does not include perf-changelog.yaml. Yet the change is squarely inside the invariant's scope: it rewrites the vLLM server launch flags for the kimik3-fp4-mi355x-vllm-agentic-mtp recipe (adds --decode-context-parallel-size 8, --dcp-comm-backend a2a, --attention-backend ROCM_AITER_MLA, --cp-kv-cache-interleave-size 1, a new MLA prefill-backend attention-config, and async-scheduling args), replaces the fixed MAX_NUM_SEQS=20 / MAX_CUDAGRAPH_CAPTURE_SIZE=60 constants with a formula derived from CONC, bumps the served image from a pinned digest (nightly-cb8104839c141609d99f1254459ef3a4f1bd4263) to the floating :nightly tag, and collapses/expands the configs/amd-master.yaml search-space from two arms (conc-list: [1,4,8] and conc-list: [10]) into one arm with a ten-point conc-list: [1,4,8,10,12,16,20,24,28,32]. The PR title itself is "Kimi-K3 Perf Tuning 2.0" and its description says the changes "compound into a substantial throughput improvement" — this is definitionally a perf-affecting recipe modification, the exact category the invariant targets.

Nothing in the current tooling would silently let this slip through if the changelog gate runs: the repo carries utils/validate_perf_changelog.py plus changelog gate tests specifically to enforce append-only, byte-sensitive changelog entries per recipe change. A missing tail entry for this PR (there is no entry keyed to PR #2674 or kimik3-fp4-mi355x-vllm-agentic-mtp describing these new flags) would fail that gate, not just look incomplete to a human reviewer.

To fix: append a new entry at the tail of perf-changelog.yaml describing the kimik3-fp4-mi355x-vllm-agentic-mtp config-key change — new DCP/MLA-prefill-backend/async-scheduling flags, the new MAX_NUM_SEQS/MAX_CUDAGRAPH_CAPTURE_SIZE formula, the image bump, and the expanded conc-list — along with a link to this PR, following the same style as the neighboring entries for similar recipe rewrites (e.g. the #2576 GLM-5.2 entry, which documents comparable knob changes).

Step-by-step proof: (1) AGENTS.md:21 requires a changelog entry for "every recipe addition or modification" affecting perf. (2) git diff/PR diff shows only kimik3_fp4_mi355x_mtp.sh and configs/amd-master.yaml changed — no perf-changelog.yaml hunk exists in the diff. (3) The two changed files unambiguously constitute a recipe modification: new server flags, new sizing formula, new image tag, new search-space. (4) Searching perf-changelog.yaml's existing content for kimik3-fp4-mi355x-vllm-agentic-mtp or PR #2674 finds no matching entry — the file's tail predates this PR. (5) Therefore the invariant is violated as written, independent of any subjective judgment call.


wait_for_amd_gpu_clean

du -h /dev/shm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Line 9 adds a bare du -h /dev/shm with no redirection, comment, or || true guard, unlike this script's other diagnostics (rocm-smi || true / amd-smi || true). It looks like a leftover debug probe from bring-up (checking /dev/shm capacity for the DRAM KV-offload path) and just adds noise to the benchmark log; under set -euo pipefail it could also abort the run if du ever returns non-zero.

Extended reasoning...

This PR replaces the script's old documentation header with a bare du -h /dev/shm call at line 9, right after wait_for_amd_gpu_clean. The command's output is never captured, tested, or referenced anywhere downstream — it exists purely as a side effect on stdout.

The rest of the script is careful about diagnostics that aren't load-bearing: rocm-smi || true and amd-smi || true (further down in the same file) are both explicitly guarded with || true so a non-zero exit doesn't tank the run, and their purpose (dumping GPU state to the log for debugging) is self-evident from context. du -h /dev/shm has neither the guard nor any comment explaining why it's there. That asymmetry, combined with it being the only such command in the entire benchmarks/ tree (per grep), strongly suggests it's a leftover from local bring-up/debugging of the DRAM KV-offload arm rather than an intentional, permanent diagnostic.

Concretely, the code path is: script starts with set -euo pipefail, sources benchmark_lib.sh, calls wait_for_amd_gpu_clean, then immediately runs du -h /dev/shm unconditionally before check_env_vars even runs. Every invocation of this benchmark — across all 10 concurrency points in the new conc-list — will emit this du output into the log, adding noise with no functional purpose.

There's also a latent robustness gap: because the line has no || true, if du ever exits non-zero (e.g., a transient permission issue or an unreadable entry under /dev/shm), set -e will abort the whole benchmark run before the server is even started. In practice these runs execute as root in the enroot container, so du should always succeed against its own tmpfs — meaning this is a latent risk rather than an observed failure — but it stands out precisely because the pattern used everywhere else in the file (|| true) exists to prevent exactly this class of failure.

To fix: either delete the line entirely (simplest, since nothing consumes its output), or if the intent is to keep an /dev/shm capacity check for debugging DRAM KV-offload sizing, add a comment explaining why and guard it the same way as the other diagnostics, e.g. du -sh /dev/shm || true # sanity-check host shm usage for KV offload.

Step-by-step proof: (1) checkout the script, (2) run it (or just bash -x it up to line 9) with any valid env vars set, (3) observe du -h /dev/shm executes and prints usage for every subdirectory under /dev/shm to stdout/log before check_env_vars is even reached — confirming it fires unconditionally and its output goes nowhere but the log. (4) To see the abort risk, chmod 000 a subdirectory under /dev/shm as a non-root user and rerun — du returns non-zero, and since there's no || true, set -e immediately kills the script, whereas the guarded rocm-smi/amd-smi calls later in the same file would survive an equivalent failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agentx AgentX benchmarks, recipes, and infrastructure AMD

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants