metal: add optional polled release fence for the TP gate (~23% decode gain with --tensor-parallel) - #743
Open
ryan5rdx wants to merge 1 commit into
Open
metal: add optional polled release fence for the TP gate (~23% decode gain with --tensor-parallel)#743ryan5rdx wants to merge 1 commit into
--tensor-parallel)#743ryan5rdx wants to merge 1 commit into
Conversation
--tensor-parallel)--tensor-parallel)
Author
|
decode/prefill appears to scale alright: ds4-bench/server/agent don't allow passing |
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.
This PR ports the fast cpu/gpu sync primitive in ml-explore/mlx#1773 to ds4. With this, and setting an already existing DS4 flag to increase the GPU duty cycle, TP decode performance roughly doubled in my testing.
I'm leaving this opt-in because it's still off by default in MLX/exo so there seems to be some hesitancy given it's largely undocumented, but works fine in my testing. I'm not a metal eng and all credit goes to prior art in ml-explore/mlx#1773, I've just had an LLM help port it and track down why the GPU was idle for so long during TP.
Speedup
All testing on 2x M3 Ultra. no MTP, 1M context:
coord:
worker:
Enabled the fast fence alone(via
DS4_METAL_FAST_SYNC=1) results in a decode gain of ~23%tg2048@0 16.56t/s -> 20.44t/s
However - by setting one other (existing)flag that drastically increase GPU util in TP we can achieve a net ~2x gain, because speedup from the fast fence is magnified:
With
DS4_TP_NO_KEEPALIVE=1andDS4_METAL_FAST_SYNC=1tg2048@0 goes from ~16.56t/s(baseline TP) to 33.12t/s <--------------------------------------!!
DS4_TP_NO_KEEPALIVE spins a dummy kernel on the GPU for power gating reasons, but in testing it drastically reduced GPU util because while there is some pause logic to stop it from spinning while real work is happening, we don't(can't?) interrupt the spin when work arrives, so we can end up with a lot of "spin" time per token. Disabling it completely resolves that. With it off - total system draw increases by ~10W during decode, imo the strongest signal that this is problematic.
With both
DS4_TP_NO_KEEPALIVE=1andDS4_METAL_FAST_SYNC=1- system draw goes from ~60W to 88W during decode. (Closing the gap we see vs prefill at ~115W, though I don't expect we'll reach that during decode because of the nature of the workload).With this - TP eclipses layer parallel distributed decode tg/s in my 2 node setup(even after enabling RDMA for layer parallel in #715)
cc @antirez - I'm making this all opt-in and not changing the default
DS4_TP_NO_KEEPALIVEbehaviour because looks like some extensive work has gone into adding it, so don't want to break an edge case I haven't run into.