Do not call MPI functions directly from finalizers - #962
Draft
giordano wants to merge 2 commits into
Draft
Conversation
Finalizers can run at any point of the program where the garbage collector is invoked, including concurrently with an MPI call made by another thread (which is permitted only when MPI was initialized with `MPI_THREAD_MULTIPLE`), or in the middle of an MPI call which runs a Julia callback, such as a reduction with a user-defined operator (calling MPI functions inside such callbacks is erroneous). Instead of calling the corresponding `MPI_*_free` function directly, the finalizers of MPI handle objects (communicators, groups, datatypes, operators, infos, requests) now capture the raw handle value in a closure and enqueue it, and the queue is drained before the next MPI call made with `@mpichk`: the fast path of the drain is a single atomic counter check, and enqueueing from a finalizer uses `trylock`, followed by re-registration of the finalizer when the lock is contended, as finalizers must never block on a lock. This design is similar to how PythonCall.jl defers the freeing of Python objects from finalizers to the next time the GIL is held. Explicit calls to `MPI.free` keep freeing the handle immediately. The finalizers of `Win` and `FileHandle` objects are unchanged: freeing those handles is a synchronizing/collective operation, which deferral would not make safe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Test that garbage-collecting MPI handle objects enqueues the freeing of the underlying handles without calling MPI functions, that the queue is drained by the next MPI call, that explicit `MPI.free` keeps freeing immediately, and that null handles (e.g. requests completed by a wait) have nothing to free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
@vchuravy does this address your concerns in #955 (comment)?
However I'm not happy at all about the implementation, as it adds a new function call before each MPI call. Benchmark:
on
master, with 2 ranks:on this branch:
The overhead is non-negligible (although of the order of ~nanoseconds)