-
Notifications
You must be signed in to change notification settings - Fork 61
Active Message APIs: support scatter-gather I/O and user-defined header data (Part 1: C++ APIs) #594
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
rapids-bot
merged 19 commits into
rapidsai:main
from
grlee77:grelee/ucxx-iov-updates-cpp
Mar 3, 2026
Merged
Active Message APIs: support scatter-gather I/O and user-defined header data (Part 1: C++ APIs) #594
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
9d4e4dc
add typedefs for AmSendMemoryTypePolicy and AmSendParams
grlee77 8e8b02a
add contiguous + IOV constructors for AM requests
grlee77 f7c8210
add Endpoint overloads for AmSendParams and std::vector<ucp_dt_iov_t>…
grlee77 88b6cc9
add test cases for host IOV AM send
grlee77 c089729
remove const from AmReceiverCallbackInfo members so copy and move con…
grlee77 87c169c
Fix segfault in IOV sends (request_am.cpp)
grlee77 f4c69e2
update failing test case
grlee77 f4bad74
support opaque userHeader field in AmHeader
grlee77 53b3cd2
document header segment size limitation in typedefs.h
grlee77 e50506e
update comment to clarify purpose of _count
grlee77 1f6604d
change iov argument to AmSend from const ref to pass by value (then m…
grlee77 5e00989
Merge branch 'main' into grelee/ucxx-iov-updates-cpp
grlee77 2e3c043
Update cpp/src/request_data.cpp
grlee77 a8f7f58
lint fixes
grlee77 49634b8
fix use after move
grlee77 382ab84
change userHeader to std::vector<std::byte>
grlee77 69d6c20
change AmHeaderSerialized to std::vector<std::byte> as well for consi…
grlee77 5377c03
Merge branch 'main' into grelee/ucxx-iov-updates-cpp
pentschev 479af66
Fix style
pentschev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Should we store
std::vector<std::byte>rather than std::string here? WDYT?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that
std::vector<std::byte>is semantically clearer for C++. The reason for this choice was consistency with the existingAmHeaderSerializedwhich has typestd::string. I think the reason forstd::stringis likely that Cython provides<string><->bytescasting so in the Cython code this currently allows simplywhere
user_headeris a Python bytes object (as inlibucxx.pyxin #595)If we change it then I think that Cython code becomes something like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with us breaking existing implementation or even API for clearer semantics, even if handling on the Cython side is a bit more brittle. With that said, I prefer that we treat C++ as first class citizen even if that means exposing code to Cython becomes sort of a second class citizen, so I would also prefer
std::vector<std::byte>here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, ok, this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let me try changing it and if it isn't too hard on the Python/Cython side we can make the change.