Skip to content
Merged
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
42 changes: 22 additions & 20 deletions benchmarks/single_node/agentic/glm5.2_fp4_b300_sglang_mtp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,33 +59,35 @@ if require_agentic_kv_offload_backend hicache; then
# conc 8 (TP8) / 64 (DP8) and the radix hit rate collapses to <0.1
# against a ~0.97 theoretical ceiling, so every turn re-prefills its
# whole history; the host tier restores those hits at C2C bandwidth.
# GLM-5.2 is DSA/MLA-family (attention_backend=dsa): every rank holds
# complete per-token KV (169.98 GB device pool per rank, replicated on
# all 8 ranks), so host capacity is controlled through the host/device
# token-capacity ratio like the DSv4 recipe, NOT a per-rank
# --hicache-size. A GB-based size of TOTAL_CPU_DRAM_GB/TP pinned the
# whole 0.80-DRAM budget (8 x 299 GB) at init on top of 465 GB of
# weights and OOM-killed the node (run 29678598595); DSv4's own
# ratio=2 default pins 2 x 170 GB x 8 = 2.7 TB here and OOMs too
# (GLM-5.2's device pool is far larger than DSv4's). Fractional 0.75
# = ~128 GB/rank = ~1.0 TB total, matching the cluster's proven ~1 TB
# host-pool envelope; validated on-node 2026-07-19 (boot + 4.2M-token
# overflow bench forcing eviction through the DSA KV+INDEXER pools).
# The ratio is relative to the device pool, so MTP's slightly smaller
# device pool (the nextn layer takes its own KV) only shrinks it.
DEFAULT_HICACHE_RATIO=0.75
HICACHE_RATIO="${HICACHE_RATIO:-$DEFAULT_HICACHE_RATIO}"
if awk -v r="$HICACHE_RATIO" -v cap="$DEFAULT_HICACHE_RATIO" 'BEGIN { exit !(r > cap) }'; then
echo "Error: HICACHE_RATIO=$HICACHE_RATIO exceeds configured limit $DEFAULT_HICACHE_RATIO" >&2
# GLM-5.2 is DSA/MLA-family (attention_backend=dsa): every TP rank holds
# complete per-token KV. Use an absolute main-pool size so all points get
# the same host token capacity even if the HBM pool changes. In SGLang
# v0.5.16, --hicache-size directly sizes only the target KV host pool;
# the DSA indexer and MTP draft pools inherit its token-slot count and use
# their own smaller bytes/token. A 270 GB target pool was measured as
# 270.00 + 61.88 + 3.46 = 335.34 GB/rank, or 2.683 TB across TP8, with
# 6,009,728 slots in each pool. This is larger than the ratio-1.50 c16
# configuration whose full run improved from 76.26 / 10,311.95 to
# 100.49 tok/s/user / 12,120.27 tok/s/GPU and retained ~1.74 TiB minimum
# node MemAvailable.
DEFAULT_HICACHE_SIZE=270
MAX_HICACHE_SIZE=270
HICACHE_SIZE="${HICACHE_SIZE:-$DEFAULT_HICACHE_SIZE}"
if ! [[ "$HICACHE_SIZE" =~ ^[0-9]+$ ]]; then
echo "Error: HICACHE_SIZE must be a positive integer, got $HICACHE_SIZE" >&2
exit 1
fi
if awk -v s="$HICACHE_SIZE" -v cap="$MAX_HICACHE_SIZE" 'BEGIN { exit !(s <= 0 || s > cap) }'; then
echo "Error: HICACHE_SIZE=$HICACHE_SIZE must be in (0, $MAX_HICACHE_SIZE]" >&2
exit 1
fi
HICACHE_WRITE_POLICY="${HICACHE_WRITE_POLICY:-write_back}"
Comment on lines +73 to 84

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.

🟡 sweep:BEGIN { exit !(.||.) }
The awk subprocess here performs a plain-integer bounds check that bash can do natively, since HICACHE_SIZE is already validated as a non-negative integer by the ^[0-9]+$ regex on the preceding line. E.g. in benchmarks/single_node/agentic/glm5.2_fp4_b300_sglang_mtp.sh:80, awk -v s="$HICACHE_SIZE" -v cap="$MAX_HICACHE_SIZE" 'BEGIN { exit !(s <= 0 || s > cap) }' can become [ "$HICACHE_SIZE" -eq 0 ] || [ "$HICACHE_SIZE" -gt "$MAX_HICACHE_SIZE" ], matching the native-bash pattern already used by sibling recipes (e.g. qwen3.5_fp4_b200_sglang_mtp.sh:61).

Extended reasoning...

The awk invocation at benchmarks/single_node/agentic/glm5.2_fp4_b300_sglang_mtp.sh:80awk -v s="$HICACHE_SIZE" -v cap="$MAX_HICACHE_SIZE" 'BEGIN { exit !(s <= 0 || s > cap) }' — is a leftover from the old HICACHE_RATIO check this PR replaces. That old check validated a fractional value (DEFAULT_HICACHE_RATIO=0.75), and bash has no native floating-point comparison, so shelling out to awk was the right call there.

