Repository navigation
Detect cache thrashing in LRUCache - #2600
Merged
Merged
Conversation
faisalmemon
pushed a commit
to faisalmemon/mlx
that referenced
this pull request
Oct 30, 2025
* Detect cache thrashing in LRUCache * Do not check cache thrashing in tests
1 task done
This was referenced Aug 17, 2026
zcbenz
pushed a commit
to strayberry/mlx
that referenced
this pull request
Aug 23, 2026
CommandEncoder::commit() returns early when an exception is thrown, for example by the graph-cache thrashing check from ml-explore#2600, leaving from_nodes_, to_nodes_, the graph keys, node_map_ and graph_ holding the nodes of the failed graph. The next commit mixes them into a fresh graph and fails in cudaGraphAddDependencies with cudaErrorInvalidValue, and every evaluation after that keeps failing until the process is restarted. Split the body into commit_impl() and restore the same state the successful path restores when it throws, so a recoverable error only fails the evaluation that caused it. Two details the recovery depends on: - check_cuda_error() never calls cudaGetLastError(), so the runtime error stays pending. ~CudaHandle() skips its destroy while an error is pending, which would leak every handle released afterwards and keep the graph from being recreated. Clear it first. - CudaHandle::operator= can throw from both the destroy and the create. A graph left populated is worse than none: its nodes are still roots and run again on the next launch, while the fresh dependencies reference only the new nodes. Drop the handle even when destroying it fails. The regression test shrinks the graph cache on a fresh stream and walks distinct graph topologies until the check fires. The cache key is topological, so varying shapes alone does not produce new keys. Before this change the test sees one thrashing error followed by five cudaGraphAddDependencies failures; after it, six independent thrashing errors and no corruption.
4 tasks done
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.
Add a simple detection for cache thrashing when using LRUCache, by counting if cache misses have exceeded 2 * capacity. When that happens an exception would be thrown: