Size the unified read-table grid from bs, and fuse the allocator's tombstone scatters - #37511
Merged
Merged
Conversation
ch-wan
requested review from
BBuf,
DarkSharpness,
HaiShaw,
HydraQYH,
Ying1123,
alphabetc1,
celve,
hanming-lu,
hnyls2002,
huangtingwei9988,
hzh0425,
ispobock,
merrymercy,
xiezhq-hermann,
yizhang2077 and
yuan-luo
as code owners
September 2, 2026 02:42
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
5 tasks
This was referenced Sep 2, 2026
ch-wan
force-pushed
the
cheng/unified-alloc-fusion
branch
2 times, most recently
from
September 2, 2026 20:54
972729f to
7d39aaf
Compare
`_free_lazy` issued five kernels per call (v2p gather, two `index_fill_`, two `torch.cat`) and `bind` two. The gather and both tombstones are one lane's work per page, so they collapse into a single kernel -- the inverse of the `alloc_bind_inplace` the alloc fast path already uses. `bind` gets the same treatment for the caller-supplied physical range the hole-draining slow path holds. Measured on Kimi-Linear TP2 + unified, bs=64 / 8k in / 10 decode steps, both runs back-to-back on the same idle GPU pair: _free_lazy 305 launches / 430us -> 183 / 259us bind 140 launches / 220us -> 70 / 70us total 71505us -> 71301us (-0.29%) Controls that must not move: all_reduce 2303 -> 2470us, moe 18138 -> 18151us. The two `torch.cat`s stay. `_free_phys_pages` is a `_CapacityField`, whose contract is REBIND-never-mutate -- the rebind is what bumps `_capacity_epoch` and invalidates the `available_size` memos. Appending in place would leave those memos silently stale, which the allocator has an idle-time checker for (`_capacity_memo_violations`). Killing the cats needs a view-into-a-backing- store scheme that keeps rebinding; that is its own change. Both launchers carry a pure-torch CPU path: the allocator's unit tests run on CPU, so without it the Triton kernels have no coverage there. Verified on GPU against the torch implementation over 20 randomized cases (both tables plus the returned physical pages), empty input, and CPU/CUDA agreement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`build_kv_read_table` launched `(bs, cdiv(max_pages, BLOCK))` blocks. In the eager path `max_pages` is the batch's live maximum, but the captured path passes `max_context_len`, and a cuda-graph capture bakes the grid -- so every decode replay ran 4096 column blocks per row for sequences 8k long, and 99.2% of the 262144 blocks loaded two scalars, failed the `cols < n_pages` mask and exited. The cost was the block scheduling, so it did not vary with the sequence length: 158.5us at seq=1024 and 160.2us at seq=131072, one launch per step. The grid now comes from `bs` alone and each program strides over its columns, bounded by the device-side `seq_lens`. The grid stays static, so a capture bakes it; the work is read from memory at replay, so it tracks the real lengths. H100, measured under graph replay (before -> after): bs= 64 seq= 8224 158.7us -> 2.6us 61x <- the profiled decode shape bs= 64 seq= 1024 158.9us -> 1.7us 92x bs=256 seq= 8224 631.3us -> 4.9us 129x bs=256 seq=131072 642.8us -> 81.4us 8x bs= 1 seq= 8224 3.4us -> 1.6us 2x BLOCK 512 / 8 warps / 1024 blocks is the joint optimum over bs 1..256 x seq 1k..128k; the surface is flat within ~10%, so it is a floor, not a tuned point. Verified against an independent CPU model over 30 randomized cases covering page_size 1..4, mixed seq_lens, `-1` holes in req_to_token, tombstoned v2p rows, and a non-zero `out` (the prefix-only contract must leave the tail intact), plus both degenerate shapes. A captured graph replays bit-identically to a fresh launch as seq_lens moves over 1 / 64 / 999 / 4096, which is the property the static grid exists for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`test_free_paths_actually_use_index_fill` asserted the free path's source contains `index_fill_`. The fusion writes the tombstones from inside the Triton kernel instead, which serves the file's actual invariant -- the `-1` never crosses the bus -- strictly better, and the test went red for it. It now checks that one of the sanctioned no-sync forms is present, so deleting the scatter still cannot pass and a new mechanism is added to the list deliberately. That list is only worth something if something runs the mechanism. Nothing did: `virtual_slot.py` has no test in the tree at all, and on CPU both launchers take their pure-torch reference path, so the kernel that does the tombstoning was never executed by the suite. Added CUDA-gated coverage that binds and frees over 20 randomized page sets and checks the returned physical pages, both tombstones, and that live bindings are untouched -- plus CPU/CUDA agreement and the empty free. Dropping `tl.store(p2v_ptr + p, -1)` from the kernel turns two of them red; that is the diff they exist for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ch-wan
force-pushed
the
cheng/unified-alloc-fusion
branch
from
September 2, 2026 23:54
7d39aaf to
9a9fae4
Compare
3 tasks
ch-wan
added a commit
to ch-wan/sglang
that referenced
this pull request
Sep 4, 2026
sgl-project#37511 and sgl-project#37512 took the unified pool's READ path off the per-step launch path: the read table is built by one kernel whose grid is sized from bs, and the flat consumers (fa3, flashinfer, triton, trtllm_mha) were converted to a packed stream. The WRITE path was left eager, and the block-table backends (trtllm_mla, cutedsl_mla, tokenspeed_mla, flashmla) were not converted at all. Hopper's MLA default is fa3, so it got both halves of that work. Blackwell runs the trtllm_mla family, which got neither -- which is why the Hopper fix did not carry over. `translate_kv_loc_for_kernel` was 6 torch ops (floor_divide, remainder, take, mul, add, clamp) and `translate_write_loc_for_kernel` layered 6 more on top (remainder, eq, floor_divide, zeros_like, where, copy). Both run per forward on `out_cache_loc`, OUTSIDE any cuda graph, so each op is a real launch on the critical path. Profiling a bs=8 decode on B300 (24 steps, 2 ranks): GPU kernel time unified 147.56 ms static 153.67 ms -6.11 ms wall span unified 81.10 ms static 77.92 ms +3.18 ms (+4.1%) cudaLaunchKernel unified 988 static 532 +456 cpu_op time unified 51.40 ms static 35.24 ms +16.16 ms GPU time was already LOWER; the loss was ~10 extra launches per step per rank. That is invisible next to a Hopper decode step and is not next to a Blackwell one. Route both translates through one Triton kernel in virtual_slot.py, the module sgl-project#37511 added for exactly this class of fusion. It carries the DCP owner rule, so the widened-loc path is the same single launch. Triton truncates division toward zero where torch floors it, so a negative loc is tested explicitly rather than relying on the division to reach the v2p sentinel row. Measured on B300, TP2 DCP2, cutedsl_mla, unified vs static decode throughput over 6 shape/batch points: mean gap -1.70% -> -0.23%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ch-wan
added a commit
to ch-wan/sglang
that referenced
this pull request
Sep 5, 2026
sgl-project#37511 and sgl-project#37512 took the unified pool's READ path off the per-step launch path: the read table is built by one kernel whose grid is sized from bs, and the flat consumers (fa3, flashinfer, triton, trtllm_mha) were converted to a packed stream. The WRITE path was left eager, and the block-table backends (trtllm_mla, cutedsl_mla, tokenspeed_mla, flashmla) were not converted at all. Hopper's MLA default is fa3, so it got both halves of that work. Blackwell runs the trtllm_mla family, which got neither -- which is why the Hopper fix did not carry over. `translate_kv_loc_for_kernel` was 6 torch ops (floor_divide, remainder, take, mul, add, clamp) and `translate_write_loc_for_kernel` layered 6 more on top (remainder, eq, floor_divide, zeros_like, where, copy). Both run per forward on `out_cache_loc`, OUTSIDE any cuda graph, so each op is a real launch on the critical path. Profiling a bs=8 decode on B300 (24 steps, 2 ranks): GPU kernel time unified 147.56 ms static 153.67 ms -6.11 ms wall span unified 81.10 ms static 77.92 ms +3.18 ms (+4.1%) cudaLaunchKernel unified 988 static 532 +456 cpu_op time unified 51.40 ms static 35.24 ms +16.16 ms GPU time was already LOWER; the loss was ~10 extra launches per step per rank. That is invisible next to a Hopper decode step and is not next to a Blackwell one. Route both translates through one Triton kernel in virtual_slot.py, the module sgl-project#37511 added for exactly this class of fusion. It carries the DCP owner rule, so the widened-loc path is the same single launch. Triton truncates division toward zero where torch floors it, so a negative loc is tested explicitly rather than relying on the division to reach the v2p sentinel row. Measured on B300, TP2 DCP2, cutedsl_mla, unified vs static decode throughput over 6 shape/batch points: mean gap -1.70% -> -0.23%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
StevenChenSE
pushed a commit
to StevenChenSE/sglang
that referenced
this pull request
Sep 6, 2026
…mbstone scatters (sgl-project#37511) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ch-wan
added a commit
to ch-wan/sglang
that referenced
this pull request
Sep 7, 2026
sgl-project#37511 and sgl-project#37512 took the unified pool's READ path off the per-step launch path: the read table is built by one kernel whose grid is sized from bs, and the flat consumers (fa3, flashinfer, triton, trtllm_mha) were converted to a packed stream. The WRITE path was left eager, and the block-table backends (trtllm_mla, cutedsl_mla, tokenspeed_mla, flashmla) were not converted at all. Hopper's MLA default is fa3, so it got both halves of that work. Blackwell runs the trtllm_mla family, which got neither -- which is why the Hopper fix did not carry over. `translate_kv_loc_for_kernel` was 6 torch ops (floor_divide, remainder, take, mul, add, clamp) and `translate_write_loc_for_kernel` layered 6 more on top (remainder, eq, floor_divide, zeros_like, where, copy). Both run per forward on `out_cache_loc`, OUTSIDE any cuda graph, so each op is a real launch on the critical path. Profiling a bs=8 decode on B300 (24 steps, 2 ranks): GPU kernel time unified 147.56 ms static 153.67 ms -6.11 ms wall span unified 81.10 ms static 77.92 ms +3.18 ms (+4.1%) cudaLaunchKernel unified 988 static 532 +456 cpu_op time unified 51.40 ms static 35.24 ms +16.16 ms GPU time was already LOWER; the loss was ~10 extra launches per step per rank. That is invisible next to a Hopper decode step and is not next to a Blackwell one. Route both translates through one Triton kernel in virtual_slot.py, the module sgl-project#37511 added for exactly this class of fusion. It carries the DCP owner rule, so the widened-loc path is the same single launch. Triton truncates division toward zero where torch floors it, so a negative loc is tested explicitly rather than relying on the division to reach the v2p sentinel row. Measured on B300, TP2 DCP2, cutedsl_mla, unified vs static decode throughput over 6 shape/batch points: mean gap -1.70% -> -0.23%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Two independent costs in the unified memory pool's hot path.
The read table's cuda-graph grid was sized by
max_context_len.build_kv_read_tablelaunched(bs, cdiv(max_pages, BLOCK))blocks. The eager path passes the batch's live maximum, but the captured path passesmax_context_len— and a cuda-graph capture bakes the grid. Every decode replay ran 4096 column blocks per row for sequences 8k long: 99.2% of the 262144 blocks loaded two scalars, failed thecols < n_pagesmask, and exited.The tell is that the cost did not vary with the sequence length — 158.5us at seq=1024 and 160.2us at seq=131072, one launch per step. It was pure block-scheduling overhead, and it grows with
cuda_graph_max_bs: 631us per step at bs=256.The allocator's free path issued five kernels where one suffices.
_free_lazy's v2p gather and both tombstone scatters are one lane's work per page, so they collapse into a single kernel — the inverse of thealloc_bind_inplacethe alloc fast path already uses.Modifications
build_kv_read_table's grid comes frombsalone; each program strides over its columns with the loop bound read from the device-sideseq_lens. The grid stays static so a capture bakes it, and the work tracks the real lengths at replay.BLOCK512 / 8 warps / 1024 blocks is the joint optimum over bs 1..256 × seq 1k..128k, flat within ~10%._free_lazyandbindinMultiEndedAllocatoruse two new fused Triton kernels (free_unbind_inplace,bind_inplace), each with a pure-torch CPU path because the allocator's unit tests run on CPU.torch.cats stay:_free_phys_pagesis a_CapacityFieldwhose contract is rebind-never-mutate, and the rebind is what invalidates theavailable_sizememos.test_free_paths_actually_use_index_fillasserted the free path's source containsindex_fill_. The fusion writes the tombstones inside the kernel instead, which serves that file's real invariant — the-1never crosses the bus — strictly better. It now checks for any of the sanctioned no-sync forms.virtual_slot.pyhad no test in the tree at all, and on CPU both launchers take their reference path, so the kernel doing the tombstoning was never executed by the suite. Added CUDA-gated coverage over 20 randomized bind/free rounds.Accuracy Tests
Kimi-Linear-48B TP2 +
--enable-unified-memory, GSM8K over 400: 0.9125 on both sides.Treat that as a sanity check, not the argument: the same unchanged server returns 0.9175 / 0.9175 / 0.9125 across three runs, so 400-example GSM8K on this box carries a ±2 question band.
The correctness argument is the kernel-level equality. The page table is verified against an independent CPU model over 30 randomized cases — page_size 1..4, mixed
seq_lens,-1holes inreq_to_token, tombstoned v2p rows, and a non-zerooutso the prefix-only contract has to leave the tail intact — plus both degenerate shapes. A captured graph replays bit-identically to a fresh launch asseq_lensmoves over 1 / 64 / 999 / 4096, which is the property the static grid exists for.The fused allocator kernels are checked against the torch implementation over 20 randomized page sets (both tables plus the returned physical pages), empty input, and CPU/CUDA agreement. Dropping
tl.store(p2v_ptr + p, -1)from the kernel turns two of the new tests red.test/registered/unit/mem_cache/: 2128 passed.Speed Tests and Profiling
Read-table grid, H100, under graph replay:
Allocator fusion, Kimi-Linear TP2 + unified, bs=64 / 8k in / 10 decode steps:
End to end, Kimi-Linear TP2, bs=64 / 8k input / 64 output, three runs a side on the same idle GPU pair:
That is 172us per decode step against the 156us the kernel saves — the same number within run-to-run spread. A fresh decode profile confirms it:
build_kv_read_table_kernel1606us → 47us over the same 10 launches, 1.18% → 0.03% of decode kernel time, with every large kernel (moe, all_reduce, MLA attention) inside noise.Both sides ran sequentially on the same idle GPU pair. That is not a formality on a shared box: a TP pool is sized from the minimum free memory across ranks, so a side-by-side run on two GPU sets measures whoever else is resident.
Kernel timings were taken under
torch.cuda.CUDAGraphreplay. A plain Python launch loop floors every Triton launch at ~16us regardless of the work, which made a 60-config sweep read as uniformly optimal.Checklist
Not in this PR
The
outbuffer these kernels fill is stillmax_bs x max_context_lenint32 — 1.0 GiB at 1M context — for the same reason the grid was. That is #37512.CI States
Latest PR Test (Base): ❌ Run #33682076564
Latest PR Test (Extra): ❌ Run #33682076343
Latest PR Test (AMD ROCm 7.2): ❌ Run #33682076492
CI States
Latest PR Test (Base): 🚫 Run #33697177974
Latest PR Test (Extra): 🚫 Run #33697177892
Latest PR Test (AMD ROCm 7.2): 🚫 Run #33697178027