Revert "fix: size the GLM-5.2 MI355X eval HiCache pool absolutely (#2983)" - #3002
Conversation
)" This reverts commit a5f23f4.
|
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}
| # env-var override for maximum throughput on nodes with >4 TB DRAM. | ||
| HICACHE_RATIO="${HICACHE_RATIO:-1.5}" | ||
| fi | ||
| # GSM8K never fills the agentic host pool; ratio 1.5 OOMs the TP4 DRAM share. | ||
| if [ "${EVAL_ONLY:-false}" = "true" ]; then | ||
| HICACHE_EVAL_SIZE_GB="${HICACHE_EVAL_SIZE_GB:-16}" | ||
| HICACHE_POOL_ARGS=(--hicache-size "$HICACHE_EVAL_SIZE_GB") | ||
| HICACHE_POOL_DESC="size=${HICACHE_EVAL_SIZE_GB} GB/rank (eval-only)" | ||
| else | ||
| HICACHE_POOL_ARGS=(--hicache-ratio "$HICACHE_RATIO" --hicache-size 0) | ||
| HICACHE_POOL_DESC="ratio=$HICACHE_RATIO" | ||
| fi | ||
| # write_through_selective skips DRAM writes for non-reusable KV blocks, | ||
| # reducing host-bus traffic without affecting the cache hit rate. | ||
| HICACHE_WRITE_POLICY="${HICACHE_WRITE_POLICY:-write_through_selective}" | ||
| HICACHE_IO_BACKEND="${HICACHE_IO_BACKEND:-direct}" | ||
| HICACHE_MEM_LAYOUT="${HICACHE_MEM_LAYOUT:-page_first_direct}" | ||
| case "$KV_OFFLOAD_BACKEND" in | ||
| hicache) | ||
| echo "HiCache (GPU+host DRAM only): $HICACHE_POOL_DESC, write_policy=$HICACHE_WRITE_POLICY, io_backend=$HICACHE_IO_BACKEND, mem_layout=$HICACHE_MEM_LAYOUT" | ||
| echo "HiCache (GPU+host DRAM only): ratio=$HICACHE_RATIO, write_policy=$HICACHE_WRITE_POLICY, io_backend=$HICACHE_IO_BACKEND, mem_layout=$HICACHE_MEM_LAYOUT" | ||
| CACHE_ARGS=( | ||
| --enable-hierarchical-cache | ||
| "${HICACHE_POOL_ARGS[@]}" | ||
| --hicache-ratio "$HICACHE_RATIO" | ||
| --hicache-write-policy "$HICACHE_WRITE_POLICY" | ||
| --hicache-io-backend "$HICACHE_IO_BACKEND" | ||
| --hicache-mem-layout "$HICACHE_MEM_LAYOUT" |
There was a problem hiding this comment.
🔴 This revert removes the EVAL_ONLY branch that capped the HiCache pool at a small fixed size and reinstates the ratio=1.5 sizing for eval-only GSM8K runs, but no new perf-changelog.yaml entry was added for config-key glm5.2-fp4-mi355x-sglang-agentic-mtp, violating AGENTS.md's non-negotiable rule that every recipe modification requires an append-only changelog entry. Fix: append a perf-changelog.yaml entry documenting this revert and its rationale (EAGLE bug fix upstream, not a DRAM-sharing issue) for this config key.
Extended reasoning...
AGENTS.md line 31 states every change that can affect benchmark performance and every recipe modification requires a new perf-changelog.yaml entry, appended at the tail. Grepping perf-changelog.yaml for glm5.2-fp4-mi355x-sglang-agentic-mtp shows only earlier entries (PR #2570, #2562) and no new entry tied to this diff/PR. This is a recipe modification (HiCache pool sizing behavior for EVAL_ONLY changes), so the changelog invariant is violated regardless of whether the revert itself is correct.
Verification: normal (repository non-negotiable invariant). The diff modifies a benchmark recipe and changes HiCache pool sizing behavior for EVAL_ONLY runs — it removes the EVAL_ONLY branch (deleted lines setting --hicache-size "$HICACHE_EVAL_SIZE_GB") and reinstates --hicache-ratio "$HICACHE_RATIO" (default 1.5) for both backends in benchmarks/single_node/agentic/glm5.2_fp4_mi355x_sglang_mtp.sh.…
|
Agree on the scheduling point: launch_mi355x-amds.sh uses salloc ... --exclusive, so there is no co-tenancy here. The sentence in #2983 about another job owning the other 4 GPUs was wrong and I'll remove it. TOTAL_CPU_DRAM_GB=1199 is only a sizing budget, not evidence of co-tenancy. I still think the failure itself is host OOM. EVAL_ONLY inherited --hicache-ratio 1.5; with a 127.90 GB/rank device pool that became 169.73 GB/rank pinned host memory, ~679 GB across TP4. The pool is registered up front but faulted in lazily, which matches the log: server ready → first lm_eval batch → SIGKILL. Also, --exclusive is not the same as --mem=0; the MI355X launcher does not request --mem=0. I don't think sgl-project/sglang#38318 explains this one: there was no kv_index_translator traceback, and both the failing and green runs used the same pre-#38318 image. The only relevant change was reducing the host pool; with --hicache-size 16, GSM8K completed at 0.9682 flexible / 0.9666 strict: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/34535781749 |
2 participants
Reverts #2983 (merge commit a5f23f4).
The HiCache pool sizing change was a workaround for a symptom, not the cause. The node runs one job at a time under
--exclusive, so the shared-DRAM-budget explanation in #2983 does not hold, and it does not explain why only the eval path was crashing. The GSM8K crash is believed to be the SGLang EAGLE bug fixed upstream in sgl-project/sglang#38318, so the recipe change should not carry forward.Discussed in #ext-amd-sw-engineering-semianalysis; Bryan Shan agreed to revert.
Note
Low Risk
Benchmark script-only change; restores prior HiCache CLI flags with no production runtime impact.
Overview
Reverts the eval-only HiCache pool workaround from #2983 in the GLM-5.2 MI355X agentic benchmark launcher.
When
agentic_kv_offload_enabledis on, bothhicacheandmooncakepaths again always pass--hicache-ratio(with DP vs TP defaults unchanged) instead of switching to a fixed--hicache-size(16 GB default) whenEVAL_ONLY=true. The Mooncake arm again sets--hicache-size 0alongside the ratio. Logging no longer usesHICACHE_POOL_DESCand always reportsratio=$HICACHE_RATIO.Reviewed by Cursor Bugbot for commit 7c84942. Bugbot is set up for automated code reviews on this repo. Configure here.