[MLX] Add off-graph KV cache ring runtime - #21532
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21532
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 94187b1 with merge base e8feb9e ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
| // (the newest key belongs to the last query), so query i spans keys | ||
| // j - i <= S - T -- the same bound MLX's "causal" applies -- and the window | ||
| // adds the lower bound j - i > S - T - window. | ||
| inline array window_causal_mask(int T, int S, int window, StreamOrDevice s) { |
There was a problem hiding this comment.
Isn't there a field for mask on attendsepc?
Should the mask be constructed in the cache and handed back?
There was a problem hiding this comment.
Mlx doesnt have a window mode but for backends that takes window like flash attention it could just pass window parameter instead of materializing the mask
There was a problem hiding this comment.
I'm a bit confused. On AttendSpec, you have std::optional mask; // Explicit only
And you just overwrite that value here?
Why can the logic in window_causal_mask not live in MLXSequanceCache.h and attach a mask there?
There was a problem hiding this comment.
Actually, you're right. I was treating AttendSpec as it is a cross-backend contract thinking that it will declare the window and each backend can render it but each backend will have its own cache class anyway. I am fixing this.
| K, | ||
| V, | ||
| AttendSpec::Mask::Explicit, | ||
| window_causal_mask(T, S, window_[l], s)}; |
There was a problem hiding this comment.
Creating this mask is not free, are you creating it once per layer IIUC
Per decode step, we only need to plan once per policy per execute step. Is that how your code is set up?
Much of this will become more apparent when we test in real model.
There was a problem hiding this comment.
Decode currently doesn't need a mask. But no, we don't plan once per policy. Policies aren't duplicated, but update_and_fetch is per layer, so we call plan() for each layer per step. I can memoize the plan and mask per policy index, invalidated when (position, T) changes.
There was a problem hiding this comment.
Why doesn't decode need a mask? Isn't the mask how you tell the ring what to attend to?
There was a problem hiding this comment.
The selection happens in the read, the planner returns the retained window as physical runs, and we read exactly those. For example, W=4, max_write=1 (ring of 4 slots), decoding at position 10. RingPolicy::plan(10, 1) gives rstart = 7, span 4, which wraps into two runs, {start:3, len:1} and {start:0, len:3}. We read those and concatenate, resulting positions 7, 8, 9, 10 in logical order.
We need a mask during prefill: a step with T > 1 reads the union of its queries' visibilities.
There was a problem hiding this comment.
Even in prefill, I'm not sure we'll want to generate a mask per layer.
Stamping to unblock, but consider doing plan once per execute per policy, not per layer.
I guess the next PR will be E2E enablement, and we can start comparing perf.
There was a problem hiding this comment.
Thanks! Do you think memoization would make sense for this problem? Keeping the plan (and mask) per policy index and invalidating when (position, T)?
| K, | ||
| V, | ||
| AttendSpec::Mask::Explicit, | ||
| window_causal_mask(T, S, window_[l], s)}; |
There was a problem hiding this comment.
Even in prefill, I'm not sure we'll want to generate a mask per layer.
Stamping to unblock, but consider doing plan once per execute per policy, not per layer.
I guess the next PR will be E2E enablement, and we can start comparing perf.
Summary
Adds ring (sliding-window) layers to the MLX KV cache, so a model can mix them with flat layers per layer. The window is expressed declaratively: AttendSpec gains a window on Causal (the mask_mod axis the header reserved), so the cache states the semantic and the handler materializes whatever the backend can't do natively. MLX SDPA has only ""/"causal"/"array" — no sliding-window mode — so the handler builds a banded bool mask, but only when the window is narrower than the span. A backend that supports windows natively would just forward the integer.
Files
handler uses it only when window < S, else plain "causal".
Testing
New: the mask is a band (checked against a hand-written 3×5 matrix, plus a window covering the span degenerating to causal); a ring decode past its window evicts the oldest and needs no mask; a multi-token step declares its window while a flat layer declares none; and a step whose runs wrap the ring is scattered and rejoined in logical order; the physical layout is out of order by then, so this pins the wrap handling.
cmake --preset mlx-release -DEXECUTORCH_BUILD_TESTS=ON
cmake --build cmake-out --target mlx_sequence_cache_test
ctest --test-dir cmake-out -R mlx_sequence_cache --output-on-failure