Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
206 changes: 206 additions & 0 deletions benchmarks/single_node/agentic/minimaxm3_fp4_mi355x.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
#!/usr/bin/env bash
set -euo pipefail
set -x
# Agentic trace replay benchmark for Minimax-M3 FP4 on MI355X using vLLM.
#
# Required env vars:
# MODEL, TP, CONC, OFFLOADING, TOTAL_CPU_DRAM_GB, RESULT_DIR
#
# OFFLOADING values:
# none - vLLM GPU KV only.
# cpu - vLLM native CPU offload.
# lmcache - LMCache MP server + vLLM LMCacheMPConnector.
source "$(dirname "$0")/../../benchmark_lib.sh"
check_env_vars MODEL TP CONC KV_OFFLOADING TOTAL_CPU_DRAM_GB RESULT_DIR DURATION EP_SIZE DP_ATTENTION
echo "MODEL=$MODEL TP=$TP CONC=$CONC KV_OFFLOADING=$KV_OFFLOADING TOTAL_CPU_DRAM_GB=$TOTAL_CPU_DRAM_GB RESULT_DIR=$RESULT_DIR DURATION=$DURATION EP_SIZE=$EP_SIZE DP_ATTENTION=$DP_ATTENTION"
PORT=${PORT:-8888}
DURATION=${DURATION:-1800}
EP_SIZE=${EP_SIZE:-1}
if [[ -n "${SLURM_JOB_ID:-}" ]]; then
echo "JOB $SLURM_JOB_ID running on ${SLURMD_NODENAME:-unknown}"
Comment on lines +5 to +20

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.

🟡 The docstring, check_env_vars call, and variable defaults in the new script are internally inconsistent: (1) the header lists OFFLOADING but check_env_vars actually requires KV_OFFLOADING; (2) the header omits DURATION, EP_SIZE, DP_ATTENTION, and KV_OFFLOAD_BACKEND, all of which check_env_vars enforces; (3) the DURATION=${DURATION:-1800} and EP_SIZE=${EP_SIZE:-1} defaults at lines 19-20 are dead code (check_env_vars has already exited if either was empty); and (4) the header enumerates none / cpu / lmcache as OFFLOADING values but the case statement switches on $KV_OFFLOAD_BACKEND and only implements the lmcache arm — cpu and none silently fall through to --no-enable-prefix-caching. The launcher passes the correct vars so CI is unaffected, but the header will actively mislead anyone reading or running this stand-alone. Fix by syncing the docstring with the actual required vars and either dropping the unreachable defaults or removing DURATION/EP_SIZE from check_env_vars.

Extended reasoning...

The script header at lines 5-20 sets up a runtime interface that contradicts itself on four related points. This is doc/dead-code inconsistency in a brand-new file, so nothing breaks in the CI-driven flow — but this recipe is very likely to be copy-pasted (it is the first vLLM+LMCache MI355X agentic entry), and future readers will be misled.

(1) Wrong variable name in the required-vars list. Line 7 documents OFFLOADING, but line 14 calls check_env_vars MODEL TP CONC KV_OFFLOADING TOTAL_CPU_DRAM_GB RESULT_DIR DURATION EP_SIZE DP_ATTENTION. Sibling recipes (dsv4_fp4_b300_sglang.sh:7, kimik2.5_fp4_b300.sh:10, qwen3.5_fp8_h100.sh:15, minimaxm3_fp8_mi325x.sh) all consistently use the KV_OFFLOADING name — the header here is out of sync with the codebase convention.

(2) Missing required vars. The header lists 6 vars; check_env_vars enforces 9. DURATION, EP_SIZE, and DP_ATTENTION are all required at line 14 but never mentioned in the header. A user reading only the header, exporting the 6 documented vars, and running the script will hit Error: The following required environment variables are not set: DURATION EP_SIZE DP_ATTENTION.

(3) Dead-code defaults. Lines 19-20 set DURATION=${DURATION:-1800} and EP_SIZE=${EP_SIZE:-1}. But check_env_vars (benchmarks/benchmark_lib.sh:234) uses [[ -z "${!var_name:-}" ]] to detect unset OR empty and exits 1 if any listed var is missing. So by the time execution reaches line 19, DURATION and EP_SIZE are guaranteed non-empty — the :- fallbacks are unreachable. Compare minimaxm3_fp8_mi325x.sh:7, which requires the same vars in check_env_vars and correctly does NOT repeat the defaults.

