Repository navigation
Python: erase a freed compiled function from every thread cache - #44
Merged
Merged
Conversation
PyCompiledFun keys its traces by the address of the Python callable. Each call records its trace in the cache of the calling thread, but the destructor erases the key only from the cache of the thread that called the function last. The new test calls the compiled function on a worker thread and then on the main thread, frees it on the main thread, and compiles the same callable again on the worker. It expects a new trace there. It fails until the destructor erases every thread cache. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The first CI run passed the new test without the fix. A trace is cached under the default stream of the calling thread, and the first op of a new thread makes that stream. The worker's first call recorded its trace before its stream existed, so no later call could match it, and the compile after the free traced again with or without the erase. The worker now makes its default stream first and calls the function twice; the test checks that the second call used the trace (one trace), which shows that a stale entry would be found. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
PyCompiledFun keys its traces by the address of the Python callable. call_impl stored the cache of the calling thread, so the destructor erased the key only from the cache of the thread that called the function last. Traces from other threads stayed after the free: they kept their arrays alive, and a later compile of a callable at the same address replayed them. The destructor now erases through the cache registry (detail::compile_erase(fun_id)) before it releases the callable, and PyCompiledFun no longer keeps a cache. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
davidtai
marked this pull request as ready for review
October 24, 2026 18:24
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
mx.compile(fun)in the Python binding keys its traces by the address of the Python callable (PyCompiledFun::fun_id). Compile caches are thread-local, and each call records its trace in the cache of the calling thread.call_implstored the cache of the calling thread inPyCompiledFun::cache, socachewas the cache of the thread that called the function last.~PyCompiledFuncalledcompile_erase(cache, fun_id)and erased only that cache. Traces that other threads recorded stayed after the free:mx.compileof a callable at the same address, called on that thread with the same input structure, shapes, dtypes and default stream, replays the old trace and does not trace the new function.Fix: the same as mlx #43 for
compile(std::function). The destructor erases the key from every live thread cache through the fork's cache registry (detail::compile_erase(fun_id), PR #10), before it releasesfun.PyCompiledFunno longer keeps a cache, andcall_implno longer reads one on each call. A moved-from object (fun_id == 0) erases nothing.This changes only the Python binding (
python/src). mlx-c and mlx-swift do not compile it, so no pin bump is needed in mlx-c, mlx-swift or mlx-swift-lm.Changes
python/src/transforms.cpp:~PyCompiledFuncallsmx::detail::compile_erase(fun_id); thecachemember and its assignment incall_implare removed.python/tests/test_compile.py:test_compile_free_erases_every_thread_cache. A worker thread makes its default stream, then calls the compiled function two times. The test checks that the second call used the trace (one trace), which shows that the worker finds the entry. The main thread then calls the function (so the main thread called it last), frees it, and the worker compiles the same callable again and calls it. The test expects a new trace there (3 traces in all). It uses only the trace count, the same observable as upstream'stest_compile_release_on_another_thread. No test-only production API.fork.yaml: section "Freed Python compiled functions leave every thread cache".Test first
f40a6f60(first test version): passed without the fix (Fork Tests https://github.com/Layr-Labs/mlx/actions/runs/40117260246). The reason: a trace is cached under the default stream of the calling thread (peek_default_streaminCompileCache::find), and the first op of a new thread makes that stream. The worker's first trace was stored before its stream existed, so no later call on the worker could match it. The compile after the free traced again with or without the erase.ee8eae07(test fixed, no fix): FAILED on the new test only,AssertionError: 2 != 3, with the GPU and with the CPU as the default device: Fork Tests https://github.com/Layr-Labs/mlx/actions/runs/40118257732 (python-gpu (1) job 120944438596, python-cpu (1) job 120944438617).bef32f93(fix): ALL GREEN. Fork Tests https://github.com/Layr-Labs/mlx/actions/runs/40119450075 (the new test passed with the GPU, job 120948067665, and with the CPU, job 120948067783), Build and Test https://github.com/Layr-Labs/mlx/actions/runs/40119449948, Fork Diff Check https://github.com/Layr-Labs/mlx/actions/runs/40119450076.Note: upstream's
test_compile_release_on_another_threadhas the same property as the first version of this test. Its worker traces before its default stream exists, so the test passes whether or not the release erases the entry. It is unchanged here.Draft upstream issue (not opened)
🤖 Generated with Claude Code