-
Notifications
You must be signed in to change notification settings - Fork 18
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
Replace single-use MPI datatypes with binary serialization #114
Conversation
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.
To document the offline discussions we've had, and add additional comments:
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.
LGTM.
As per offline discussion yesterday, I've decided to duplicate the dependency count inside |
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.
One minor thing, otherwise LGTM!
Duplicates dependency count in command_frame
So far, we are creating one custom composite
MPI_Datatype
for eachMPI_Isend
to transfer variably-sized commands and buffer subranges. For reasons not investigated further, this yields abysmal performance at least on IBM Spectrum MPI.This PR replaces those single-use MPI datatypes with a serialization into frames, contiguous
struct
s with a zero-sized array as the last member, a common idiom for typing a header together with variably-sized data. These are sent and received over MPI as simple byte streams. The smart pointer typeunique_frame_ptr
is introduced to allocate and manage frame types with arbitrary, variably-sized payloads.As a downside, this change adds a semantically unnecessary allocation and copy of incoming buffer transfers: MPI receives
data_frame
s, whereas the buffer manager keeps uncommitted transfers asraw_buffer_data
. This might be noticeable for very large transfers. The solution is likely to get rid of the owningraw_buffer_data
altogehter, but this will affect a lot of code, so it should be addressed in a follow-up PR.