[Apple Silicon] Add custom Metal RoPE kernel with fused KV cache store - #22868
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive MLX-based KV cache system and model runner for Apple Silicon, enabling features like radix-cache prefix sharing and custom Metal RoPE kernels. The implementation includes a flat KV pool (MlxKVPool), contiguous request-local buffers (ContiguousKVCache), and attention patching to support batched decoding. However, several critical issues were identified: the custom Metal RoPE kernels do not correctly handle models with partial RoPE (where rope_dim < head_dim), leading to corrupted KV caches. Additionally, the write_token method in ContiguousKVCache lacks necessary allocation and bounds checks, which will cause runtime errors during the first token write. There is also an opportunity to optimize the batched decode logic by replacing a Python loop with a more efficient batched concatenation of pre-allocated buffers.
There was a problem hiding this comment.
Nice work! Since this is the first Metal kernel for SGLang, it's important to establish the right approach for integrating it. Future kernels will likely follow this pattern, so getting the foundation right is critical.
You may take a look at how https://github.com/vllm-project/vllm-metal/tree/880af64e95f75832649d55d8260ad823244fc8b0/vllm_metal/metal/kernels_v2 works.
alexnails
left a comment
There was a problem hiding this comment.
I will have more comments, just doing a quick review
4a4b2cf to
da5b2c7
Compare
|
@Jonahcb is also working on custom Metal kernels (paged attention). Maybe we can discuss the best way to integrate them. |
4088039 to
5b64f3d
Compare
830ab19 to
fe7bcac
Compare
|
@alexnails @yeahdongcn Updated the PR . |
| def is_available() -> bool: | ||
| """Return whether the Metal extension and metallib were loaded.""" | ||
| return _metal is not None and _IMPORT_ERROR is None | ||
|
|
||
|
|
||
| def _require_metal() -> Any: | ||
| if _metal is None: | ||
| raise ImportError( | ||
| "sgl_kernel._metal is not available. Build with " | ||
| "`TOOLCHAINS=metal python sgl-kernel/setup_metal.py build_ext --inplace`." | ||
| ) from _IMPORT_ERROR | ||
| return _metal |
There was a problem hiding this comment.
Should we still care about these?
There was a problem hiding this comment.
Keeping it now to facilitate fallback and validation
There was a problem hiding this comment.
I think _import_sgl_kernel_metal already provides sufficient protection at the call site, so we can remove these checks.
|
|
||
| # MPS (Apple Silicon) | ||
| SGLANG_USE_MLX = EnvBool(False) | ||
| SGLANG_DISABLE_CUSTOM_ROPE = EnvBool(False) |
There was a problem hiding this comment.
Based on the performance data, I didn't see a clear overall win (large batch size seems to be meaningless for mac) from the AOT kernels (though this is our first attempt at adding custom AOT kernels to the MLX backend). So I think we should introduce something like SGLANG_MLX_USE_CUSTOM_ROPE and keep the default value as false.
| # AOT custom Metal RoPE kernel state. When populated, the wrapper invokes | ||
| # `sgl_kernel.metal.rope_pool_fused` to rotate Q/K and scatter K/V into | ||
| # the shared pool for the new decode token. | ||
| rope_config: dict = field(default_factory=dict) | ||
| rope_base: float = 0.0 | ||
| kv_pool: Optional[Any] = None # MlxKVPool | ||
| new_token_slots: Optional[mx.array] = None # int32 [B], slot per request | ||
|
|
There was a problem hiding this comment.
Please consider introducing an explicit abstraction before adding more fields directly to the runner/context, for example:
@dataclass
class MlxAOTKernelContext:
rope: Optional[MlxAOTRoPEContext] = None
@dataclass
class MlxAOTRoPEContext:
config: dict
base: float
kv_pool: MlxKVPool
new_token_slots: Optional[mx.array]Then the model runner can have one helper such as _build_aot_kernel_context(...), and BatchedDecodeContext only needs a single aot field. That makes it clear which data belongs to optional AOT kernels and keeps the regular MLX decode path from accumulating kernel-specific kwargs.
There was a problem hiding this comment.
done, Claude is amazing.
…TCHGLU Register SGLANG_MLX_FUSE_SWITCHGLU in environ.py and consume it via envs.SGLANG_MLX_FUSE_SWITCHGLU.get() instead of raw os.environ.get, matching the convention used in PR sgl-project#26188 for SGLANG_MLX_FUSE_SWIGLU and the codebase pattern reinforced in PR sgl-project#22868. Drops the previously unused 'import os' from model_runner.py.
Signed-off-by: Xiaodong Ye <yeahdongcn@gmail.com>
|
Just pushed a new commit to make the MLX AOT kernel selection into a backend-level registry (we will have more AOT kernels in the future). @adityavaid feel free to drop or update. |
|
/tag-and-rerun-ci |
|
/rerun-failed-ci |
Path B's gate read SGLANG_MLX_FUSE_SWIGLU via raw os.environ.get. The repo convention is to register every SGLANG_* var in srt/environ.py and read it through envs.<NAME>.get(), matching SGLANG_USE_MLX. The sgl-project#22868 review flagged raw getenv usage (SGLANG_RPF_N) as a convention violation; apply the same fix here so the var is discoverable in the canonical list. - Declare SGLANG_MLX_FUSE_SWIGLU = EnvBool(False) next to SGLANG_USE_MLX. - Consume via envs.SGLANG_MLX_FUSE_SWIGLU.get() in MlxModelRunner and drop the now-unused import os. Behavior preserved: default off; EnvBool treats 1/true/yes/y as enabled.
sgl-project#22868) Signed-off-by: Xiaodong Ye <yeahdongcn@gmail.com> Co-authored-by: Xiaodong Ye <yeahdongcn@gmail.com>
sgl-project#22868) Signed-off-by: Xiaodong Ye <yeahdongcn@gmail.com> Co-authored-by: Xiaodong Ye <yeahdongcn@gmail.com>
Motivation
This PR adds the first AOT-compiled Metal kernel to SGLang's Apple Silicon backend: a fused NeoX RoPE + KV pool scatter that replaces two
mx.fast.ropecalls and the per-request KV-pool scatter in batched decode with a single C++ entry that emits one Metal command buffer.On Apple Silicon, each kernel dispatch incurs ~1-4ms of GPU idle time due to Metal command buffer overhead (profiled in #22114). Reducing dispatch count directly improves decode latency. This kernel consolidates the Q and K rotation into one dispatch, cutting the RoPE step from ~224us (2 × 112us) to ~127us — a 1.8x speedup for the RoPE operation.
Part of the Apple Device Support roadmap (#19137)
NOTE : this builds on #23449's scaffolding
Kernel logic
A single
.metallibproduced at build time by Apple'sxcrun metal+xcrun metallib(same pipeline MLX itself uses), exporting threespecialised kernels per dtype (f16, bf16, f32). All three are dispatched
into one Metal command buffer per batched-decode forward, per layer:
rope_qrope_k_poolk_pool[slot]v_to_poolv_pool[slot]Loaded once at import via
register_library(path). Per-shape pipelinesare specialised through Metal function constants so a single
.metallibserves every model configuration.
Optimisations over a naïve fused-RoPE kernel
( Inspired by MLX's own rope.metal )
Baseline : single 1-D-grid kernel that recovers
(token, head, dim)via div/mod and branches onis_q.Layered on top of that:
.metallibinstead of runtime JIT — Apple's full compiler-optimisation pass at build time.2.. 3D thread-grid (
pos.x = dim,pos.y = token,pos.z = head) — eliminates the div/mod hot path.(32, 1, 1)that uses 1.(HEAD_DIM,NUM_QO_HEADS,NUM_KV_HEADS,INV_DIM_LOG2_BASE,HEADS_PER_THREAD)— compiler folds constants per pipeline.metal::exp2+metal::fast::cos/sin) — no precomputed cos/sin cache.HEADS_PER_THREADfn-const,SGLANG_RPF_Nenv override). Default N=1 — kernel is launch-overhead-bound at our shapes; N>1 left for future hardware.mlx::core::Primitiveintegration — required for kernel writes to be visible to MLX's lazy graph and downstream ops.kv_pool[slot]in the same command buffer, eliminating the separatepool[slots] = …scatters that would otherwise be needed after RoPE.copy_shared_bufferreturns the input pool buffer as the output; the user-side handle is rebound, GPU memory is the same allocation.Accuracy Tests
( Adding Unit Tests )
E2E server test with
Qwen/Qwen3-0.6B:Custom RoPE kernel enabledBenchmarking and Profiling
Kernel microbenchmark on Apple M4 Pro (48GB), Qwen3-0.6B dimensions (32 tokens, 16 Q heads, 2 KV heads, head_dim=64)
Focused local profile timing:
nq=32,nk=8,hd=128)nq=16,nk=2,hd=64)Large-batch bf16 sweep:
Takeaway: this is not a universal speedup. The fused AOT path helps on larger
LLaMA-like shapes, especially larger bf16 batches, but regresses on Qwen-like
small-KV-head shapes. The PR keeps the implementation guarded and easy to A/B.
How to Run Tests
Checklist
Review Process
/tag-run-ci-label,/rerun-failed-ci,/tag-and-rerun-ciCI States
Latest PR Test (Base): ❌ Run #26619584932
Latest PR Test (Extra): ❌ Run #26619584836