Repository navigation
perf(cache): make the dense KV tail trim a logical rewind instead of a buffer rewrite - #1255
Merged
Merged
Conversation
…a buffer rewrite Every speculative round ends by rolling the KV cache back to the accepted prefix, and KVCache::trim re-sliced the entire live window to do it: an O(context) copy to discard a few tail tokens, plus a second O(context) concatenate in the next update_* because the re-slice threw away the step-aligned capacity and forced a regrow. Fp16 and Int8 trims now move only the offset, exactly like RotatingKVCache::trim always has: the buffers (scale sidecars included) keep their allocation, every reader already slices to buffer_idx() rather than trusting the physical length, and the next slice_update overwrites the abandoned slots. The update_fp16/update_int8 regrow guard changes from 'prev % step != 0' to 'prev != buffer len' so a retained capacity is normalized to the live prefix before extending. Turbo modes keep the physical trim: their packed sidecars feed fused kernels and fold/dequant helpers that have not been audited for a stale tail, and the speculative rollbacks this serves run on Fp16/Int8 caches. Turbo4Delegated and pool-backed paths are untouched. New test pins the contract: a trimmed-then-overwritten cache fetches a window byte-identical to a reference that never saw the rejected block, in both Fp16 and Int8. Refs #1209
6 tasks
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
KVCache::trimre-sliced the entire live window to discard a few rejected tail tokens, so every speculative rollback paid an O(context) copy, and then a second one: the re-slice threw away the step-aligned capacity, forcing the nextupdate_*through its regrow path (another full-buffer concatenate). The cost grew with the conversation while the work it undoes stayed constant at a few tokens, which is exactly what #1209 filed.Fp16 and Int8 trims are now a logical rewind: only
offsetmoves, the buffers (scale sidecars included) keep their allocation, and the nextslice_updateoverwrites the abandoned slots. This is the contractRotatingKVCache::trimhas always had ("it moves no data at all, it only rewindsoffsetandidxand lets the next append overwrite"), applied to the dense cache. The regrow guard inupdate_fp16/update_int8changes fromprev % step != 0toprev != buffer length, so a retained capacity is normalized to the live prefix on the rare genuine growth.Related issues
Closes #1209.
Type of change
perfWhy every reader stays correct
Capacity larger than the live window is already the normal state of these buffers between growths:
update_*writes into step-aligned pre-allocated buffers and every reader derives the window from the offsets, not from the physical length.update_and_fetchandkeys_viewslice tobuffer_idx();trim_front_keep_sinkslices[keep, live_len)computed from offsets;gather_within_tailworks inbuffer_idx()coordinates; the detached-handletrim_tobounds byoffset. What the logical trim adds is stale content (instead of zeros) beyond the live window, and nothing reads past it.The modes that do not share this property keep the physical trim, with the reason recorded in the code:
gather_positions
The acceptance criteria ask for the same treatment or a note. Note: a compaction has to write its survivors, so the rewrite cannot be avoided outright; what can be avoided is its scope, and #1214's
gather_within_tailalready did that for the production tree rollback (it writes O(block) through the sameslice_updatethe append path uses and keeps the capacity).gather_positionshas no production callers left, only tests.Measurements
Apple M5 Max 128 GB, macOS 26.6.1, offline CLI, temperature 0, ABBA per context, warm-up discarded, 6 s cooldowns. Arms:
mainat9e2c6675vs this branch.Rollback cost against context length (acceptance criterion 1), measured as the A/B it costs end to end. Pairing:
qwen2.5-7b-instruct-4bit+qwen2.5-0.5b-4bitclassic speculative (--num-draft-tokens 3), a dense 28-layer full-attention target, chosen because every layer's KV pays the rewrite. 300 new tokens on ~1k/8k/24k-token prompts:The saved term scales linearly with context, which is the O(context)-per-round copy this PR removes. The issue's arithmetic estimated sub-millisecond per round from one buffer's bytes; the measured cost is larger because it is paid per layer and twice (trim, then regrow).
Byte identity (acceptance criterion 4): generated text diffed at all three contexts, before vs after: identical. Also identical on both MTP pairings below.
MTP pairings, short context (regression check on the paths #1185/#1204 measure):
Test plan
cargo fmt --checkcargo clippy --workspace --all-targets --features metal,accelerate -- -D warningscargo test --release -p mlxcel-core --features metal,accelerate— failure set unchanged frommainon this machine (4 pre-existing numeric-tolerance reds, re-verified on unmodifiedmain)kv_cache_trim_then_append_matches_untrimmed_reference: a trimmed-then-overwritten cache fetches a window byte-identical to a reference that never saw the rejected block, in Fp16 and Int8test_kv_cache_trim_basicupdated to pin the new contract (offset moves, capacity stays, fetched window is offset-sized)