This PR switches the variable to an integer-only HICACHE_SIZE, and critically, the line immediately above the awk call (benchmarks/single_node/agentic/glm5.2_fp4_b300_sglang_mtp.sh:76) already gates entry with [[ "$HICACHE_SIZE" =~ ^[0-9]+$ ]], so by the time execution reaches the awk line, HICACHE_SIZE is guaranteed to be a plain non-negative integer string. At that point the awk BEGIN { exit !(s <= 0 || s > cap) } reduces to a pure integer bounds check, which is exactly what bash's [ ]/test integer comparison operators (-eq, -gt, etc.) are built to do without spawning a subprocess.

Step-by-step proof:

  1. Line 76: [[ "$HICACHE_SIZE" =~ ^[0-9]+$ ]] fails (and the script exits 1) for anything that is not a sequence of digits — no negative numbers, no decimals, no non-numeric strings can reach line 80.
  2. Line 80 therefore only ever awk-evaluates two already-validated non-negative integers, s (=HICACHE_SIZE) and cap (=MAX_HICACHE_SIZE).
  3. awk 'BEGIN { exit !(s <= 0 || s > cap) }' with two plain integers is behaviorally identical to [ "$HICACHE_SIZE" -eq 0 ] || [ "$HICACHE_SIZE" -gt "$MAX_HICACHE_SIZE" ] (bash's test does base-10 integer comparison for these operators, with no octal reinterpretation of leading zeros, so there is no edge case where the two diverge for a digit-only string).
  4. Every sibling agentic HiCache recipe already uses the native-bash form for integer size checks — e.g. qwen3.5_fp4_b200_sglang_mtp.sh:61 and qwen3.5_fp4_b300_sglang_mtp.sh:61 both do if [ "$HICACHE_SIZE_GB" -lt 1 ] || [ "$HICACHE_SIZE_GB" -gt "$MAX_HICACHE_SIZE_GB" ]. The awk BEGIN{exit ...} idiom appears nowhere else in benchmarks/ except this file and its B200 sibling's fractional-ratio check (where awk is still required).

Existing code does not prevent this because nothing here is incorrect — the awk call works fine, it is just an unnecessary subprocess spawn and a less-idiomatic form for what is now a trivial integer comparison, carried over unchanged from the pre-PR fractional-ratio logic. The impact is negligible (one extra fork/exec at script startup, no correctness or performance issue in the benchmark run itself), so this is a pure readability/cleanliness nit, not something that blocks merging.

Fix: replace the awk call with the native bash form shown above. As a secondary, weaker observation, DEFAULT_HICACHE_SIZE and MAX_HICACHE_SIZE (lines 73-74) are both hardcoded to the literal 270; the prior code avoided this duplication by reusing DEFAULT_HICACHE_RATIO itself as the cap. This is a minor, defensible stylistic choice (separating "default" from "hard cap" is a reasonable pattern) rather than a bug, but collapsing them to one constant would remove the duplication if desired.

HICACHE_IO_BACKEND="${HICACHE_IO_BACKEND:-direct}"
HICACHE_MEM_LAYOUT="${HICACHE_MEM_LAYOUT:-page_first_direct}"
echo "HiCache CPU tier: ratio=$HICACHE_RATIO, capacity=${TOTAL_CPU_DRAM_GB} GB, write_policy=$HICACHE_WRITE_POLICY, io_backend=$HICACHE_IO_BACKEND, mem_layout=$HICACHE_MEM_LAYOUT"
echo "HiCache CPU tier: conc=$CONC, target_size=$HICACHE_SIZE GB, total_capacity=${TOTAL_CPU_DRAM_GB} GB, write_policy=$HICACHE_WRITE_POLICY, io_backend=$HICACHE_IO_BACKEND, mem_layout=$HICACHE_MEM_LAYOUT"
CACHE_ARGS=(
--enable-hierarchical-cache
--hicache-ratio "$HICACHE_RATIO"
--hicache-size "$HICACHE_SIZE"
--hicache-write-policy "$HICACHE_WRITE_POLICY"
Comment on lines +75 to 91

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 270 GB/rank --hicache-size is a hardcoded constant rather than being derived from TOTAL_CPU_DRAM_GB, which per benchmarks/single_node/agentic/README.md all backends must consume unless they only expose --hicache-ratio (the DSv4 exception) — this recipe now uses the absolute --hicache-size flag, so it no longer qualifies. The measured footprint (335.34 GB/rank × TP8 = 2,683 GB) exceeds the README's own worked TP8/B300 budget of 2,399 GB by ~12%, and being fixed, it won't shrink if dram-utilization or cluster DRAM is later lowered.

Extended reasoning...

What the bug is: benchmarks/single_node/agentic/README.md states a DRAM KV-offload policy: 'Benchmark scripts must consume TOTAL_CPU_DRAM_GB rather than replace it with model-specific constants. Backends with per-rank or per-pool settings must divide this aggregate budget accordingly. DSv4 SGLang is the exception because it exposes only --hicache-ratio.' Before this PR, glm5.2_fp4_b300_sglang_mtp.sh used --hicache-ratio (the documented exception style). This PR switches to --hicache-size, an absolute per-pool flag, sized from a new hardcoded DEFAULT_HICACHE_SIZE=270 (GB/rank) constant. TOTAL_CPU_DRAM_GB is still a required env var and still appears in the code, but only in the logging echo on line 87 — it is never consumed in the size calculation anymore.\n\nCode path: In the modified script (lines ~75-91), HICACHE_SIZE defaults to the constant 270 and is validated only against a hardcoded ceiling MAX_HICACHE_SIZE=270 — never against TOTAL_CPU_DRAM_GB. Since GLM-5.2 moved off --hicache-ratio in this same PR, it lost the one property (relative sizing to the device pool via a ratio) that let it stand in for budget-awareness; it is now squarely a 'per-pool setting' the README says must divide the aggregate budget, and it doesn't.\n\nWhy nothing else catches it: The only guard present is the (0, MAX_HICACHE_SIZE] bounds check, which is a static self-referential cap (270 ⇐ 270) — it can never reject a value that's actually too large for the cluster's real DRAM budget, because it doesn't look at TOTAL_CPU_DRAM_GB at all.\n\nConcrete proof (budget math): configs/runners.yaml cluster b300-nv has available-cpu-dram-mib: 2,964,436 and gpus-per-node: 8; configs/nvidia-master.yaml pins this recipe at tp: 8, dram-utilization: 0.80. Applying the README's own documented formula, floor(min(2,964,436, 2,861,022) * 1,048,576 * 0.80 * 8/8 / 1e9), yields TOTAL_CPU_DRAM_GB ≈ 2,399 GB — matching the README's own worked TP8/B300 example exactly. But the PR's own added comment states the measured per-rank pool footprint is 270.00 + 61.88 (indexer) + 3.46 (MTP draft) = 335.34 GB/rank, i.e. 335.34 × TP8 = 2,683 GB aggregate — about 284 GB (12%) over the 2,399 GB budget the script is supposed to respect, before even accounting for model weights (~465 GB, per the removed comment) or other host memory.\n\nImpact: Because the 270 value is a fixed constant, it will not scale down if dram-utilization is lowered or the cluster's available DRAM shrinks (e.g. a different B300 SKU or a future runner config change) — defeating the entire point of computing TOTAL_CPU_DRAM_GB as a budget in the first place. The removed comment in this same file documents that an earlier GB-based sizing attempt (TOTAL_CPU_DRAM_GB/TP = 299 GB/rank) already OOM-killed the node once (run 29678598595), which underscores that absolute host-memory sizing on this recipe is a real, previously-hit failure mode, not a hypothetical.\n\nSuggested fix: Derive HICACHE_SIZE from TOTAL_CPU_DRAM_GB divided by TP with a documented discount for the indexer/MTP-draft overhead (e.g. floor(TOTAL_CPU_DRAM_GB / TP * discount_factor)), so the pool scales with the generated budget while still avoiding the OOM that a naive undiscounted division hit previously. Alternatively, if the team decides GLM-5.2's absolute-size requirement genuinely can't track the aggregate budget, the README should be updated to add GLM-5.2 as a second documented exception alongside DSv4, so the convention and the code stay in sync.\n\nMitigating factors (why this is a nit, not blocking): The 270 GB value was empirically validated on-node, with the PR comment citing ~1.74 TiB of retained MemAvailable during a full run, and 2,683 GB fits comfortably within the B300 node's ~3.1 TB of physical DRAM. The PR also carries the full-sweep-fail-fast label, so CI will surface any OOM before merge on the pinned cluster/config. The risk is prospective (future dram-utilization or cluster DRAM changes) and a documented-convention violation, not an issue that manifests as a concrete failure under the PR's own tested conditions.

--hicache-io-backend "$HICACHE_IO_BACKEND"
--hicache-mem-layout "$HICACHE_MEM_LAYOUT"
Expand Down
8 changes: 8 additions & 0 deletions perf-changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6086,3 +6086,11 @@
- "Use TP8/EP1 concurrency 2 through 24, published FlashInfer 0.6.17 CUDA 13 wheels, and golden MTP acceptance length 3.39."
- "Use the lmsysorg/sglang:nightly-dev-cu13-20260815-a5ba081f image."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2592

- config-keys:
- glm5.2-fp4-b300-sglang-agentic-mtp
scenario-type:
- agentic-coding
description:
- "Use a 270 GB/rank absolute HiCache target pool for glm5.2-fp4-b300-sglang-agentic-mtp, replacing the previous ratio-based sizing."
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2651