Optimize discarded global random state updates#3857
Open
MicroMilo wants to merge 1 commit into
Open
Conversation
MicroMilo
marked this pull request as ready for review
July 17, 2026 03:49
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.
Proposed changes
Fixes #2711.
MLX's global random state advances through a chain of
splitoperations. When random values are created but discarded, their sample branches are dead, but the state branch remains live. Evaluating a later random value therefore dispatches every intermediateRandomBitskernel.This PR:
RandomAdvanceprimitive for CPU, Metal, and CUDA that applies repeated split-state transitions in one dispatch;eval's existing degree walk;vmap, andcompiletests.The rewrite does not reduce the number of Threefry state transitions. It reduces graph and dispatch overhead by executing those transitions inside one primitive.
Correctness
The new primitive was compared bit-for-bit with repeated
mx.random.split(key)[0]across multiple seeds and step counts. The integrated rewrite also checks that:Local validation on an Apple M5:
python/tests/test_random.py: 18 tests passed;test_eval.py,test_vmap.py,test_compile.py, andtest_export_import.py: 117 tests passed;pre-commit run --all-filespassed.The CUDA implementation is included but was not compiled locally. Its build and runtime behavior still needs CI or reviewer validation in a CUDA-capable environment; this limitation does not affect the CPU/Metal evidence above.
Performance
An integrated 61-repetition alternating A/B run on Apple M5 measured:
Each graph was constructed before timing. The timed region starts after
mx.synchronize(), then runsmx.eval(...)followed by another synchronization. It therefore includes eval-time graph traversal, rewrite detection, scheduling, dispatch, and execution, but excludes Python graph construction. Baseline/candidate order alternated, and the report retained every raw paired sample.A fresh separate-build smoke comparison of this branch against
mainreproduced approximately 1.25x at 16 calls, 3.29x at 1000 calls, and 3.44-3.71x at 5000 calls.Fully used RNG chains showed no significant regression in the alternating A/B experiment. A 500-node non-random graph showed a small ~1.9% detection-overhead signal in that experiment, while smaller non-random graphs did not. A fresh separate-build smoke run did not show a consistent slowdown, but the cost of performing this detection in
evalremains an explicit review point.Design points for review
eval's existing degree walk because eligibility depends on reachability from the current eval roots. Would a separate graph-rewrite pass be a better architectural fit, despite requiring the same root-relative liveness information?random::advancein the C++ API and binds it as the private Python hook_advancefor direct equivalence and transform tests. Would you prefer keeping the C++ helper internal and removing the Python hook once the integrated tests are sufficient?Checklist
pre-commit run --all-filesto format my code / installed pre-commit prior to committing changes