(4) Values list documents a third variable name. Lines 9-12 enumerate none / cpu / lmcache as OFFLOADING values. The case statement at line 90 switches on $KV_OFFLOAD_BACKEND — a third, undocumented name — and only implements the lmcache arm. Setting KV_OFFLOAD_BACKEND=cpu (or =none) falls through to the initial OFFLOAD_ARGS=(--no-enable-prefix-caching) from line 47, which is neither vLLM native CPU offload nor GPU-KV-only in any meaningful sense.

Step-by-step proof. Take the header at face value and run: MODEL=amd/MiniMax-M3-MXFP4 TP=4 CONC=8 OFFLOADING=lmcache TOTAL_CPU_DRAM_GB=3000 RESULT_DIR=/tmp/x ./minimaxm3_fp4_mi355x.sh. set -u is on, so line 14 checks KV_OFFLOADING (which the user did not set because the header said OFFLOADING); check_env_vars reports 4 missing vars: KV_OFFLOADING, DURATION, EP_SIZE, DP_ATTENTION. Even if the user then also exports KV_OFFLOADING=lmcache DURATION=1800 EP_SIZE=1 DP_ATTENTION=false, the case at line 90 needs KV_OFFLOAD_BACKEND=lmcache — still-undocumented — otherwise LMCache never starts and vLLM runs with --no-enable-prefix-caching only.

Fix. Two clean options: (a) sync the header — replace OFFLOADING with KV_OFFLOADING, add KV_OFFLOAD_BACKEND, DURATION, EP_SIZE, DP_ATTENTION to the required list, drop the invented cpu/none values (or implement them), and delete lines 19-20; or (b) treat DURATION/EP_SIZE as optional-with-defaults — remove them from check_env_vars, keep the defaults, and add them to the header as optional. The sibling minimaxm3_fp8_mi325x.sh follows option (a); either is fine so long as the docstring and code match. Non-blocking (nit) — CI passes because configs/amd-master.yaml supplies the right variables via the launcher, so this only bites stand-alone users and future readers.

