Skip to content

Clean up completed communicator receives#804

Merged
Binyang2014 merged 6 commits into
mainfrom
binyli/request
May 15, 2026
Merged

Clean up completed communicator receives#804
Binyang2014 merged 6 commits into
mainfrom
binyli/request

Conversation

@Binyang2014
Copy link
Copy Markdown
Contributor

@Binyang2014 Binyang2014 commented May 15, 2026

Summary

  • Release the reference after last requests are ready.
  • Keep ordered receive chaining for repeated rank/tag operations while cleaning up completed receive bookkeeping.

Binyang2014 and others added 2 commits May 15, 2026 16:58
Erase completed receive bookkeeping from the communicator once the deferred receive future finishes, while preserving ordered receive chaining for repeated rank/tag operations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Binyang2014
Copy link
Copy Markdown
Contributor Author

/azp run mscclpp-ut

@Binyang2014 Binyang2014 requested a review from Copilot May 15, 2026 17:03
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@Binyang2014 Binyang2014 requested a review from a team May 15, 2026 17:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR refactors ordered “receive” operations to centralize chaining via a helper and to clean up receive bookkeeping once the final request completes.

Changes:

  • Introduces a generic makeOrderedRecvFuture helper to enforce ordered receive chaining per (remoteRank, tag).
  • Adds a ScopeGuard-based cleanup path intended to erase completed (remoteRank, tag) entries from the “last receive” bookkeeping.
  • Refactors recvMemory, connect, and buildSemaphore to use the new helper.

Comment thread src/core/communicator.cc Outdated
Comment thread src/core/communicator.cc Outdated
Comment thread src/core/communicator.cc Outdated
@Binyang2014
Copy link
Copy Markdown
Contributor Author

/azp run mscclpp-ut

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Comment thread src/core/communicator.cc Outdated
@Binyang2014
Copy link
Copy Markdown
Contributor Author

/azp run mscclpp-ut

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@Binyang2014 Binyang2014 enabled auto-merge (squash) May 15, 2026 21:06
@Binyang2014 Binyang2014 merged commit 60a6d72 into main May 15, 2026
13 checks passed
@Binyang2014 Binyang2014 deleted the binyli/request branch May 15, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants