Repository navigation
[AMD] [AGENTX] Kimi-K3 Performance Tuning #2602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
f296829
d28c525
38faee4
c0119e6
ad8f584
eb81b48
ed6c730
b7d1f4f
8366815
9bc673b
c1215ff
f080865
4901088
84bc103
aab67c0
57fd591
081d1b8
8a52651
e985b3e
9a96547
9dacd22
d820ae1
25d1e54
bbe304c
8988497
7ef3ff8
1b1dd63
a72c8b9
127db36
cbfe9d7
99cc45e
4c50610
bf0bf1b
71313ac
e631afb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| #!/usr/bin/env bash | ||
| # Fix AITER JIT modules building against a different pybind11 than the prebuilt | ||
| # .so files shipped in the image. | ||
| # | ||
| # Symptom (kills the server during warmup, before it binds): | ||
| # TypeError: fmha_fwd_bf16_opus_fwd(): incompatible function arguments | ||
| # RuntimeError: Engine core initialization failed | ||
| # | ||
| # Cause: aiter/jit/utils/cpp_extension.py adds the standalone pybind11 include | ||
| # via -I, which outranks the -isystem path holding torch's bundled pybind11. | ||
| # The prebuilt modules (module_aiter_core.so) use torch's pybind11 | ||
| # (PYBIND11_INTERNALS_VERSION 11); the standalone package is version 12. | ||
| # pybind11 keeps a SEPARATE type registry per internals id, so the JIT module | ||
| # cannot see aiter_tensor_t registered by the core module. | ||
| # | ||
| # Effect: lets ROCM_AITER_FA be used for MLA prefill instead of falling back to | ||
| # FLASH_ATTN. Measured on 8x MI355X / Kimi-K3 MXFP4 TP8: | ||
| # ~24k ctx 12,953 -> 13,524 tok/s (+4.4%) | ||
| # ~93k ctx 11,174 -> 13,423 tok/s (+20.1%) | ||
| # | ||
| # Idempotent: safe to run repeatedly. No-op if already patched or not needed. | ||
| set -euo pipefail | ||
|
|
||
| PY=${PYTHON:-python3} | ||
|
|
||
| TARGET=$($PY - <<'EOF' | ||
| import os | ||
| try: | ||
| import aiter.jit.utils.cpp_extension as m | ||
| print(os.path.abspath(m.__file__)) | ||
| except Exception: | ||
| print("") | ||
| EOF | ||
| ) | ||
|
|
||
| if [ -z "$TARGET" ] || [ ! -f "$TARGET" ]; then | ||
| echo "[aiter-fix] aiter cpp_extension not found; nothing to do." | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Is there actually an internals-version mismatch to fix? | ||
| NEED=$($PY - <<'EOF' | ||
| import os, re | ||
| try: | ||
| import torch, pybind11 | ||
| except Exception: | ||
| print("no"); raise SystemExit | ||
| def ver(p): | ||
| f = os.path.join(p, "pybind11", "detail", "internals.h") | ||
| if not os.path.isfile(f): | ||
| return None | ||
| m = re.search(r"define\s+PYBIND11_INTERNALS_VERSION\s+(\d+)", open(f).read()) | ||
| return int(m.group(1)) if m else None | ||
| t = ver(os.path.join(os.path.dirname(torch.__file__), "include")) | ||
| s = ver(pybind11.get_include()) | ||
| print("yes" if (t is not None and s is not None and t != s) else "no") | ||
| EOF | ||
| ) | ||
|
|
||
| if [ "$NEED" != "yes" ]; then | ||
| echo "[aiter-fix] pybind11 internals versions already agree; no patch needed." | ||
| exit 0 | ||
| fi | ||
|
|
||
| if grep -q "_use_torch_pybind11" "$TARGET"; then | ||
| echo "[aiter-fix] already patched: $TARGET" | ||
| else | ||
| cp -n "$TARGET" "$TARGET.orig" || true | ||
| $PY - "$TARGET" <<'EOF' | ||
| import sys, io | ||
| path = sys.argv[1] | ||
| src = io.open(path, encoding="utf-8").read() | ||
| old = " extra_include_paths.append(pybind11.get_include())\n" | ||
| new = ( | ||
| " # PATCHED: prefer torch's bundled pybind11 so JIT modules land in the\n" | ||
| " # same pybind11 type registry as the prebuilt .so files. Mismatched\n" | ||
| " # PYBIND11_INTERNALS_VERSION otherwise yields:\n" | ||
| " # TypeError: ...(): incompatible function arguments\n" | ||
| " _use_torch_pybind11 = False\n" | ||
| " if not torch_exclude:\n" | ||
| " _use_torch_pybind11 = os.path.isdir(\n" | ||
| " os.path.join(TORCH_INCLUDE_ROOT, \"pybind11\")\n" | ||
| " )\n" | ||
| " if not _use_torch_pybind11:\n" | ||
| " extra_include_paths.append(pybind11.get_include())\n" | ||
| ) | ||
| if old not in src: | ||
| sys.stderr.write("[aiter-fix] ERROR: anchor line not found; aborting.\n") | ||
| sys.exit(1) | ||
| if src.count(old) != 1: | ||
| sys.stderr.write("[aiter-fix] ERROR: anchor line not unique; aborting.\n") | ||
| sys.exit(1) | ||
| io.open(path, "w", encoding="utf-8").write(src.replace(old, new)) | ||
| print("[aiter-fix] patched", path) | ||
| EOF | ||
| fi | ||
|
|
||
| # Drop JIT artifacts built against the wrong pybind11 so they rebuild. | ||
| # Ask aiter for its jit dir: it honours AITER_JIT_DIR and falls back to ~/.aiter | ||
| # when dist-packages is not writable, so deriving it from $TARGET is wrong. | ||
| JITDIR=$($PY -c 'from aiter.jit.core import get_user_jit_dir; print(get_user_jit_dir())' 2>/dev/null || true) | ||
| if [ -z "$JITDIR" ] || [ ! -d "$JITDIR" ]; then | ||
| JITDIR=$(dirname "$(dirname "$TARGET")") | ||
| fi | ||
| echo "[aiter-fix] jit dir: $JITDIR" | ||
| # Sweep every module, not just the one we happened to hit first: any module | ||
| # JIT-built before the patch carries the wrong internals id. | ||
| shopt -s nullglob | ||
| for so in "$JITDIR"/*.so; do | ||
| if grep -qa "__pybind11_internals_v12" "$so" 2>/dev/null; then | ||
| rm -f "$so" | ||
| rm -rf "$JITDIR/build/$(basename "${so%.so}")" | ||
| echo "[aiter-fix] removed stale v12 module: $(basename "$so")" | ||
| fi | ||
| done | ||
| shopt -u nullglob | ||
|
|
||
| echo "[aiter-fix] done." |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| #!/usr/bin/env bash | ||
| # Let DSpark spec-decode keep FULL cudagraphs on ROCm. | ||
| # | ||
| # vllm/v1/attention/backends/mla/triton_mla.py declares | ||
| # TritonMLAMetadataBuilder._cudagraph_support = UNIFORM_SINGLE_TOKEN_DECODE | ||
| # which caps min_cg_support below UNIFORM_BATCH, so config/compilation.py:1443 | ||
| # downgrades FULL_AND_PIECEWISE -> PIECEWISE whenever spec-decode is enabled: | ||
| # "CUDAGraphMode.FULL_AND_PIECEWISE is not supported with spec-decode for | ||
| # attention backend TritonMLABackend" | ||
| # v1/worker/gpu/spec_decode/dflash/speculator.py:110-127 then gives the DSpark | ||
| # drafter CUDAGraphMode.NONE -- fully eager, with NO warning logged. Every draft | ||
| # layer and the Markov head dispatch kernel-by-kernel from Python each step. | ||
| # | ||
| # TRITON_MLA cannot simply be swapped out: it is the only ROCm MLA backend with | ||
| # supports_non_causal_multi_token_decode = True, which the DSpark draft needs. | ||
| # ROCM_AITER_MLA fails with "non-causal attention not supported". | ||
| # | ||
| # The builder already sets supports_non_causal_multi_token_decode = True and | ||
| # calls _init_reorder_batch_threshold(1, supports_spec_as_decode=True) "so | ||
| # full-cudagraph capture admits it", so UNIFORM_BATCH is the consistent value. | ||
| # | ||
| # MEASURED on 8x MI355X, Kimi-K3 MXFP4 TP8, DSpark, single stream, 600-tok gens: | ||
| # before: 14.05 tok/s, ITL 71.16 ms (PIECEWISE, drafter eager) | ||
| # after : 77.65 tok/s, ITL 12.88 ms (FULL cudagraphs) = 5.52x | ||
| # output verified correct in both ("17*23" -> 391, finish_reason stop) | ||
| # | ||
| # Idempotent. No-op if already patched or if the anchor is absent. | ||
| set -euo pipefail | ||
| PY=${PYTHON:-python3} | ||
|
|
||
| TARGET=$($PY - <<'EOF' | ||
| import os | ||
| try: | ||
| import vllm.v1.attention.backends.mla.triton_mla as m | ||
| print(os.path.abspath(m.__file__)) | ||
| except Exception: | ||
| print("") | ||
| EOF | ||
| ) | ||
| if [ -z "$TARGET" ] || [ ! -f "$TARGET" ]; then | ||
| echo "[triton-mla-fix] triton_mla.py not found; nothing to do." | ||
| exit 0 | ||
| fi | ||
| if grep -q "AttentionCGSupport.UNIFORM_BATCH" "$TARGET"; then | ||
| echo "[triton-mla-fix] already patched: $TARGET" | ||
| exit 0 | ||
| fi | ||
| cp -n "$TARGET" "$TARGET.orig" || true | ||
| $PY - "$TARGET" <<'EOF' | ||
| import sys, io | ||
| path = sys.argv[1] | ||
| src = io.open(path, encoding="utf-8").read() | ||
| old = """ _cudagraph_support: ClassVar[AttentionCGSupport] = ( | ||
| AttentionCGSupport.UNIFORM_SINGLE_TOKEN_DECODE | ||
| )""" | ||
| new = """ # PATCHED: UNIFORM_SINGLE_TOKEN_DECODE forced a PIECEWISE downgrade under | ||
| # spec-decode, which silently made the DSpark drafter fully eager. | ||
| _cudagraph_support: ClassVar[AttentionCGSupport] = AttentionCGSupport.UNIFORM_BATCH""" | ||
| if src.count(old) != 1: | ||
| sys.stderr.write("[triton-mla-fix] anchor missing or not unique; aborting.\n") | ||
| sys.exit(1) | ||
| io.open(path, "w", encoding="utf-8").write(src.replace(old, new)) | ||
| print("[triton-mla-fix] patched", path) | ||
| EOF | ||
| echo "[triton-mla-fix] done." |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -637,7 +637,7 @@ dsr1-fp8-mi355x-sglang-disagg-mtp: | |
| - "DECODE_MTP_SIZE=2" | ||
|
|
||
| kimik3-fp4-mi355x-vllm-agentic-mtp: | ||
| image: vllm/vllm-openai-rocm:nightly-cb8104839c141609d99f1254459ef3a4f1bd4263 | ||
| image: vllm/vllm-openai-rocm:nightly-3ee2df30337a301164c46ae444b76ee67e71c106 | ||
| model: moonshotai/Kimi-K3 | ||
| model-prefix: kimik3 | ||
| runner: cluster:mi355x-amds | ||
|
|
@@ -648,7 +648,7 @@ kimik3-fp4-mi355x-vllm-agentic-mtp: | |
| agentic-coding: | ||
| - dram-utilization: 0.50 | ||
| search-space: | ||
| - { tp: 8, kv-offloading: none, conc-list: [1, 4, 8] , spec-decoding: mtp} | ||
| # - { 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 } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 AGENTS.md:21 requires a Extended reasoning...
This PR makes several performance-affecting modifications to the Despite this, Nothing in the existing code prevents this omission — there is no CI check enforced here that fails a PR for a missing changelog entry visible in this diff, so the requirement relies entirely on author/reviewer discipline per AGENTS.md. Concretely, a reviewer or downstream consumer relying on Step-by-step proof:
Fix: append a new entry at the tail of |
||
|
|
||
| dsr1-fp4-mi355x-sglang-disagg: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Minor dead-code nit: on line 214,
MLA_PREFILL_BACKEND="${MLA_PREFILL_BACKEND:-ROCM_AITER_FA}"already guarantees the variable is non-empty (bash's:-fires on both unset and empty), so theif [ -n "$MLA_PREFILL_BACKEND" ]on line 216 is always true and the implicit empty-MLA_PREFILL_ARGSelse-branch is unreachable. Consider dropping the guard and buildingMLA_PREFILL_ARGSunconditionally, matching howCOMPILATION_CONFIG_ARGS/SPEC_ARGSare built elsewhere in this file.Extended reasoning...
What the bug is: Line 214 sets
MLA_PREFILL_BACKEND="${MLA_PREFILL_BACKEND:-ROCM_AITER_FA}". Bash's${var:-default}form (with the colon) substitutesdefaultwhenevervaris either unset or set to the empty string — this is distinct from${var-default}(no colon), which only substitutes on unset. Because of the colon, after line 214 executes,$MLA_PREFILL_BACKENDis guaranteed to be non-empty: it is either the caller's non-empty override or the literalROCM_AITER_FAfallback.The code path: Line 216 then guards the array construction with
if [ -n "$MLA_PREFILL_BACKEND" ]; then MLA_PREFILL_ARGS=(--attention-config ...); fi. Since the variable can never be empty at this point, the condition is always true, and the implicitelse(which would leaveMLA_PREFILL_ARGS=()empty) is dead code that can never execute — not even viaMLA_PREFILL_BACKEND=""as an explicit env override, since that empty string is itself replaced by the default on line 214 before the check ever runs.Why nothing catches this today: There's no test or lint that exercises unreachable branches in this shell script, and the guard reads as plausible defensive code at a glance (mirroring the real, reachable guards used for
EP_ARGSat line ~199 andOFFLOAD_ARGS's case statement), so it's easy to miss that this particular guard's precondition was already foreclosed on the prior line.Impact: None functionally — behavior is identical whether or not the
ifis present, since the branch it guards against can never be taken. This is purely a readability/maintainability nit.Proof by walkthrough:
MLA_PREFILL_BACKENDunset → line 214::-fires →MLA_PREFILL_BACKEND=ROCM_AITER_FA.MLA_PREFILL_BACKEND=""→ line 214::-fires (colon form applies to empty too) →MLA_PREFILL_BACKEND=ROCM_AITER_FA.MLA_PREFILL_BACKEND=FLASH_ATTN→ line 214::-does not fire →MLA_PREFILL_BACKEND=FLASH_ATTN.[ -n "$MLA_PREFILL_BACKEND" ]evaluates to true, soMLA_PREFILL_ARGSis always built with the--attention-configflag — the else-path (empty array) is unreachable in every case.Suggested fix: Drop the
if/fiand buildMLA_PREFILL_ARGSunconditionally, e.g.:This matches the pattern already used for
COMPILATION_CONFIG_ARGSandSPEC_ARGSearlier in the same file, both of which are built unconditionally without a reachable empty-array path.