BP5: Rerouting improvements#4983
Merged
eisenhauer merged 3 commits intoMay 22, 2026
Merged
Conversation
scottwittenburg
force-pushed
the
rerouting-improvements
branch
2 times, most recently
from
08bf207 to
5d994e1
Compare
scottwittenburg
force-pushed
the
rerouting-improvements
branch
from
5d994e1 to
9a8d2ae
Compare
Contributor
Author
|
@eisenhauer I have left a WIP commit on here for testing in CI, but I will remove that commit in preparation to merge, once you are comfortable with the changes. |
eisenhauer
reviewed
May 6, 2026
scottwittenburg
force-pushed
the
rerouting-improvements
branch
from
9a8d2ae to
7b8ba22
Compare
The MPI version of ISend already returned a Comm::Req object that wrapped however many actual requests resulted from the send (there could be multiple if it was batched). This commit exposes MPI_Test as Test, which returns true if MPI_Test returns true for all the encapsulated request objects.
scottwittenburg
force-pushed
the
rerouting-improvements
branch
from
7b8ba22 to
9a536ca
Compare
Previously the buffer pool for non-blocking sends was a fixed size (set at 100), and had it no guards against re-using (and thus, obliterating) a buffer while a previous ISend using the buffer may still have been incomplete. This commit introduces a role-appropriate pool size, and also adds logic that forces the next ISend to wait if all buffers in the pool are actively being used by a prior ISend. To reduce the likelihood of the background messaging thread spinning and consuming CPU while nothing is happening, this commit introduces a conditional yield within the background thread loop. If no work is done during an iteration of the loop (message receipt and response, rerouting pair identified and messaged, etc...) then yield() is called at the end of the loop. MPI send-to-self can sometimes be slow, and it was used by each subcoordinator to signal itself to write at the beginning of the workflow. This commit replaces the self-send with direct signalling of the main thread to begin writing. Previously, once the loop finished, the thread exited immediately, possibly causing buffers associated with incomplete ISends to be destroyed. Now that we're collecting the request object from each ISend (stored in the buffer pool), this commit iterates the pool one final time to wait for any incomplete ISend to complete.
scottwittenburg
force-pushed
the
rerouting-improvements
branch
from
9a536ca to
bb41ea9
Compare
Member
|
OK, I think this looks good now. I think you can remove the bit that forces rerouting and lets get this merged! |
scottwittenburg
force-pushed
the
rerouting-improvements
branch
from
bb41ea9 to
140091b
Compare
Contributor
Author
|
@eisenhauer I think this is ready for approval/merge whenever you're ready. |
eisenhauer
approved these changes
May 22, 2026
Contributor
Author
|
Thank you for your help getting this merged @eisenhauer! |
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
2 participants
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.
This PR includes one correctness fix, and several small, potential performance improvements to the rerouting aggregator (#4647).
This PR should not be merged while it contains the final WIP commit that forces rerouting (whenever possible) in CI.
Correctness fix
Previously the buffer pool for non-blocking sends was a fixed size (set at 100), and it had no guards against re-using (and thus, obliterating) a buffer while a previous
ISendusing the buffer may still have been incomplete. This commit introduces a role-appropriate pool size, and also adds logic that forces the nextISendto wait if all buffers in the pool are actively being used by a priorISend.Performance improvements
To reduce the likelihood of the background messaging thread spinning and consuming CPU while nothing is happening, this PR introduces a conditional yield within the background thread loop. If no work is done during an iteration of the loop (message receipt and response, rerouting pair identified and messaged, etc...) then
yield()is called at the end of the loop.MPI send-to-self can sometimes be slow, and it was used by each subcoordinator to signal itself to write at the beginning of the workflow. This PR replaces the self-send with direct signalling of the main thread to begin writing.
Previously, once the loop finished, the thread exited immediately, possibly causing buffers associated with incomplete
ISendto be destroyed. Now that we're collecting the request object from eachISend(stored in the buffer pool), this PR iterates the pool one final time to wait for any incompleteISendto complete.