Repository navigation
feat: back paged scheduler sequences with the shared KV block pool - #167
Merged
Merged
Conversation
Sub-step (a) of #121: make the live batch scheduler's paged decode backend actually use the PagedBlockPool that #118-120 built and #152 proved through a real model. Path A — the pool rides inside the KVCache, so the LanguageModel trait and forward signature stay unchanged. CachePool refactor (behaviour-preserving): `paged_pool` and `SequenceCacheSet.paged` become `Rc<RefCell<…>>` so a pool-backed sequence shares one authoritative block table across its per-layer caches. Every access site (allocate, detach/adopt/park, sync, stats, restore, accessors) now scopes its borrows tightly; the request-boundary pool/state borrows never nest with the forward's `update_and_fetch` borrows. Live wiring: `CachePool::allocate` builds pool-backed caches (`KVCache::new_paged`) for paged sequences whose model uses the dense external KVCache slice (qwen3/llama3) under an Fp16 layout; model-owned families (gemma3/llama4/qwen3_5) and Turbo4 layouts keep their existing dense placeholders. The qwen3/llama3 batched-decode dispatch skips the native dense-pointer kernel when any cache `is_paged_backed()` and falls through to the per-sequence `update_and_fetch` loop, whose pool intercept writes the new K/V (`write_prefill`) and gathers the visible window (`gather_visible`). `update` gained the same intercept so the single-sequence fused causal prefill path (llama3) also writes to the pool. `sync_paged_state_with_dense` skips pool-backed sequences (the pool is authoritative), and `trim`/`trim_front` are dense-side no-ops for them. Hybrid-SSM carve-out: confirmed `effective_decode_storage_backend` already excludes recurrent models — they keep `supports_batching() == false` and the `supports_paged_decode_backend()` default `false`, either of which forces the dense backend. Added a test pinning the combined SSM profile across batch sizes. Verification: new `tests/paged_scheduler_parity.rs` (`#[ignore]`, qwen3-0.6b-4bit) drives the scheduler's paged allocation path (single-sequence and batched B=2) and asserts identical greedy tokens to the dense backend. Drive-by: fixed a pre-existing `clippy::identity_op` in `utils.rs` (`0 + 2`).
…n T=1 batched decode The uniform-batch fast path collapsed fast_rope_batched to a single fast_rope over the whole [B, n_heads, T, D] tensor when all offsets matched. In batched decode (T==1) the Q/K/V arrive transposed from [B, T, n_heads, D], where swapping the size-1 sequence dim leaves stride[batch] == stride[sequence]; MLX's fast::rope Metal kernel then confuses the batch index for an intra-sequence position and rotates batch element b to position offset+b instead of offset. This silently corrupted RoPE for every batch row past the first even when sequences shared the same offset -- a latent bug affecting dense batched decode (the function's existing callers), surfaced by the paged B=2 same-prompt parity test. Drop the uniform fast path; always slice to [1, n_heads, T, D] (B==1 cannot alias) and concatenate. Adds test_fast_rope_batched_non_contiguous_t1_symmetric.
…ing is Fp16-only) The pool-backing gate keys on paged_layout.cache_mode == Fp16, but the scheduler built Int8 (and Turbo3Asym) paged layouts via PagedKvLayout::uniform, which stamps cache_mode = Fp16, so those sequences were pool-backed and stored as raw Fp16 -- correct output but silently losing the Int8 memory saving. Build the non-Turbo4 paged layout with uniform_with_mode(effective_mode, 0) so the layout carries the real cache mode; the gate then pool-backs only genuine Fp16 and Int8/Turbo3 keep their existing dense path. Pool-native quantized storage is a later phase.
This was referenced Jun 6, 2026
be-free-snowboy
pushed a commit
to freestylejs/snowbrain-mlxcel
that referenced
this pull request
Oct 27, 2026
…ablup#167) * feat: back paged scheduler sequences with the shared KV block pool Sub-step (a) of lablup#121: make the live batch scheduler's paged decode backend actually use the PagedBlockPool that lablup#118-120 built and lablup#152 proved through a real model. Path A — the pool rides inside the KVCache, so the LanguageModel trait and forward signature stay unchanged. CachePool refactor (behaviour-preserving): `paged_pool` and `SequenceCacheSet.paged` become `Rc<RefCell<…>>` so a pool-backed sequence shares one authoritative block table across its per-layer caches. Every access site (allocate, detach/adopt/park, sync, stats, restore, accessors) now scopes its borrows tightly; the request-boundary pool/state borrows never nest with the forward's `update_and_fetch` borrows. Live wiring: `CachePool::allocate` builds pool-backed caches (`KVCache::new_paged`) for paged sequences whose model uses the dense external KVCache slice (qwen3/llama3) under an Fp16 layout; model-owned families (gemma3/llama4/qwen3_5) and Turbo4 layouts keep their existing dense placeholders. The qwen3/llama3 batched-decode dispatch skips the native dense-pointer kernel when any cache `is_paged_backed()` and falls through to the per-sequence `update_and_fetch` loop, whose pool intercept writes the new K/V (`write_prefill`) and gathers the visible window (`gather_visible`). `update` gained the same intercept so the single-sequence fused causal prefill path (llama3) also writes to the pool. `sync_paged_state_with_dense` skips pool-backed sequences (the pool is authoritative), and `trim`/`trim_front` are dense-side no-ops for them. Hybrid-SSM carve-out: confirmed `effective_decode_storage_backend` already excludes recurrent models — they keep `supports_batching() == false` and the `supports_paged_decode_backend()` default `false`, either of which forces the dense backend. Added a test pinning the combined SSM profile across batch sizes. Verification: new `tests/paged_scheduler_parity.rs` (`#[ignore]`, qwen3-0.6b-4bit) drives the scheduler's paged allocation path (single-sequence and batched B=2) and asserts identical greedy tokens to the dense backend. Drive-by: fixed a pre-existing `clippy::identity_op` in `utils.rs` (`0 + 2`). * fix: per-row RoPE in fast_rope_batched to avoid MLX stride aliasing on T=1 batched decode The uniform-batch fast path collapsed fast_rope_batched to a single fast_rope over the whole [B, n_heads, T, D] tensor when all offsets matched. In batched decode (T==1) the Q/K/V arrive transposed from [B, T, n_heads, D], where swapping the size-1 sequence dim leaves stride[batch] == stride[sequence]; MLX's fast::rope Metal kernel then confuses the batch index for an intra-sequence position and rotates batch element b to position offset+b instead of offset. This silently corrupted RoPE for every batch row past the first even when sequences shared the same offset -- a latent bug affecting dense batched decode (the function's existing callers), surfaced by the paged B=2 same-prompt parity test. Drop the uniform fast path; always slice to [1, n_heads, T, D] (B==1 cannot alias) and concatenate. Adds test_fast_rope_batched_non_contiguous_t1_symmetric. * fix: keep Int8/Turbo3 paged sequences on the dense KV path (pool-backing is Fp16-only) The pool-backing gate keys on paged_layout.cache_mode == Fp16, but the scheduler built Int8 (and Turbo3Asym) paged layouts via PagedKvLayout::uniform, which stamps cache_mode = Fp16, so those sequences were pool-backed and stored as raw Fp16 -- correct output but silently losing the Int8 memory saving. Build the non-Turbo4 paged layout with uniform_with_mode(effective_mode, 0) so the layout carries the real cache mode; the gate then pool-backs only genuine Fp16 and Int8/Turbo3 keep their existing dense path. Pool-native quantized storage is a later phase.
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.
Summary
Sub-step (a) of #121 (epic #116): wire the live batch scheduler's paged decode backend onto the
PagedBlockPoolthat #118–120 built and #152 proved through a real model. Path A — the pool rides inside theKVCache, so theLanguageModeltrait andforwardsignature stay unchanged.This is one of three sub-steps and does not close #121 — the radix adopt/donate +
try_adopt_cached_prefixguard removal lands in sub-step (b), and the multi-model RMS sweep in sub-step (c).What changed
Phase 1 —
CachePoolRc refactor (behaviour-preserving)CachePool.paged_pool→Option<Rc<RefCell<PagedBlockPool>>>andSequenceCacheSet.paged→Option<Rc<RefCell<PagedSequenceState>>>, so a pool-backed sequence shares ONE authoritative block table across its per-layer caches (each cache'sPagedBackingholds a cheapRcclone of the same handles).allocate*,detach_paged/adopt_paged/release_detached_paged/park_*(paged_detach.rs),sync_paged_state_with_*,paged_stats/stats_for_sequences,restore_paged_state,ensure_paged_pool*,release, and thepaged_pool_ref/paged_pool_mut/get_paged_state/paged_stateaccessors (now hand outRef/RefMutguards) — with tightly-scoped borrows. Request-boundary pool/state borrows never nest with the forward'supdate_and_fetchborrows;adopt_pageddrops every pool borrow before theprepare_sequence_statecallback.Ref/RefMuttype change).Phase 2 — live paged wiring
CachePool::allocatebuilds pool-backed caches (KVCache::new_paged) for paged sequences whose model's natural backend is the dense external KVCache slice (qwen3/llama3) under an Fp16 layout. Model-owned families (gemma3/llama4/qwen3_5, which drive their ownModelOwnedSequenceStateand ignore the caller's caches) and Turbo4 layouts keep their existing dense placeholders — byte-identical to before.KVCache::is_paged_backed(). The qwen3/llama3 batched-decode dispatch returnsNonefrom its native-paged-kernel selection when any cache is pool-backed, falling through to the per-sequenceupdate_and_fetchloop — the feat: transparent pool-backed KVCache + real-model paged parity test #152-validated pool path (write_prefill+gather_visible). The gemma3/llama4 shared dispatch helpers are untouched because those models never receive pool-backed caches.KVCache::updategained the same pool intercept asupdate_and_fetch(factored into a sharedwrite_paged), so llama3's single-sequence fused causal prefill path — which callsupdatewithout a fetch — also writes to the pool. qwen3's single-sequence prefill already routes throughupdate_and_fetch.sync_paged_state_with_denseskips pool-backed sequences (the pool is authoritative —write_prefilladvancesstate.lenin lockstep with the cache offset).trim/trim_frontare dense-side no-ops for pool-backed caches (they would otherwise desync from /unwrapthe absent dense buffers; paged trimming is a pool-API concern).effective_decode_storage_backendgates onsupports_paged_decode_backend()(trait defaultfalse) andsupports_batching()(which mamba/mamba2/jamba/nemotron_h/nemotron_nas/recurrent_gemma/qwen3_next/rwkv7 override tofalse); either flag forces the dense backend. Added a test pinning the combined SSM profile across batch sizes.Phase 3 — verification
tests/paged_scheduler_parity.rs(#[ignore], skip-if-absent) drives the scheduler's paged allocation path viaCachePool::allocate_with_layout(the exact APIBatchScheduler::allocate_sequence_statecalls for the paged backend) and asserts identical greedy tokens to the dense backend, for both the single-sequence path and a batchedB=2decode (which exercises theis_paged_backed()decode guard).BatchScheduler::newispub(crate), so thisCachePool-driven harness is the faithful constructible driver from an integration test.Drive-by
clippy::identity_opinsrc/lib/mlxcel-core/src/utils.rs(0 + 2→2) that cold clippy fails on (from perf: support variable-length prompts in B>1 batched MTP bursts #162).Test plan
cargo test --lib -p mlxcel-core --features metal,accelerate cache::— 370 pass (incl. 2 new sync tests)cache::paged_detach/cache::detachregression suites — unchanged, passcargo test --lib --features metal,accelerate hybrid_ssm_workers_never_use_paged_decode_backend— passcargo clippy -p mlxcel-core --tests --features metal,accelerate -- -D warnings(cold) — cleancargo clippy --lib --features metal,accelerate -- -D warnings— cleancargo check --test paged_scheduler_parity --features metal,accelerate— compilescargo fmt --check— cleanReal-model parity test (loads qwen3-0.6b-4bit on the GPU; run by the orchestrator):
Part of #121
Follow-up fixes (after the real-model parity run)
The orchestrator's
paged_scheduler_parityreal-model run surfaced two issues, now fixed on this branch (both crates cold-clippy clean; core regression 832 pass; scheduler/batch 173 pass; parity now passes for BOTH single-sequence and batched B=2 — qwen3-0.6b-4bit, 16 greedy tokens identical to dense):fast_rope_batchedstride aliasing — pre-existing, affects dense batched decode tooBatched B=2 parity initially diverged (row 1 looped). Root cause:
fast_rope_batched's uniform-batch fast path collapsed to a singlefast_ropeover the full[B, n_heads, T, D]tensor. In T==1 batched decode the Q/K/V arrive transposed from[B, T, n_heads, D], where swapping the size-1 sequence dim leavesstride[batch] == stride[sequence]; MLX'sfast::ropekernel then rotates batch elementbto positionoffset + binstead ofoffset. This silently corrupted RoPE for batch rows past the first even with identical offsets — a latent bug in dense batched decode (the function's existing callers) that the paged same-prompt test exposed. Fix: drop the uniform fast path, always go per-row (B==1cannot alias) + regression testtest_fast_rope_batched_non_contiguous_t1_symmetric.Int8/Turbo3 paged → dense (pool-backing is Fp16-only)
The Fp16 pool-backing gate keyed on the layout's
cache_mode, but the scheduler built Int8/Turbo3 paged layouts withcache_mode = Fp16, so those sequences were pool-backed and stored as Fp16 (correct output, lost Int8 memory saving). Fix: the scheduler now builds the non-Turbo4 paged layout with the realeffective_mode, so the gate pool-backs only genuine Fp16 and Int8/Turbo3 keep their dense path. Pool-native quantized storage is a later phase.