fi
# ROCR/HIP visibility for vLLM 0.14+
if [ -n "${ROCR_VISIBLE_DEVICES:-}" ]; then
export HIP_VISIBLE_DEVICES="$ROCR_VISIBLE_DEVICES"
fi
if [[ "$MODEL" != /* ]]; then hf download "$MODEL"; fi
rocm-smi || true
amd-smi || true
# `hf download` creates the target dir if missing and is itself idempotent.
# When MODEL_PATH is unset (stand-alone runs), fall back to the HF_HUB_CACHE
# Either way, MODEL_PATH is what the server is launched with.
if [[ -n "${MODEL_PATH:-}" ]]; then
if [[ ! -d "$MODEL_PATH" || -z "$(ls -A "$MODEL_PATH" 2>/dev/null)" ]]; then
hf download "$MODEL" --local-dir "$MODEL_PATH"
fi
else
hf download "$MODEL"
export MODEL_PATH="$MODEL"
fi

Check warning on line 39 in benchmarks/single_node/agentic/minimaxm3_fp4_mi355x.sh

View check run for this annotation

Claude / Claude Code Review

Redundant hf download doubles model download time/disk

Line 26 unconditionally runs `hf download "$MODEL"` into the default HF cache, and then the MODEL_PATH block at lines 32-39 does a second, independent `hf download` (either via `--local-dir "$MODEL_PATH"` or the same bare download again) — no sibling agentic recipe has this extra pre-download step. For amd/MiniMax-M3-MXFP4 this means the full checkpoint is downloaded twice on a fresh workspace, wasting bandwidth and disk, and the server is launched with $MODEL_PATH so the first download is never
Comment on lines +26 to +39

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 26 unconditionally runs hf download "$MODEL" into the default HF cache, and then the MODEL_PATH block at lines 32-39 does a second, independent hf download (either via --local-dir "$MODEL_PATH" or the same bare download again) — no sibling agentic recipe has this extra pre-download step. For amd/MiniMax-M3-MXFP4 this means the full checkpoint is downloaded twice on a fresh workspace, wasting bandwidth and disk, and the server is launched with $MODEL_PATH so the first download is never actually used.

Extended reasoning...

At benchmarks/single_node/agentic/minimaxm3_fp4_mi355x.sh:26, the script does:

if [[ "$MODEL" != /* ]]; then hf download "$MODEL"; fi

This downloads the model into the default HF_HUB_CACHE whenever MODEL is a hub id rather than a local path. Immediately after (lines 32-39), the script runs the MODEL_PATH block:

if [[ -n "${MODEL_PATH:-}" ]]; then
    if [[ ! -d "$MODEL_PATH" || -z "$(ls -A "$MODEL_PATH" 2>/dev/null)" ]]; then
        hf download "$MODEL" --local-dir "$MODEL_PATH"
    fi
else
    hf download "$MODEL"
    export MODEL_PATH="$MODEL"
fi

This is a second, independent download call. None of the sibling agentic recipes in this directory (dsv4_fp4_mi355x_vllm.sh, kimik2.5_fp4_b200.sh, minimaxm3_fp8_mi325x.sh, minimaxm3_fp8_mi300x.sh) have the line-26 pre-download — they only contain the MODEL_PATH if/else block. The comment directly above the block ("hf download creates the target dir if missing and is itself idempotent... Either way, MODEL_PATH is what the server is launched with") describes only that block, confirming line 26 was not meant to be part of the intended design.

Code path that triggers it: This recipe runs on runner: cluster:mi355x-amds (per configs/amd-master.yaml), which is the cluster launcher path where MODEL_PATH is set by the launcher before invoking the script. So every sweep config actually taken by this recipe hits: line 26 downloads amd/MiniMax-M3-MXFP4 (a large FP4 checkpoint) into HF_HUB_CACHE, then line 34 downloads it again via hf download "$MODEL" --local-dir "$MODEL_PATH". Modern huggingface_hub writes --local-dir downloads directly rather than hard-linking from the shared cache, so this materializes two full on-disk copies and (absent a warm cache) two full network transfers. The server is then launched with vllm serve "$MODEL_PATH", so the line-26 cache copy is never used at all — it's pure waste.

Why nothing else prevents it: hf download is idempotent/resumable, so it doesn't error out, it just silently does the work twice. There's no cache-sharing between the default hub cache and an explicit --local-dir target that would let the second call skip the transfer.

Impact: Roughly double the network transfer time and disk usage for this step, and — on constrained runners — the risk of two full copies of a large FP4 checkpoint existing simultaneously, which could threaten disk exhaustion. In the stand-alone (MODEL_PATH-unset) branch, line 26 and line 37 both hit the same bare hf download "$MODEL" call, so the second one is a cheap cache-hit rather than a real duplicate transfer — the double-download problem specifically affects the cluster/sweep path where MODEL_PATH is set.

Step-by-step proof (cluster path, fresh workspace, no warm cache):

  1. Launcher sets MODEL=amd/MiniMax-M3-MXFP4, MODEL_PATH=/data/models/minimax-m3-fp4 and invokes the script.
  2. Line 26: $MODEL doesn't start with /, so hf download amd/MiniMax-M3-MXFP4 runs, pulling the full checkpoint into $HF_HUB_CACHE (e.g. ~/.cache/huggingface/hub).
  3. Line 32: MODEL_PATH is set and (on a fresh node) empty/missing, so the inner condition is true.
  4. Line 34: hf download amd/MiniMax-M3-MXFP4 --local-dir "$MODEL_PATH" runs, pulling the same full checkpoint a second time into $MODEL_PATH.
  5. vllm serve "$MODEL_PATH" starts the server from the line-34 copy; the line-26 copy in the hub cache is dead weight.

Fix: Remove line 26 entirely — it duplicates work already handled correctly by the MODEL_PATH if/else block, matching the pattern used by every sibling recipe.

Severity is a nit, not a blocking issue: the benchmark still produces a correct measurement on the green CI path (the unofficial sweep runs completed successfully per the PR timeline), and this only wastes download time/disk rather than causing a failure or incorrect result.

resolve_trace_source
install_agentic_deps
# ---- Server config ----------------------------------------------------------
SERVER_LOG="$RESULT_DIR/server.log"
LMCACHE_LOG="$RESULT_DIR/lmcache_server.log"
mkdir -p "$RESULT_DIR"
OFFLOAD_ARGS=(--no-enable-prefix-caching)
# ---- Lmcache config ----------------------------------------------------------
LMCACHE_PID=""
cleanup_lmcache_server() {
if [[ -n "$LMCACHE_PID" ]] && kill -0 "$LMCACHE_PID" 2>/dev/null; then
kill "$LMCACHE_PID" 2>/dev/null || true
wait "$LMCACHE_PID" 2>/dev/null || true
fi
}
trap cleanup_lmcache_server EXIT
wait_for_lmcache_ready() {
{ set +x; } 2>/dev/null
local attempts="${LMCACHE_READY_ATTEMPTS:-120}"
local tail_pid=""
while [ ! -f "$LMCACHE_LOG" ]; do
if [[ -n "$LMCACHE_PID" ]] && ! kill -0 "$LMCACHE_PID" 2>/dev/null; then
echo "LMCache server died before creating log file. Exiting." >&2
exit 1
fi
sleep 1
done
tail -f -n +1 "$LMCACHE_LOG" &
tail_pid=$!
for ((i = 1; i <= attempts; i++)); do
if curl --output /dev/null --silent --fail "http://127.0.0.1:${LMCACHE_HTTP_PORT}/healthcheck"; then
kill "$tail_pid" 2>/dev/null || true
wait "$tail_pid" 2>/dev/null || true
return 0
fi
if [[ -n "$LMCACHE_PID" ]] && ! kill -0 "$LMCACHE_PID" 2>/dev/null; then
echo "LMCache server died before becoming healthy. Log follows:" >&2
kill "$tail_pid" 2>/dev/null || true
wait "$tail_pid" 2>/dev/null || true
cat "$LMCACHE_LOG" >&2 || true
exit 1
fi
sleep 1
done
echo "Timed out waiting for LMCache server healthcheck. Log follows:" >&2
kill "$tail_pid" 2>/dev/null || true
wait "$tail_pid" 2>/dev/null || true
cat "$LMCACHE_LOG" >&2 || true
exit 1
}
case "$KV_OFFLOAD_BACKEND" in
lmcache)
unset VLLM_USE_SIMPLE_KV_OFFLOAD
git clone https://github.com/LMCache/LMCache.git
cd LMCache
Comment on lines +93 to +94

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.

🟡 WARNING: The master config now declares kv-offload-backend: { name: lmcache, version: "0.5.1" }, but this clone still installs whatever LMCache default-branch HEAD is at run time — nothing checks out 0.5.1.

Why it matters: process_agentic_result.py embeds KV_OFFLOAD_BACKEND_METADATA into every ingested agg_*.json, so all results from this recipe will be recorded as running LMCache 0.5.1 while actually running an arbitrary, drifting HEAD commit. That silently breaks cross-run comparability and makes the recorded provenance wrong. The sibling recipes both pin: dsv4_fp4_mi355x_vllm.sh:280 does git checkout 9229067c... (matching the SHA in its config's version field), and kimik2.5_fp4_b200.sh:99 installs lmcache==0.5.1.

Fix: check out the version the config declares (or, if you need an unreleased commit, pin a SHA and put that SHA in the config's version field like the dsv4 recipe does):

Suggested change
git clone https://github.com/LMCache/LMCache.git
cd LMCache
git clone https://github.com/LMCache/LMCache.git
cd LMCache
git checkout v0.5.1

(verify the exact tag name on the LMCache repo — releases are tagged v<version>; if you actually validated against a different commit, pin that SHA here and update the config metadata to match)

pip install -r requirements/build.txt
CXX=hipcc BUILD_WITH_HIP=1 pip install -e . --no-build-isolation
cd ..
Comment on lines +91 to +97

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.

🟡 The LMCache install at minimaxm3_fp4_mi355x.sh:93 clones https://github.com/LMCache/LMCache.git with no pinned tag/SHA (--branch/--depth), so each benchmark run installs whatever HEAD of the LMCache default branch happens to be at that moment — silently invalidating cross-run perf comparability, which is the whole point of this recipe. The clone is also non-idempotent under set -euo pipefail: a retried run in the same workspace dies at git clone with 'destination path already exists'. Prefer a versioned pip install (as in kimik2.5_fp4_b200.sh:99: agentic_pip_install --quiet --no-cache-dir lmcache), or pin the clone (git clone --depth 1 --branch <tag> ...) with an rm -rf LMCache guard first (as in minimaxm3_fp8_mi325x.sh:87 for mooncake).

Extended reasoning...

What the bug is

At benchmarks/single_node/agentic/minimaxm3_fp4_mi355x.sh:91-97, the lmcache branch of the offload-backend case builds LMCache from source like this:

lmcache)
    unset VLLM_USE_SIMPLE_KV_OFFLOAD
    git clone https://github.com/LMCache/LMCache.git
    cd LMCache
    pip install -r requirements/build.txt
    CXX=hipcc BUILD_WITH_HIP=1 pip install -e .   --no-build-isolation
    cd ..

There is no --branch <tag>, no --depth 1, no post-clone git checkout <sha> — the clone resolves to whatever HEAD of the LMCache default branch (main) is at run time.

Impact — reproducibility

The entire purpose of a recipe in this repo is to produce numerically comparable perf points that get ingested into perf-changelog.yaml and rolled up across time. The single search-space row in configs/amd-master.yaml for this recipe is:

- { tp: 4, kv-offloading: dram, kv-offload-backend: lmcache, conc-list: [1, 4, 8, 16, 32] }

i.e. every configuration this recipe measures goes through the lmcache branch and therefore through the LMCache library. LMCache is an active project — LMCache/main moves. Two runs of this recipe on different days will install different LMCache revisions and can produce different tokens/s, TTFT and cache-hit numbers without any change to this repository. That is the exact anti-pattern the codebase avoids everywhere else: configs/amd-master.yaml pins every vLLM/SGLang image to a digest-suffixed nightly tag (see the dsv4-fp4-mi355x-vllm block comment that explicitly warns "pin to a digest-suffixed nightly tag rather than the floating :nightly"), and the sibling Mooncake source builds in minimaxm3_fp8_mi325x.sh:87 and minimaxm3_fp8_mi300x.sh:87 use git clone --depth 1 --branch "$mooncake_tag" ... with mooncake_tag='v0.3.11.post1'.

Impact — idempotency

The script begins with set -euo pipefail at line 2. git clone <url> into an existing non-empty directory fails with fatal: destination path 'LMCache' already exists and is not an empty directory, exit code 128. There is no rm -rf LMCache guard or [[ -d LMCache ]] || ... check. If a prior run in the same workspace didn't clean up (retried run, workspace reuse across configs in the sweep, or actions/checkout not pruning untracked directories), the second attempt aborts before the LMCache server ever starts.

Regression from the pre-existing pattern

The sibling NVIDIA recipe benchmarks/single_node/agentic/kimik2.5_fp4_b200.sh:99 uses a versioned PyPI install through the isolated agentic venv:

agentic_pip_install --quiet --no-cache-dir lmcache

This was the established pattern for installing LMCache in an agentic recipe. Switching to an unpinned HEAD clone is a strict regression. (The MI355X build does need a HIP source build — no ROCm wheel on PyPI — so falling back to a git clone is fine, but it must be pinned.)

Step-by-step proof of the non-idempotency failure

  1. Sweep dispatches the config { tp: 4, kv-offloading: dram, kv-offload-backend: lmcache, conc: 1 }; the script cds into the workspace, runs git clone https://github.com/LMCache/LMCache.git, builds, benchmarks, exits 0. LMCache/ remains in the workspace.
  2. Sweep dispatches the next config { tp: 4, ..., conc: 4 }; the script cds into the same workspace again, hits git clone at line 93.
  3. Git prints fatal: destination path 'LMCache' already exists and is not an empty directory and returns exit 128.
  4. set -e immediately aborts the script with exit 128 — wait_for_lmcache_ready, vllm serve, and run_agentic_replay_and_write_outputs never run. The config is recorded as failed.

(Whether the workspace persists across configs depends on the launcher; the current launch_mi355x-amds.sh mounts $GITHUB_WORKSPACE at /workspace, so in practice fresh CI containers usually paper over this — but any local re-run or workspace-reuse path trips the failure.)

Fix

A one-line change, either

agentic_pip_install --quiet --no-cache-dir lmcache==<version>

or, keeping the HIP source build:

rm -rf LMCache
git clone --depth 1 --branch <tag-or-sha> https://github.com/LMCache/LMCache.git

Severity

Nit — a clean container run in CI passes, the numerical result is some valid measurement, and there is no user-visible crash on the green path. But this is directly identified as an anti-pattern relative to the sibling recipes in the same directory and it silently erodes benchmark comparability over time, so worth calling out before merge.

python3 -c "import lmcache.integration.vllm.lmcache_mp_connector" >/dev/null
# Let the external MP server own the full CPU KV pool so vLLM does not
# split --kv-offloading-size across TP ranks through the integrated
# LMCache backend.
TOTAL_CPU_DRAM_GB="${TOTAL_CPU_DRAM_GB:-3000}"
TOTAL_CPU_DRAM_PARTITION_GB="${TOTAL_CPU_DRAM_PARTITION_GB:-${TOTAL_CPU_DRAM_GB}}"
LMCACHE_HOST="${LMCACHE_HOST:-127.0.0.1}"
LMCACHE_PORT="${LMCACHE_PORT:-5555}"
LMCACHE_HTTP_PORT="${LMCACHE_HTTP_PORT:-8080}"
# LMCacheMPConnector concatenates lmcache.mp.host and port into the
# ZMQ endpoint. Bind the server to a raw host, but pass the connector a
# ZMQ-style host string.
LMCACHE_CONNECT_HOST="${LMCACHE_CONNECT_HOST:-tcp://$LMCACHE_HOST}"
LMCACHE_L1_SIZE_GB="${LMCACHE_L1_SIZE_GB:-$((TOTAL_CPU_DRAM_PARTITION_GB))}"
LMCACHE_L1_INIT_SIZE_GB="${LMCACHE_L1_INIT_SIZE_GB:-20}"
# LMCache read locks are leases on chunks that lookup has promised
# vLLM can retrieve. The default 300s TTL is too short for this
# long-context agentic queue: TP8/conc32 can spend >300s between
# lookup and retrieve while GPU KV is saturated, which leaves the
# object present in L1 but no longer readable. Keep the 2.5 TB pool
# size unchanged and only extend the lookup-to-retrieve lease.
LMCACHE_L1_READ_TTL_SECONDS="${LMCACHE_L1_READ_TTL_SECONDS:-7200}"
# (srok) check 256 vs 32
#LMCACHE_CHUNK_SIZE="${LMCACHE_CHUNK_SIZE:-32}"
LMCACHE_CHUNK_SIZE="${LMCACHE_CHUNK_SIZE:-256}"
LMCACHE_MAX_WORKERS="${LMCACHE_MAX_WORKERS:-$((TP * 2))}"
export PYTHONHASHSEED="${PYTHONHASHSEED:-0}"
export LMCACHE_BLOCKING_TIMEOUT_SECS=60

Check warning on line 125 in benchmarks/single_node/agentic/minimaxm3_fp4_mi355x.sh

View check run for this annotation

Claude / Claude Code Review

LMCACHE_BLOCKING_TIMEOUT_SECS=60 contradicts the extended L1 read TTL rationale

The script copies the LMCache read-lease comment and `LMCACHE_L1_READ_TTL_SECONDS=7200` verbatim from the sibling `dsv4_fp4_mi355x_vllm.sh` recipe, but pairs it with `LMCACHE_BLOCKING_TIMEOUT_SECS=60` instead of the sibling's `1200` — a 20x divergence for an identical rationale comment. This looks like the TTL block was copied without its paired timeout, and is worth reconciling with the sibling before merge, though the exact runtime impact of the two knobs interacting is not fully established i
Comment on lines +113 to +125

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.

🟡 The script copies the LMCache read-lease comment and LMCACHE_L1_READ_TTL_SECONDS=7200 verbatim from the sibling dsv4_fp4_mi355x_vllm.sh recipe, but pairs it with LMCACHE_BLOCKING_TIMEOUT_SECS=60 instead of the sibling's 1200 — a 20x divergence for an identical rationale comment. This looks like the TTL block was copied without its paired timeout, and is worth reconciling with the sibling before merge, though the exact runtime impact of the two knobs interacting is not fully established in-repo.

Extended reasoning...

minimaxm3_fp4_mi355x.sh:113-125 is a near-verbatim copy of the LMCache block in dsv4_fp4_mi355x_vllm.sh:306-316 — same read-lease comment ("LMCache read locks are leases on chunks that lookup has promised vLLM can retrieve... TP8/conc32 can spend >300s between lookup and retrieve while GPU KV is saturated"), same LMCACHE_L1_READ_TTL_SECONDS=7200, same CHUNK_SIZE=256, same PYTHONHASHSEED default. The sibling pairs that 7200s TTL with LMCACHE_BLOCKING_TIMEOUT_SECS=1200; this recipe instead hardcodes LMCACHE_BLOCKING_TIMEOUT_SECS=60 (line 125) — both are explicit exports, not inherited defaults, so 60 is a deliberate typed value rather than an oversight in the traditional sense, but a 20x-smaller value than the co-designed sibling using the identical comment is a strong signal that the paired timeout was not updated when the TTL block was copied over.

The comment's own stated rationale is that under GPU-KV saturation, the gap between an LMCache lookup and the subsequent retrieve can exceed 300s — which is exactly why the read lease was extended from LMCache's 300s default to 7200s. A 60s blocking timeout is 5x shorter than that very 300s window the comment calls out as the problem case, so on its face the two settings pull in opposite directions.

I want to be upfront about the strongest counterargument raised during review, which is well-founded: LMCACHE_L1_READ_TTL_SECONDS and LMCACHE_BLOCKING_TIMEOUT_SECS are not proven in this repo to govern the same operation. The TTL is a server-side read lease covering the lookup-to-retrieve queue gap, while the blocking timeout may instead bound a different code path (e.g. an allocator/store-side wait for CPU-pool space) inside LMCache's own internals, which aren't visible from this codebase. If that's the case, a 60s blocking-timeout wouldn't necessarily fire during the >300s lookup-to-retrieve gap the TTL is protecting against, and the 'defeats the extended TTL' framing would be an overstatement of the mechanism. It's also true that this recipe's search space (configs/amd-master.yaml: { tp: 4, kv-offload-backend: lmcache, conc-list: [1,4,8,16,32] }) only ever runs TP4, whereas the comment's example scenario is TP8/conc32 and the sibling recipe that uses 1200s runs TP8 with additional connector settings (LMCACHE_TX_MODE=lmcache_driven) that this recipe does not set — so the two scripts aren't apples-to-apples, and the worst case the comment describes isn't guaranteed to occur at TP4.

None of that changes the fact that this is a self-inconsistent piece of copied code: the same comment, in the same position, justifying the same TTL value, is paired with two different blocking-timeout values 20x apart in the two scripts that share it, and the sibling script's value was clearly chosen deliberately to match the TTL rationale (its comment and its 1200s value were introduced together). At TP4 with only 4 GPUs' worth of aggregate KV/HBM (rather than 8), saturation-driven queuing delays could plausibly appear at lower concurrency than TP8, not higher, so 'this recipe never runs TP8' doesn't fully rule out the scenario either.

Concretely, to reproduce the concern: run this recipe at conc=32 (the top of its sweep) with kv-offload-backend=lmcache. Under load, some fraction of vLLM's blocking LMCache retrieve calls will be issued for chunks that lookup found present in L1 but that are queued behind other in-flight transfers on the LMCACHE_MAX_WORKERS=TP*2=8-worker MP server. If any such retrieve call takes longer than 60s to complete — which is exactly the scenario the 7200s TTL was raised to tolerate — that call errors/times out well before the chunk's still-valid 7200s lease would matter, and the request falls back to recompute rather than a cache hit. Whether the specific knob LMCACHE_BLOCKING_TIMEOUT_SECS is the one that fires in that path (as opposed to a different mechanism) is the part that can't be statically confirmed from this repository alone.

Fix: align LMCACHE_BLOCKING_TIMEOUT_SECS with the sibling's 1200 (or otherwise document why 60 is intentional here), so the two co-designed values don't visibly contradict each other and the high-concurrency (conc 16/32) measurements this recipe exists to produce aren't put at risk by a value that looks like a copy-paste gap.

echo "Starting LMCache MP server..."
LMCACHE_CMD=(
lmcache server
--host "$LMCACHE_HOST"
--port "$LMCACHE_PORT"
--http-host "$LMCACHE_HOST"
--http-port "$LMCACHE_HTTP_PORT"
--l1-size-gb "$LMCACHE_L1_SIZE_GB"
--l1-init-size-gb "$LMCACHE_L1_INIT_SIZE_GB"
--l1-read-ttl-seconds "$LMCACHE_L1_READ_TTL_SECONDS"
--chunk-size "$LMCACHE_CHUNK_SIZE"
--max-workers "$LMCACHE_MAX_WORKERS"
--eviction-policy LRU
)
printf '%q ' "${LMCACHE_CMD[@]}" > "$RESULT_DIR/lmcache_command.txt"
printf '\n' >> "$RESULT_DIR/lmcache_command.txt"
"${LMCACHE_CMD[@]}" > "$LMCACHE_LOG" 2>&1 &
LMCACHE_PID=$!
echo "LMCache server PID: $LMCACHE_PID"
wait_for_lmcache_ready
# Remove --disable-hybrid-kv-cache-manager and enable hybrid kv cache manager (default)
# This gives extra cache hit than disabling hybrid kv cache manager
OFFLOAD_ARGS=(
--kv-transfer-config
"{\"kv_connector\":\"LMCacheMPConnector\",\"kv_connector_module_path\":\"lmcache.integration.vllm.lmcache_mp_connector\",\"kv_role\":\"kv_both\",\"kv_connector_extra_config\":{\"lmcache.mp.host\":\"$LMCACHE_CONNECT_HOST\",\"lmcache.mp.port\":$LMCACHE_PORT}}"
)
;;
esac
# ---- LLM server config ----------------------------------------------------------
PARALLEL_ARGS=(--tensor-parallel-size "$TP")
if [ "${DP_ATTENTION}" = "true" ]; then
PARALLEL_ARGS=(
--tensor-parallel-size 1
--data-parallel-size "$TP"
--enable-expert-parallel
)
elif [ "$EP_SIZE" -gt 1 ]; then
PARALLEL_ARGS+=(--enable-expert-parallel)
fi
echo "Starting vllm server..."
export PYTHONNOUSERSITE=1
export VLLM_ENGINE_READY_TIMEOUT_S=3600
export VLLM_USE_BREAKABLE_CUDAGRAPH=0
export VLLM_ROCM_USE_AITER=1
export VLLM_ROCM_USE_AITER_MOE=1
export VLLM_ROCM_USE_AITER_FUSION_SHARED_EXPERTS=1
# INT4 quantized all-reduce for the (~1.5 MB) decode all-reduces, which are the
# single biggest decode kernel at high concurrency. The MIN_SIZE_KB override is
# required: vLLM's default INT4 quick-reduce size gate for (bf16, TP4) is 16 MB,
# so it never fires for decode-sized tensors without it.
export VLLM_ROCM_QUICK_REDUCE_QUANTIZATION=INT4
export VLLM_ROCM_QUICK_REDUCE_CAST_BF16_TO_FP16=0
export VLLM_ROCM_QUICK_REDUCE_QUANTIZATION_MIN_SIZE_KB=256
VLLM_CMD=(
vllm serve "$MODEL_PATH"
--served-model-name "$MODEL"
--host 0.0.0.0
--port "$PORT"
"${PARALLEL_ARGS[@]}"
--trust-remote-code
--block-size 128
--gpu-memory-utilization 0.85
--language-model-only
--attention-backend TRITON_ATTN
--moe-backend aiter
--kv-cache-dtype fp8
--tool-call-parser minimax_m3
--enable-auto-tool-choice
--reasoning-parser minimax_m3
--max-num-seqs "$CONC"
"${OFFLOAD_ARGS[@]}"
)
printf '%q ' "${VLLM_CMD[@]}" | tee "$RESULT_DIR/vllm_command.txt"
printf '\n' | tee -a "$RESULT_DIR/vllm_command.txt"
"${VLLM_CMD[@]}" > "$SERVER_LOG" 2>&1 &
SERVER_PID=$!
echo "Server PID: $SERVER_PID"
wait_for_server_ready --port "$PORT" --server-log "$SERVER_LOG" --server-pid "$SERVER_PID"
# ---- Run benchmark ----------------------------------------------------------
build_replay_cmd "$RESULT_DIR"
run_agentic_replay_and_write_outputs "$RESULT_DIR"
14 changes: 14 additions & 0 deletions configs/amd-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3094,3 +3094,17 @@ minimaxm3-fp8-mi325x-vllm-agentic:
- { tp: 4, ep: 4, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, conc-list: [3, 4, 5, 6, 7, 8, 9, 10, 12, 14, 16] }
- { tp: 8, ep: 8, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, conc-list: [10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 32] }
- { tp: 8, ep: 8, dp-attn: true, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, conc-list: [24, 32, 36, 40, 44, 48, 52, 56, 60, 64, 72, 80, 96], router: { name: vllm-router, version: "0.1.14" } }

minimaxm3-fp4-mi355x-vllm-agentic:
image: vllm/vllm-openai-rocm:nightly-69715823df89b11ee684b84066390cbb9092d5c1
model: amd/MiniMax-M3-MXFP4
model-prefix: minimaxm3
runner: cluster:mi355x-amds
precision: fp4
framework: vllm
multinode: false
scenarios:
agentic-coding:
- dram-utilization: 0.80
search-space:
- { tp: 4, kv-offloading: dram, kv-offload-backend: { name: lmcache, version: "0.5.1" }, conc-list: [1, 4, 8, 16, 32] }
7 changes: 7 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4750,3 +4750,10 @@
- "Image: lmsysorg/sglang:nightly-dev-cu13-20260709-074bb928"
- "6 topologies across 1k/1k and 8k/1k: 1P1D TP4 STP + wide-EP (DEP4 prefill / DEP16 decode) from 1P1D up to 8P1D, recipes under benchmarks/multi_node/srt-slurm-recipes/sglang/qwen3.5/gb300-fp8/"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2137

- config-keys:
- minimaxm3-fp4-mi355x-vllm-agentic
description:
- "Add Minimax-M3 FP4 vLLM Single Node Agentic Support"
- "Image: vllm/vllm-openai-rocm:nightly-69715823df89b11ee684b84066390cbb9092d5c1"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2129
Loading