Bind new_thread_unsafe_stream - #122
Open
CharlieTLe wants to merge 1 commit into
Open
Conversation
Streams are thread affine as of mlx 0.31.2: a stream's GPU command encoder is registered in a thread_local map, so evaluating on a stream from a thread other than the one that created it raises "There is no Stream(gpu, N) in current thread." mlx added `new_thread_unsafe_stream` for language bindings that do their own locking (ml-explore/mlx#3578), but mlx-c does not expose it, so those bindings have no way to reach it. mlx-swift currently carries a local C shim for this. Requires mlx 0.32.0, so the pin moves from v0.31.2. The existing bindings build and the examples pass unchanged against it.
4 tasks
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.
Summary
Expose mlx's
new_thread_unsafe_streamasmlx_stream_new_thread_unsafe.Streams became thread affine in mlx 0.31.2 (#3348, #3281): a stream's GPU command encoder is registered in a
thread_localmap, so evaluating on a stream from a thread other than the one that created it raisesThere is no Stream(gpu, N) in current thread.mlx added
new_thread_unsafe_streamin ml-explore/mlx#3578 specifically for this case:mlx-c does not bind it, so those bindings can't reach it. mlx-swift currently carries a local C shim to work around exactly this, which this PR would let it delete.
Before / after
examples/example-thread-stream.ccreates a stream on the main thread and evaluates on it from a worker thread. Withmlx_stream_new_device:With
mlx_stream_new_thread_unsafe:Note on the mlx pin
new_thread_unsafe_streamonly exists in mlx 0.32.0, soGIT_TAGmoves fromv0.31.2tov0.32.0. I kept that to the one-line bump rather than regenerating bindings for 0.32.0, on the assumption you'd rather do that as its own change — happy to drop the bump if you'd prefer to land this after a 0.32.0 regen.The existing bindings compile unchanged against 0.32.0, and
example,example-grad,example-closure,example-graphandexample-exportall pass.Naming
I went with
mlx_stream_new_thread_unsafeto sit alongsidemlx_stream_new_devicein themlx_stream_new*constructor family. Happy to rename tomlx_new_thread_unsafe_stream(closer to the C++ name) or anything else you prefer.Possible follow-up
new_thread_local_stream/stream_from_thread_local_stream/clear_streamsare also unbound. They need a new opaquemlx_thread_local_streamtype with its own private accessors, so I left them out to keep this focused — glad to do it in a separate PR if it's wanted.Checklist
clang-formatusing the repo's.clang-format