-
Notifications
You must be signed in to change notification settings - Fork 31
Update request pool to allow individual request operations #48
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
base: master
Are you sure you want to change the base?
Conversation
@@ -193,7 +226,8 @@ namespace mpl { | |||
/// Cancels a pending request in the pool. | |||
/// \param i index of the request for which shall be cancelled | |||
void cancel(size_type i) { | |||
MPI_Cancel(&requests_[i]); | |||
if (requests_[i] != MPI_REQUEST_NULL) | |||
MPI_Cancel(&requests_[i]); |
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.
Actually I'm not quite sure about this. I added this check since I notice that it is not impossible for user to push an already moved request into the pool. So when I noticed that base_request::cancel
(line 100) has this check, I added also here. But if there's an assumption that user will never push a null request into the pool, this check can be removed for consistency.
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 need to double-check this. But I think the check against MPI_REQUEST_NULL
is actually needed here. Thanks for spotting this.
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 am not sure if this is actually an improvement. I can hardly imagine a situation where one wants to put a request into a request_pool
and wait for its completion individually (rather than using waitall
or waitany
). If one needs to wait for a specific request, one should not move it into a pool.
Furthermore, the new functions are inconsistent with the existing class method. The new methods return a status object, whereas the existing one operate on status objects that are maintained by the pool but not accessible by the user.
There is a common situation where we need both individual The issue actually raise from the flexibility. If we use native MPI interface, it is natural to perform both individual operations like If we wish to keep the current pattern of
|
I think try_get_status is a more semantic naming than get_status_of. |
Addressing #47. Changes:
request_pool::cancel