-
Notifications
You must be signed in to change notification settings - Fork 292
Switch GLM-5.2 MI355X HiCache write policy to write_through #3008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,11 +72,10 @@ export SGLANG_OPT_USE_TOPK_V2=false | |
| # | ||
| # Per-arm L2 ratio (sizing rationale below) applies to both backends unless | ||
| # overridden via HICACHE_RATIO. TP arm (182.7 GB/rank device pool): the | ||
| # agentic-coding corpus saturates any fixed DRAM pool at conc ≥ 10; ratio 1.5 | ||
| # (~2.9 TB pinned) is the safe default for cluster:mi355x-amds nodes (~3.0 TB | ||
| # available DRAM per runners.yaml). ratio=2.5 (~4.8 TB) yields higher | ||
| # throughput at conc 10-12 but exceeds physical DRAM on these nodes and must | ||
| # be set via HICACHE_RATIO env-var override on nodes that can accommodate it. | ||
| # agentic-coding corpus saturates any fixed DRAM pool at conc ≥ 10; ratio 1.0 | ||
| # (~453 GB pinned at TP4) is the default. Larger ratios trade DRAM headroom | ||
| # for host-tier capacity and must be set via the HICACHE_RATIO env-var | ||
| # override on nodes that can accommodate them. | ||
| # The DP-attention arm (159.4 GB/rank) only runs at conc >= 32, where the host | ||
| # tier just absorbs overflow - ratio 0.5 (~1.2 TB pinned, ~1.8 TB of load | ||
| # headroom) at negligible hit-rate cost (ratio 1.5 OOMs the host mid-storm at | ||
|
|
@@ -86,14 +85,11 @@ if agentic_kv_offload_enabled; then | |
| if [ "$DP_ATTENTION" = "true" ]; then | ||
| HICACHE_RATIO="${HICACHE_RATIO:-0.5}" | ||
| else | ||
| # ratio=1.5 (~2.9 TB pinned): safe default within the ~3.0 TB DRAM | ||
| # available on cluster:mi355x-amds nodes. Set HICACHE_RATIO=2.5 via | ||
| # env-var override for maximum throughput on nodes with >4 TB DRAM. | ||
| HICACHE_RATIO="${HICACHE_RATIO:-1.5}" | ||
| # ratio=1.0 (~113 GB/rank, ~453 GB pinned at TP4). Raise via the | ||
| # HICACHE_RATIO env-var override on nodes with more DRAM headroom. | ||
| HICACHE_RATIO="${HICACHE_RATIO:-1.0}" | ||
| 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_WRITE_POLICY="${HICACHE_WRITE_POLICY:-write_through}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 This diff changes default HICACHE_RATIO (1.5->1.0) and HICACHE_WRITE_POLICY (write_through_selective->write_through) for the TP arm, both of which affect benchmark performance, but adds no perf-changelog.yaml entry, violating AGENTS.md's non-negotiable invariant that every performance-affecting change requires one. Fix: append a new perf-changelog.yaml entry (byte-preserving, tail-only) describing this default change for the affected config-keys before merging. [also at: perf-changelog.yaml:1 - Two perf-affecting default changes (HICACHE_RATIO 1.5->1.0, HICACHE_WRITE_POLICY…] Extended reasoning...AGENTS.md line 31: "Every change that can affect benchmark performance ... requires a new perf-changelog.yaml entry. The file is append-only and byte-sensitive." The diff lowers HICACHE_RATIO from 1.5 to 1.0 (less host DRAM pinned, lower cache hit rate under high concurrency) and switches HICACHE_WRITE_POLICY from write_through_selective to write_through (every KV write now hits the host bus instead of skipping non-reusable blocks), both explicitly performance-tuning changes per the PR description itself. Verification: normal (raised by review-scope AGENTS.md non-negotiable invariant). The diff changes performance-affecting HiCache defaults for the TP arm — |
||
| HICACHE_IO_BACKEND="${HICACHE_IO_BACKEND:-direct}" | ||
| HICACHE_MEM_LAYOUT="${HICACHE_MEM_LAYOUT:-page_first_direct}" | ||
| case "$KV_OFFLOAD_BACKEND" in | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recipe change omits changelog entry
Medium Severity
The new HiCache defaults (
HICACHE_RATIO1.0andHICACHE_WRITE_POLICYwrite_through) change published AgentX results forglm5.2-fp4-mi355x-sglang-agentic-mtp, but this PR does not append aperf-changelog.yamlentry. Changelog-driven sweeps will not select the recipe, so the new curve is not recorded.Reviewed by Cursor Bugbot for commit b169be3. Configure here.