[kernel] Wire MLA split-KV decode - #601
Conversation
8e145c0 to
c7b7564
Compare
| @@ -1204,14 +1217,45 @@ static void dispatch_mla_paged_attention( | |||
| }); | |||
|
|
|||
| auto dt = dtype_to_metal(q_nope.dtype()); | |||
| const bool pure_decode = total_q_tokens == num_seqs; | |||
| const int max_seq_len = max_num_blocks_per_seq * block_size; | |||
| const int mla_partition_size = mla_partition_size_for_context(max_seq_len); | |||
| const int max_num_partitions = | |||
| mla_partition_size > 0 | |||
| ? (max_seq_len + mla_partition_size - 1) / mla_partition_size | |||
| : 1; | |||
| const int gate_grid = (num_heads / heads_per_tg) * total_q_tokens; | |||
| constexpr int kMlaMaxSinglePassGrid = 32; | |||
| const bool occupancy_limited = gate_grid < kMlaMaxSinglePassGrid; | |||
There was a problem hiding this comment.
This policy is too hard-coded for shared dispatch. Before landing, prove 32K+ split Metal beats the default MLX path, then put the thresholds behind a small device-aware split-plan helper.
| @pytest.mark.parametrize( | ||
| ("ctx_len", "num_seqs", "num_heads", "heads_per_tg"), | ||
| [ | ||
| (32768, 1, 2, 2), | ||
| (32768, 2, 1, 1), | ||
| (81920, 1, 1, 1), | ||
| ], | ||
| ) | ||
| def test_decode_split_kv_long_context_matches_reference( | ||
| ctx_len: int, | ||
| num_seqs: int, | ||
| num_heads: int, | ||
| heads_per_tg: int, | ||
| ) -> None: | ||
| """Exercise the MLA split-KV path, including multi-partition reduce.""" | ||
| block_size = 32 | ||
| dtype = mx.float16 | ||
| ( | ||
| q_nope, | ||
| q_pe, | ||
| latent_cache, | ||
| block_tables, | ||
| context_lens, | ||
| cu_seqlens_q, | ||
| block_tables_np, | ||
| ) = _make_inputs( | ||
| num_seqs=num_seqs, | ||
| num_heads=num_heads, | ||
| ctx_len=ctx_len, | ||
| block_size=block_size, | ||
| dtype=dtype, | ||
| seed=17, | ||
| ) | ||
|
|
||
| out = metal_mla_paged_attention( | ||
| q_nope=q_nope, | ||
| q_pe=q_pe, | ||
| latent_cache=latent_cache, | ||
| block_tables=block_tables, | ||
| context_lens=context_lens, | ||
| cu_seqlens_q=cu_seqlens_q, | ||
| scale=0.125, | ||
| heads_per_tg=heads_per_tg, | ||
| ) | ||
|
|
||
| expected = _expected_output( | ||
| q_nope, | ||
| q_pe, | ||
| latent_cache, | ||
| block_tables_np, | ||
| ctx_lens=[ctx_len] * num_seqs, | ||
| scale=0.125, | ||
| ) | ||
| rtol, atol = _tolerance(dtype) | ||
| diff = mx.abs(out.astype(mx.float32) - expected.astype(mx.float32)) | ||
| max_abs = mx.max(diff).item() | ||
| assert mx.allclose( | ||
| out.astype(mx.float32), expected.astype(mx.float32), rtol=rtol, atol=atol | ||
| ).item(), f"split-KV MLA mismatch: max_abs_diff={max_abs:.5f}" |
There was a problem hiding this comment.
These parity tests can still pass if split dispatch never runs. Assert the selected split plan, cover bf16/block16, and add one mixed long/short batch.
| constexpr int kMlaMaxNumPartitions = 8; | ||
| constexpr int64_t kMlaSplitScratchByteLimit = 512 * 1024 * sizeof(float); | ||
| const int64_t split_scratch_bytes = | ||
| static_cast<int64_t>(total_q_tokens) * num_heads * | ||
| ((max_num_partitions - 1) * kv_lora_rank * | ||
| static_cast<int>(q_nope.itemsize()) + | ||
| max_num_partitions * sizeof(float)); | ||
| const bool scratch_budget_ok = | ||
| split_scratch_bytes <= kMlaSplitScratchByteLimit; | ||
| const bool partition = | ||
| pure_decode && occupancy_limited && mla_partition_size > 0 | ||
| && max_num_partitions >= 2 | ||
| && max_num_partitions <= kMlaMaxNumPartitions && scratch_budget_ok; |
There was a problem hiding this comment.
This scratch guard looks unreachable under the existing bounds. Delete the dead calculation and keep the occupancy and partition-count gates.
|
docs/configuration.md:15 :: This is stale now. The Metal MLA path is no longer only single-pass; document the 32K+ split/reduce path and its eligibility. |
LxYuan0420
left a comment
There was a problem hiding this comment.
Requesting changes for now. After the policy/test cleanup, please share rerunnable 32K+ proof from the rebased head: MLX vs single-pass Metal vs split Metal, with device, dtype, heads, batch, context, and repeated latency numbers.
|
@rohash123 If M4 Max 128 GB would help you get that 32K+ proof, just prepare a bench script for me, happy to help. |
Signed-off-by: Rohan Arora <rohanarora@berkeley.edu>
Signed-off-by: Rohan Arora <rohanarora@berkeley.edu>
c7b7564 to
1f18cfd
Compare
LxYuan0420
left a comment
There was a problem hiding this comment.
Closing this for now and keeping #360 open.
The problem is real, but this branch is too large and still does not prove a win over default MLX. A restart should preserve the existing one-pass Metal path and include rerunnable 32K+ MLX vs single-pass vs split evidence.
|
@rohash123 If you eventually need me to run your 32K+ benchmark, just let me know. Just curious, there are amazing MLX optimizations in youssofal/MTPLX , unsloth/ and jundot/omlx, could any of them help with MLA split-KV decode? |
Why this PR exists
This is an implementation for the Phase 3 item in #360: MLA-specific long-context partition/reduce decode.
#360 says the regular paged-attention split-KV work in #437 is useful reference material, but it did not cover MLA. This PR adds the MLA version.
In plain terms: when decoding one token from a very long prompt, MLA has to read a lot of cached context. Before this PR, one small Metal launch did that read mostly as one long scan. This can leave the GPU under-used at low concurrency. This PR lets MLA split that long scan into several independent pieces, run those pieces in parallel, and then combine the partial answers into the same final answer.
What changed
paged_mla_attentioncan now run over a slice of the context instead of always scanning the full context in one pass.paged_mla_attention_reduceMetal kernel combines those slices into the final output.LSE, log-sum-exp) so the result matches normal attention math instead of doing a separate softmax per slice.paged_ops.cppnow chooses the split path only for conservative cases:This keeps the existing MLA cache layout and runtime contract. It does not change scheduling, prefill, prefix cache behavior, or non-MLA attention.
Correctness
This PR adds targeted coverage to the existing MLA kernel test suite. The new split-KV cases compare partitioned MLA decode against the dense MLX reference and cover:
I also reran the existing MLA kernel and MLA paged-backend tests, so the old single-pass MLA path and wrapper routing coverage are still exercised.
Local validation on the clean PR branch:
Wrapper benchmark
#360 asks for performance claims that include wrapper overhead, not only isolated kernel time. I re-ran the comparison through
MLAPagedAttentionWrapper.__call__rather than calling the Metal kernel directly.This benchmark includes the wrapper work around the kernel: query projection, KV projection for the new token, RoPE, cache write, block-table/context metadata conversion, Metal attention call, unembed, and output projection.
Hardware: Apple M5 Pro, 20-core GPU, 24 GB, macOS 26.5.2.
Setup: synthetic absorbed-MLA decode wrapper, one decoded token, fp16,
kv_lora_rank=512,qk_rope_head_dim=64, block size 32. The benchmark compared the new automatic split path with the old single-pass path in the same Python process, using the same tensors, shuffled order, median latency, and explicitmx.synchronize(). For the "old single-pass" numbers, I temporarily disabled the split decision in the local benchmark checkout so everything except the dispatch choice stayed identical.The table reports the median across five shuffled trials. Each trial measured the median of 50 timed wrapper calls per mode after warmup.
The exact percentages still move around across runs on my local machine, especially in this synthetic harness. The robust signal is that the split path is faster across the long-context, low-concurrency cases I tested. The chart below shows absolute latency with trial-to-trial min/max whiskers instead of a single-run speedup chart.
Raw benchmark summary: https://gist.github.com/rohash123/4ecf29d05091ec9053dbfd0fba83ea18
Serving sanity check
I also ran one small real-server check through
vllm serve+vllm bench serveto make sure this path works outside the wrapper harness.This is intentionally a sanity check, not the main performance claim: longer serving runs at 32K+ context put too much memory pressure on my local Mac. The useful signal here is that an MLA model loads, allocates paged MLA KV cache, serves through the OpenAI-compatible endpoint, and completes generation successfully with this PR.
Server:
Environment:
Client:
Result:
Scope notes
MLX is still the default path for MLA. This PR only affects users who opt into the Metal MLA kernel with
VLLM_METAL_MLA_KERNEL=1.When that opt-in is enabled, the C++ side chooses between the existing one-shot Metal path and the new split path based on the request shape.
This PR does not change the MLA cache format, request scheduling, or upstream vLLM MLA behavior.
I also ran the wrapper-level grid from the MLA kernel RFC comment: heads
{16, 64, 128}, batch{1, 8, 32}, context{128, 2048, 8192}, and both fp16/bf16. This compares the MLX default path with the existing opt-in Metal MLA path (VLLM_METAL_MLA_KERNEL=1).Important: this grid stops at 8K context, while this PR's new split path only starts at 32K+ context. So these numbers do not measure the new split/reduce optimization. They are included as a broader default-readiness check for Metal MLA overall.
Result: Metal wins 24/54 cells overall, but not the full grid. This supports keeping MLX as the default.
Raw grid CSV: https://gist.githubusercontent.com/rohash123/4ecf29d05091ec9053dbfd0fba83ea18/raw/ea5953f827b1a86cbb3371429117a1949125aad1/mla_production_grid_wrapper.csv
Progresses #360.