perf(agentx): add LMCache offload arm to Kimi-K3 B300 at conc 56/70 - #2802
perf(agentx): add LMCache offload arm to Kimi-K3 B300 at conc 56/70#2802sammshen wants to merge 1 commit into
Conversation
708c117 to
11d6805
Compare
|
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 关于重新运行失败任务的文档 |
1 similar comment
|
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 关于重新运行失败任务的文档 |
|
Claude finished @sammshen's task in 3m 22s —— View job Review of PR #2802
LGTM - no blocking issues found. Verified against the review checklist:
One caveat: claims that depend on LMCache 0.5.5rc2 internals (the |
11d6805 to
83657b8
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33575084654 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline hardcoded-port finding, I also checked the LMCacheMPConnector's kv_connector_extra_config (line 248) against the sibling Mooncake arm: it omits chunk-size (fine — that's a server-side-only --chunk-size flag, not duplicated in the connector config) and omits a kv_load_failure_policy key (an asymmetry with the Mooncake arm, but not something I can call a bug without knowing LMCacheMPConnector's own default).
Extended reasoning...
A CONFIRMED finding (hardcoded LMCACHE_PORT/LMCACHE_HTTP_PORT instead of PORT-derived offsets, verified in the diff at benchmarks/single_node/agentic/kimik3_fp4_b300_vllm_mtp.sh around the new lmcache) case branch) is already queued as an inline comment, which on its own signals a human should look at this PR before merge. My role here is limited to deciding whether the top-level review body should add anything beyond that inline comment. I re-read the diff myself and confirmed two candidate concerns that were investigated and ruled out: (1) LMCACHE_CHUNK_SIZE is computed once and passed only to the standalone lmcache server process via --chunk-size, not duplicated into the vLLM-side kv_connector_extra_config — this is expected since chunk-size is a server-side sizing parameter, not a connector wire-protocol parameter; (2) the new LMCacheMPConnector's kv_connector_extra_config (line 248) lacks a kv_load_failure_policy key that the sibling MooncakeStoreConnector arm sets, which is a real API-shape asymmetry worth noting but not verifiable as a bug without visibility into LMCacheMPConnector's own defaults. Per the narrow exception for findings-present runs, I'm recording this ruled-out context in a short defer note rather than staying fully silent, since it adds information beyond what the inline comment alone conveys, but I am not restating the inline finding itself. No new top-level bug worth raising was found beyond what's already queued.
| LMCACHE_PORT=6555 | ||
| LMCACHE_HTTP_PORT=8090 |
There was a problem hiding this comment.
🔴 LMCACHE_PORT and LMCACHE_HTTP_PORT are hardcoded (6555, 8090) instead of derived from
Extended reasoning...
benchmark_lib.sh:18-23 states launchers set PORT per-runner (e.g. from RUNNER_NAME) precisely so concurrent gh-runners on a shared host don't collide; every existing auxiliary port in this and sibling scripts (MOONCAKE_MASTER_PORT, SGLANG_BACKEND_PORT, VLLM_ROUTER_METRICS_PORT) is PORT+offset for this reason. The new lmcache branch instead hardcodes --port 6555 and --http-port 8090. If two sweep jobs (e.g. this new lmcache arm at conc 56 and another lmcache-using benchmark, or a retried run) land on the same shared host concurrently, the second lmcache server fails to bind and the job fails or silently attaches to the first job's server, corrupting results — a regression class not present in the mooncake arm on the same script.
Verification: nit. Factually correct: the new lmcache branch hardcodes ports (lines 214-215: LMCACHE_PORT=6555, LMCACHE_HTTP_PORT=8090) whereas the pre-existing mooncake branch in the SAME file derives its port from $PORT (line 131: MOONCAKE_MASTER_PORT=$((PORT + 12000))). benchmark_lib.sh:17-23 states PORT is set per-runner "to avoid collisions across concurrent gh-runners on a shared host," and the…
There was a problem hiding this comment.
Confirmed and fixed in 799bc77. Both ports were copied verbatim from the MI355X arm without adapting to this file's convention. Now derived from PORT alongside the existing MOONCAKE_MASTER_PORT=$((PORT + 12000)):
LMCACHE_PORT=$((PORT + 13000))
LMCACHE_HTTP_PORT=$((PORT + 14000))
+13000/+14000 do not collide with the offsets already in use across benchmarks/ (+1, +2000, +10000, +12000).
On the kv_load_failure_policy asymmetry: intentional. The Mooncake arm sets recompute explicitly because MooncakeStoreConnector would otherwise fail the request on a load miss. LMCacheMPConnector already falls back to recompute, so there is no equivalent key to set.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| - "Add an LMCache 0.5.5rc2 DRAM offload arm at concurrency 56 and 70, where the GPU prefix cache hit rate falls to 88.0% and 59.9%." | ||
| - "Size the LMCache chunk as block size x DCP size (1536 x 8 = 12288), required for chunking under decode context parallel." | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2802 | ||
| append-only: true |
There was a problem hiding this comment.
Append-only merges a new series
Medium Severity
The changelog marks this entry append-only, but the PR treats kvdram-lmcache as a new visual series. Append-only attaches the new points to the existing same-image DRAM curve instead of publishing a separate LMCache series.
Reviewed by Cursor Bugbot for commit 83657b8. Configure here.
There was a problem hiding this comment.
Good catch on the inconsistency — the PR body did say "kvdram-lmcache is a new visual series" in an earlier revision. That claim was wrong and has been removed; the body now states the opposite.
To be precise about the mechanism: append-only is not what merges the two arms. _matrix_visual_series_key (utils/process_changelog.py:183) keys on model, model-prefix, precision, framework, runner, disagg, scenario, and offload_mode — where offload_mode is only "on"/"off". The KV backend is not a series dimension at all, so kvdram-mooncake and kvdram-lmcache are the same visual series by construction, with or without the flag. CONTRIBUTING states this directly: the recipe fingerprint keeps two distinct recipes at the same concurrency as "distinct database points without splitting the visual curve."
So the alternative implied here — publishing a separate LMCache series — is not reachable from the changelog flag; it would require changing the series keying repo-wide.
What append-only actually controls is whether this run schedules the 2 added points and extends the existing curve, or re-runs all 13 and republishes it. Either way one curve. Verified by running the real validator against origin/main: with the flag, append_only_delta emits exactly conc56_kvdram-lmcache and conc70_kvdram-lmcache and passes the existing-curve check, because both arms carry the same non-null image.
The distinct exp-names are still load-bearing — they keep the two arms as separate database points and separate result artifacts, which is what the concurrency overlap is for. They were never claimed to produce separate legend entries.
83657b8 to
799bc77
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33575552537 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33664556424 |
799bc77 to
a2bb1e2
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a2bb1e2. Configure here.
| agentic_pip_install --quiet --no-cache-dir --no-deps \ | ||
| "mooncake-transfer-engine-cuda13==0.3.11.post1" | ||
| agentic_pip_install --quiet --no-cache-dir --no-deps \ | ||
| "lmcache==$LMCACHE_VERSION" |
There was a problem hiding this comment.
LMCache install skips runtime deps
Medium Severity
lmcache==0.5.5rc2 is installed with --no-deps, so declared runtime packages such as cupy-cuda13x, cuda-python, and sortedcontainers are not pulled in. Every other LMCache recipe in this repo that uses --no-deps installs those extras explicitly. The connector import check can pass while lmcache server or its GPU workers later fail on a missing module.
Reviewed by Cursor Bugbot for commit a2bb1e2. Configure here.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33678199170 |
a2bb1e2 to
dafd9a7
Compare
1a24bc9 to
88d0e6f
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33696259777 |
Adds an LMCache 0.5.5rc2 DRAM offload arm to kimik3-fp4-b300-vllm-agentic-dspark at concurrency 56 and 70. Those are the two points where the offload tier carries the run. In the concurrency sweep from run 33472235399, GPU prefix cache hit rate holds 90.4-96.3% from conc 1 through 48, so nearly all prefix reuse is served out of HBM and the external tier is barely exercised. At conc 56 the pool drops to 21.6M tokens under gpu-memory-utilization 0.90 and GPU hit falls to 88.0%; at conc 70 it falls to 59.9%. Both points are past the drafting cutoff, so they share a topology and memory budget. Under DCP the LMCache chunk must be a multiple of block size x DCP size; Kimi-K3's hybrid KDA/MLA groups resolve a local block of 1536, so DCP 8 gives 12288, matching the validated Kimi-K3 DCP8 e2e run 33463265881. cp_kv_cache_interleave_size stays at vLLM's default of 1: the pinned image defines the option but carries none of the auto-resolution that sets it to the local block size. The cumem allocator is disabled on this arm only. The MP connector shares KV tensors with its out-of-process server over CUDA IPC, and cuMem-allocated memory has no legacy IPC handle, so _share_cuda_() fails with cudaErrorInvalidValue during initialize_from_config. The MP server runs a single GPU transfer worker, matching the validated configuration.
88d0e6f to
ff72feb
Compare
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33698612745 |
1 participant
Description
Adds an LMCache 0.5.5rc2 DRAM offload arm to
kimik3-fp4-b300-vllm-agentic-dsparkat concurrency 56 and 70.Those are the two points where the offload tier carries the run. From run 33472235399, GPU prefix cache hit rate holds 90.4–96.3% from conc 1 through 48, so the external tier is barely exercised there. At conc 56 the KV pool drops to 21.6M tokens under
gpu-memory-utilization0.90 and GPU hit falls to 88.0%; at conc 70 it falls to 59.9%. Both are past the drafting cutoff, so they share topology, image and memory budget.Configuration
--chunk-size 12288= block size x DCP size (1536 x 8), required under decode context parallel. Matches the validated Kimi-K3 DCP8 e2e run 33463265881.5894fdfdefaultscp_kv_cache_interleave_sizeto 1 and has none of the auto-resolution that sets it to the local block size, so DCP runs on the flat layout the connector already supports. LMCache#4834 is a no-op at interleave 1 and its hybrid-group rule resolves the same 12288. An image that resolves the interleave to 1536 would need #4834 plus an explicit flag.--separate-object-groupsfor the KDA/MLA layout; one MP server per node at the fullTOTAL_CPU_DRAM_GB;--max-gpu-workers8, overridable viaLMCACHE_GPU_WORKERS.dram-utilizationunchanged at 0.75.PORT(+13000/+14000) alongside the existingMOONCAKE_MASTER_PORToffset, so concurrent runners on a shared host don't collide.mooncake-transfer-engine-cuda13is installed because LMCache's transfer-channel layer imports it at module load.Series behaviour and
append-onlyBoth arms land on one visual series, and this is by construction rather than a consequence of the flag.
_matrix_visual_series_key(utils/process_changelog.py:183) keys on model, precision, framework, runner, disagg, scenario andoffload_mode("on"/"off") — the KV backend is not a series dimension. Per CONTRIBUTING, the recipe fingerprint keeps two recipes at the same concurrency as "distinct database points without splitting the visual curve." Publishing LMCache as its own line is not reachable from this PR; it would require changing the series keying.append-only: truetherefore only controls scheduling: the sweep runs the 2 added points and extends the existing curve rather than re-running all 13. Verified againstorigin/main—append_only_deltaemits exactlyconc56_kvdram-lmcacheandconc70_kvdram-lmcache.The overlapping concurrencies still give distinct exp-names (
kvdram-lmcachevskvdram-mooncake) viaagentic_kv_offload_suffix, so the two arms remain separate database points and separate result artifacts at c56/c70.Reviewer note: the resulting published curve will contain DRAM-offload points from two different backends — the existing arm at conc 1–48 and LMCache at conc 56/70 — indistinguishable to a chart reader. Flagging for a call on whether that is acceptable for this recipe before these results are ingested.
Behavioural isolation for the complete-diff review: every script change is inside the
lmcache)case or gated onKV_OFFLOAD_BACKEND, the existing arm's entry is untouched, and its recipe fingerprint is unchanged at61ba8917bcdef3e3.Test plan
b300-nvchunk_size=12288inlmcache_server.log, no interleave rejection at connector startupNote
Medium Risk
Touches benchmark orchestration and KV offload integration (new backend, vLLM allocator flag, long-running sidecar); limited to two concurrency points but failures would skew perf curves.
Overview
Adds an LMCache 0.5.5rc2 external KV tier to the Kimi-K3 B300 vLLM agentic recipe, parallel to the existing Mooncake path.
When
KV_OFFLOAD_BACKEND=lmcache, the script installslmcacheandmooncake-transfer-engine-cuda13, starts an out-of-processlmcache server(DCP-sized chunk 12288, L1 sized fromTOTAL_CPU_DRAM_GB, 3600s read TTL, healthcheck wait), wires vLLM viaLMCacheMPConnector, and tears the server down on exit. vLLM drops--enable-cumem-allocatoron this arm so CUDA IPC to the MP server works.nvidia-master.yamladds a sweep arm forkimik3-fp4-b300-vllm-agentic-dsparkat concurrency 56 and 70 (where GPU prefix-cache hit rate drops and DRAM offload matters), overlapping Mooncake concurrencies but distinct exp-names via backend suffix.perf-changelog.yamlrecords the change withappend-only: trueso only those two points are scheduled.Reviewed by Cursor Bugbot for commit ff72feb. Bugbot is set up for automated code reviews on this repo. Configure here.