perf(agentx): restore LMCache retrieves on Kimi-K3 MI355X DCP8 - #2831
perf(agentx): restore LMCache retrieves on Kimi-K3 MI355X DCP8#2831sammshen wants to merge 2 commits into
Conversation
Pin lmcache 0.5.5rc5+rocm7.2 from the versioned v0.5.5rc5-rocm release. rc5 is the merge commit of LMCache #4936, which fixes the DCP cache identity: the scheduler and the workers derived different model names, so every lookup missed and the DRAM tier was write-only at concurrency 44-52. The previous pin also no longer resolves. nightly-rocm keeps only its newest wheel, so 0.5.5.dev89+rocm7.2 disappeared when that tag rebuilt. Also derive the MP server ports from $PORT rather than the fixed 6555/8090, and raise the L1 read-lock TTL to 3600s.
|
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 关于重新运行失败任务的文档 |
2 similar comments
|
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 关于重新运行失败任务的文档 |
|
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 关于重新运行失败任务的文档 |
sammshen
added
AMD
agentx
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good — straightforward dependency re-pin plus port/TTL fixes for the Kimi-K3 MI355X LMCache benchmark. Reviewed: version-string parity between the script and configs/amd-master.yaml, the $PORT-derived LMCache port arithmetic against how $PORT is already made unique per concurrent runner cell (runners/launch_mi355x-amds.sh), and the perf-changelog.yaml entry — it's a pure tail append using the exact pull/XXX placeholder convention documented in CONTRIBUTING.md/utils/validate_perf_changelog.py.
Extended reasoning...
Overview
The diff touches three files for a single logical change: re-pinning the LMCache KV-offload dependency used by the Kimi-K3 FP4 MI355X agentic-MTP benchmark from a nightly dev build to an immutable versioned pre-release (0.5.5rc5+rocm7.2), moving the LMCache MP-server ports to be derived from $PORT, raising --l1-read-ttl-seconds to 3600, and appending a matching perf-changelog.yaml entry. No source/runtime logic outside this one benchmark path is touched.
Security risks
None identified. This is a version pin and shell arithmetic change in a benchmark script plus a YAML config/changelog update — no auth, crypto, or permission-sensitive code paths are involved. The LMCache index URL is switched from a nightly to a versioned immutable release tag, which if anything reduces supply-chain risk (immutable artifact vs. a rolling "latest wheel" tag).
Level of scrutiny
Low-to-medium is appropriate: config/version-pin changes with matching recipe updates are the well-established, mechanical pattern this repo uses per AGENTS.md/CONTRIBUTING.md, and the change is self-contained to one benchmark cell. I did verify the two substantive risk points myself: (1) the version string is identical across the benchmark script and configs/amd-master.yaml (both updated together as required), and (2) the $PORT-derived port arithmetic (PORT+13000, PORT+14000) is safe because $PORT is already guaranteed unique per concurrently-running cell on a shared runner (runners/launch_mi355x-amds.sh derives it from RUNNER_NAME), which is the same invariant the main vLLM server's --port "$PORT" already relies on — so the new LMCache ports inherit that uniqueness without introducing a new collision class.
Other factors
The perf-changelog.yaml diff is a clean tail-append (git diff shows only additions after the prior last entry, no altered bytes), and the pull/XXX placeholder exactly matches PR_LINK_PLACEHOLDERS in utils/validate_perf_changelog.py and the example in CONTRIBUTING.md, so it is not something requiring hand-editing. I also confirmed the --use-chat-template convention mentioned in the task's "conventions noted" does not apply to this file — this script calls run_eval, not run_benchmark_serving (the same pattern as every other *_mtp.sh file under benchmarks/single_node/agentic/), so that note was a false lead for this particular script. Given the mechanical nature of the change, consistency with existing patterns, and no unresolved objections in the PR timeline, this is a reasonable approve candidate; I'm choosing not to add a "what was reviewed" style defer/approve verdict beyond a light one since nothing here appears to need deeper human scrutiny.
This review covers commit 565db2a, which is no longer the latest commit on this pull request; later commits are not covered by it.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33931292450 |
1 participant
The LMCache DRAM tier on the DCP8 arm is currently write-only. Under DCP the connector's scheduler and worker roles derived different LMCache model names, so every lookup missed:
Measured on run 33773561410, per GPU:
So the c44/48/52 points on
mainmeasure DCP and concurrency, not the offload tier, and they pay the store bandwidth without ever reading it back.Changes
lmcache 0.5.5rc5+rocm7.2from the versionedv0.5.5rc5-rocmrelease. rc5 isstatus=identicalto the merge commit of LMCache#4936, which fixes the above.nightly-rocm. That tag keeps only its newest wheel, so the previous0.5.5.dev89+rocm7.2pin no longer resolves at all —--find-linksnow offers onlydev94. Versioned-rocmreleases are immutable (v0.5.5rc4-rocmstill carries its wheel), so an exact pin keeps working.$PORTinstead of the fixed6555/8090, which two cells on one runner would contend for.--l1-read-ttl-secondsto 3600. The L1 read lock is held from prefetch through retrieve; at these concurrencies a queued request exceeds the 300s default and the retrieve fails. Latent until now only because no retrieve ever ran.Sweep scope
Not
append-only— the recipe fingerprint changes, so all 9 points of the config key re-run. That is intended: c44/48/52 need re-measuring with the tier actually serving, and the dcp1 points share the recipe.Verified: matrix generates the expected 9 points on rc5,
perf-changelog.yamladds no deletions against main,pytest utils/matrix_logic utils/test_process_changelog.py280 passed.Note
Medium Risk
Touches agentic benchmark dependency pins and LMCache server wiring; incorrect versions or ports would skew or break MI355X Kimi-K3 offload measurements, but scope is limited to that perf recipe.
Overview
Fixes the Kimi-K3 MI355X agentic LMCache path so DRAM offload can read KV again under DCP8, and keeps installs reproducible on ROCm.
LMCache pin: Moves from
0.5.5.dev89+rocm7.2on the rollingnightly-rocmindex to0.5.5rc5+rocm7.2from the immutablev0.5.5rc5-rocmrelease (rc5 includes the DCP cache-identity fix so scheduler and workers agree on model names). The matrix inconfigs/amd-master.yamlis updated to match.Server recipe (
kimik3_fp4_mi355x_mtp.sh): LMCache MP ports are derived from$PORT(+13000/+14000) instead of fixed6555/8090to avoid collisions when two cells share a host.--l1-read-ttl-secondsis raised to 3600 so queued retrieves at these concurrencies do not fail on the 300s default lock TTL.Changelog: Documents the pin, DCP retrieve fix, port derivation, and TTL change for
kimik3-fp4-mi355x-vllm-agentic-mtp(full 9-point recipe re-run expected).Reviewed by Cursor Bugbot for commit 6d4b648. Bugbot is set up for automated code reviews on this repo. Configure here.