Skip to content

Revised MemoryChannel interfaces #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 25, 2025
Merged

Revised MemoryChannel interfaces #508

merged 11 commits into from
Apr 25, 2025

Conversation

chhwang
Copy link
Contributor

@chhwang chhwang commented Apr 22, 2025

  • Moved the MemoryChannel::copy() method out of the MemoryChannel as a standalone function.
  • Renamed mscclpp::putPackets() and mscclpp::getPackets() to mscclpp::copyToPackets() and mscclpp::copyFromPackets() respectively for consistency.
  • Renamed MemoryChannel::getPackets() to MemoryChannel::unpackPackets() for clarity. Renamed getPacketBuffer to packetBuffer.
  • Added the MemoryChannel::unpackPacket() method that unpacks one packet in the buffer.
  • Added the BaseMemoryChannel class that only contains a semaphore without memory addresses.
  • Removed the MemoryDevice2DeviceSemaphoreDeviceHandle::signalPacket() method that is lacking use cases.

@chhwang
Copy link
Contributor Author

chhwang commented Apr 22, 2025

/azp run mscclpp-ut

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang chhwang force-pushed the chhwang/rev-channel-ctor branch from 8d4c6ad to be711d9 Compare April 22, 2025 20:39
@chhwang
Copy link
Contributor Author

chhwang commented Apr 22, 2025

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@chhwang chhwang requested a review from Binyang2014 April 22, 2025 21:17
@chhwang
Copy link
Contributor Author

chhwang commented Apr 22, 2025

/azp run mscclpp-test

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang chhwang requested a review from caiomcbr April 22, 2025 21:34
@chhwang
Copy link
Contributor Author

chhwang commented Apr 24, 2025

/azp run mscclpp-test

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@Binyang2014 Binyang2014 left a comment

Choose a reason for hiding this comment

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

LGTM, One question is how to use BaseMemoryChannel.
Seems we could use like this right?

void kernel(mscclpp::DeviceHandle<BaseMemoryChannel> ch, src /*local src*/, dst /*remote dst*/) {
  ch[peer_id].signal;
  ch[peer_id].wait;
  mscclpp::copy(dst, src, size);
}

@chhwang
Copy link
Contributor Author

chhwang commented Apr 24, 2025

LGTM, One question is how to use BaseMemoryChannel. Seems we could use like this right?

void kernel(mscclpp::DeviceHandle<BaseMemoryChannel> ch, src /*local src*/, dst /*remote dst*/) {
  ch[peer_id].signal;
  ch[peer_id].wait;
  mscclpp::copy(dst, src, size);
}

Yes this is how I intend

@chhwang chhwang enabled auto-merge (squash) April 24, 2025 23:59
@chhwang chhwang merged commit 710f668 into main Apr 25, 2025
14 checks passed
@chhwang chhwang deleted the chhwang/rev-channel-ctor branch April 25, 2025 00:02
chhwang added a commit that referenced this pull request Apr 25, 2025
chhwang added a commit that referenced this pull request Apr 25, 2025
* Wrong offsets in `unpackPackets()`
* Added Python binding of `BaseMemoryChannel`
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.

2 participants