Skip to content

[mem_cache] test: ratchet the allocator and pool_host layout - #35638

Draft
alphabetc1 wants to merge 1 commit into
sgl-project:mainfrom
alphabetc1:refactor/mem-cache-layout-ratchet
Draft

alphabetc1 wants to merge 1 commit into
sgl-project:mainfrom
alphabetc1:refactor/mem-cache-layout-ratchet

Conversation

@alphabetc1

@alphabetc1 alphabetc1 commented Aug 20, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

Part of #25371. A directory is not a rule.

allocator/ landed in May and pool_host/ has been landing since June, but nothing
stops new code from ignoring them. #29678 added multi_ended_allocator.py — 2559
lines, 3 allocator classes — at the mem_cache/ top level two months after
allocator/ existed. And while the split was being agreed, memory_pool.py grew from
2257 to 5042 lines and from 11 to 17 classes.

The restructure only converges if the layout cannot regress while it is in progress.
This adds that guard, plus the skill that tells coding agents the same rules.

Modifications

test/registered/unit/mem_cache/test_mem_cache_layout_ratchet.py — four shrink-only
pins, each covering one way the layout regressed:

Pin Catches
R1 _LEGACY_HOMES a class filed under the wrong layer directory
R2 _TOP_LEVEL_MODULES a new catch-all module at the mem_cache/ root
R3 _SHRINKING_MODULES a new class appended to a module slated for deletion
R4 _FORBIDDEN_DEPS an import that reverses a layer boundary

Role is decided by transitive base class, never by class name — HostTensorAllocator
is named Allocator but hands out pinned host memory, not KV slots. Class names do
collide (three modules define TreeNode), so ancestry is unioned across same-named
definitions: over-approximating turns a collision into a review conversation, where
keeping only one definition would turn it into a silent miss.

R1 covers allocator/ and pool_host/ only. mem_cache/pool/ does not exist yet,
and an error telling someone to file a class into a package nobody has created is a
trap, not a rule. R3 stands in for the pool layer meanwhile — it needs no target
package to exist, and it is the rule that catches the case R1 and R2 both miss: a class
appended to a file that already exists, which is exactly how memory_pool.py gained
PageMajorMHATokenToKVPool, MHATokenToKVPoolMXFP8, MHATokenToKOnlyPool,
MiniMaxSparseKVPool and DSATokenToKVPool. When the first pool/ PR lands, pool
joins the enforced roles and its classes move from R3's pin into _LEGACY_HOMES.

Every pin also fails when it rots — a class that has moved, a module that is gone.
So each migration PR under #25371 shrinks a pin, and the pins double as the roadmap's
progress ledger.

R4 passes on main today, so it locks in a property the code already has rather than
demanding new work. Scope is python/sglang/srt/mem_cache/ only; hardware_backend/
is explicitly excluded as a separate device axis.

Also adds .claude/skills/mem-cache-layout/SKILL.md and its trigger line in
.claude/rules/modify-component-must-read.md. The skill teaches; only the ratchet
blocks — human PRs never read .claude/skills/.

Accuracy Test

Not applicable — no runtime code changes. The test is AST-only and reads no tensors.

Benchmark and Profiling Results

Not applicable.

Checklist

  • Format your code according to the Code Formatting with Pre-Commit.
  • Add unit tests as outlined in the Running Unit Tests.
  • Update documentation / docstrings / example tutorials as needed, according to Writing Documentation.
  • Provide throughput / latency benchmark results and accuracy evaluation results as needed, according to Benchmark and Profiling.
  • For reviewers: If you haven't made any contributions to this PR and are only assisting with merging the main branch, please remove yourself as a co-author when merging the PR.
  • Please feel free to join our Slack channel at https://slack.sglang.ai to discuss your PR.

Stacked on #35306 ([mem_cache][9/N] DSAIndexerPoolHost). The pins are generated
against a tree with that move applied. Draft until #35306 merges; landing this first
instead would only mean adding DSAIndexerPoolHost back to _LEGACY_HOMES and
_SHRINKING_MODULES["memory_pool_host.py"].

Verification

Run on a GB300 devbox against the real package (PYTHONPATH shadowing confirmed via
sglang.srt.__path__, so the test reads the working tree, not the image's copy):

4 passed, 16 warnings in 17.75s

A guard nobody has seen go red is not a guard, so all six failure paths were exercised
by injecting the violation into a scratch copy of the tree — the four rules above, plus
the two rot cases (a pinned class that moved, a pinned module that was deleted). Each
fired with an actionable message naming the target file. The HostKVCache-subclass
injection was additionally reproduced on the devbox and the tree restored (md5 checked).

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ❌ Run #33621060344
Latest PR Test (Extra): ❌ Run #33621060175
Latest PR Test (AMD ROCm 7.2): ❌ Run #33621060311

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@alphabetc1
alphabetc1 force-pushed the refactor/mem-cache-layout-ratchet branch from 164e0cc to b855c8c Compare September 2, 2026 10:44

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation hicache Hierarchical Caching for SGLang

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant