fix: size the GLM-5.2 MI355X eval HiCache pool absolutely - #2983
Conversation
|
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[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good — a small, well-scoped bash fix. Reviewed the EVAL_ONLY branch added to glm5.2_fp4_mi355x_sglang_mtp.sh (confirmed the sweep path's --hicache-ratio "$HICACHE_RATIO" --hicache-size 0 args are byte-for-byte unchanged, just routed through the new HICACHE_POOL_ARGS variable), the new test_agentic_eval_hicache.py (executes the real CACHE_ARGS block via bash stubs, not a reimplementation), and the CI workflow path/list additions in test-changelog-gate.yml. No perf-changelog.yaml entry is needed since the throughput-sweep serve shape is unchanged.
Extended reasoning...
Overview
The diff touches three files: a benchmark shell script (adds an EVAL_ONLY branch that sizes the HiCache host pool absolutely via --hicache-size $HICACHE_EVAL_SIZE_GB instead of the ratio-based --hicache-ratio, refactored through shared HICACHE_POOL_ARGS/HICACHE_POOL_DESC variables used by both the plain-HiCache and HiCache+Mooncake code paths), a new pytest file that extracts and executes the script's CACHE_ARGS block in bash-stub isolation, and a CI workflow YAML that adds both files to the path-trigger list and the new test to the pytest invocation.
Security risks
None. This is an internal benchmark/eval script for sizing a memory pool; no user input, auth, or external data handling is involved. The new test spawns bash -c with a fixed, hardcoded script body and stubbed builtins — no injection surface.
Level of scrutiny
Low-to-moderate is appropriate: this is a narrowly-scoped bash conditional gated by an existing EVAL_ONLY env var, and I traced through the full script to confirm the non-eval (sweep) path's emitted args are unchanged from before the diff (--hicache-ratio "$HICACHE_RATIO" --hicache-size 0), just refactored into a variable. The new if/else only changes behavior when EVAL_ONLY=true, which per AGENTS.md/PR description is exactly the previously-broken case (server OOM-killed during GSM8K eval). I also verified the CI YAML additions are simple, mechanical path/list entries matching the new/changed files, with no other edits.
Other factors
The PR includes a detailed, verifiable root-cause narrative (timestamps, ratio math, DRAM-share budget) and states the change was verified by mutation testing (reverting the branch fails 3 of the 8 new tests). The new tests exercise the actual script logic rather than a reimplementation, matching the repo's stated test-quality bar. A previously-investigated candidate issue (that HICACHE_EVAL_SIZE_GB="${HICACHE_EVAL_SIZE_GB:-16}" doesn't handle an explicit 0 override specially) was already flagged as a minor/inline finding rather than a blocking bug, and does not change my overall assessment of correctness for the intended use case. No perf-changelog.yaml entry is needed since the sweep serve shape is unchanged, consistent with AGENTS.md's requirement (which is about benchmark-affecting changes) and the PR author's own explanation.
EVAL_ONLY inherited the agentic corpus' host-pool sizing. --hicache-ratio is relative to the device pool, so ratio 1.5 resolves to 169.73 GB per rank, or 679 GB across the 4 TP ranks. A TP4 server on cluster:mi355x-amds is budgeted TOTAL_CPU_DRAM_GB=1199: agentic_dram_offload_gb() clamps the node's DRAM to MAX_AGENTIC_AVAILABLE_CPU_DRAM_MIB, scales it by dram-utilization, then by gpu_count/gpus-per-node, so a job on the node's other 4 GPUs owns an equal share. The pool claims 57% of that share, and because it is pinned it is unreclaimable. Overshooting does not fail at allocation time. In the GSM8K eval run the pool allocated on all 4 ranks, the server reported ready and served its warmup /generate, and the OOM killer then SIGKILLed it as soon as lm_eval's first batch started touching those pages. The eval reported no score. GSM8K serves ~1k-token 5-shot prompts at conc 12 and never fills a pool that size, so size it absolutely for EVAL_ONLY: 16 GB/rank is 64 GB across the ranks, 5% of the share. HiCache stays enabled, since covering the host<->device KV copy path is most of what this eval is worth, and 16 GB/rank still holds ~350k tokens so the write policy's DRAM writes and the 5-shot prefix hits both keep going through the host tier. Throughput sweeps keep their ratio-based sizing untouched; --hicache-size 0 leaves the ratio in charge, which is already the server default, so their serve shape is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
dfc3b10 to
e793701
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cff6c0c. Configure here.
3 participants
Problem
The GSM8K eval for
glm5.2_tp4_conc12_kvdram-hicache_spec-mtpreported no score because the SGLang server was SIGKILLed mid-eval (runnermi355x-amds_07).EVAL_ONLYinherited the agentic corpus' host-pool sizing.--hicache-ratiois relative to the device pool, so ratio 1.5 resolved to 169.73 GB per rank — 679 GB across the 4 TP ranks.A TP4 server on
cluster:mi355x-amdsis budgetedTOTAL_CPU_DRAM_GB=1199, not the node's RAM:agentic_dram_offload_gb()clamps node DRAM toMAX_AGENTIC_AVAILABLE_CPU_DRAM_MIB, scales bydram-utilization, then bygpu_count/gpus-per-node, so a job on the node's other 4 GPUs owns an equal share. The pool therefore claimed 57% of this server's share, and being pinned it is unreclaimable.Overshooting a pinned pool does not fail at allocation time, which is why this looked like a mystery kill:
/generate200 OKlm_evalstarts its first batch132625 Killed— OOM killer, connection refusedFix
GSM8K serves ~1k-token 5-shot prompts at conc 12 and never fills a pool that size, so size it absolutely under
EVAL_ONLY: 16 GB/rank = 64 GB across ranks, 5% of the share.HiCache stays enabled deliberately — covering the host↔device KV copy path is most of what this eval is worth (cf. the recent upstream HiCache correctness fixes in pointer handling and copy round widths). At 16 GB/rank the pool still holds ~350k tokens, so the write policy's DRAM writes and the 5-shot prefix hits both keep going through the host tier.
Throughput sweeps are untouched. They keep ratio-based sizing;
--hicache-size 0leaves the ratio in charge and is already the server default, so the sweep serve shape is behaviourally identical tomain:Verification
The emitted
CACHE_ARGSwere checked by executing the script's cache block in isolation (its prologue downloads weights and waits on GPUs, so the block is exercised on its own) across the eval path, both sweep arms, the Mooncake arm, and theHICACHE_EVAL_SIZE_GBoverride — including that--hicache-sizeis emitted exactly once, since it overrides--hicache-ratio. Confirmed by mutation: reverting theEVAL_ONLYbranch makes those checks fail.No
perf-changelog.yamlentry: this changes no throughput serve shape, andrun-sweep.ymlonly triggers on that file.Note
Low Risk
Benchmark launch-script tuning only; no change to throughput sweep serve flags beyond clearer pooling of existing ratio-based args.
Overview
Fixes GSM8K eval-only runs on the GLM-5.2 MI355X agentic benchmark script that were dying mid-
lm_evalwhen the host HiCache pool was sized with the agentic ratio (1.5× device pool → ~170 GB/rank), consuming most of the job’s pinned CPU DRAM share and triggering the OOM killer.When
EVAL_ONLY=true, the script now passes--hicache-size(default 16 GB/rank, overridable viaHICACHE_EVAL_SIZE_GB) instead of--hicache-ratio, while keeping HiCache enabled for the host↔device KV path. Throughput sweeps are unchanged: they still emit--hicache-ratioplus--hicache-size 0.Both hicache and mooncake arms share new
HICACHE_POOL_ARGS/HICACHE_POOL_DESCso logs andCACHE_ARGSstay consistent.Reviewed by Cursor Bugbot for commit cff6c0c. Bugbot is set up for automated code reviews on this repo. Configure here.