[SYCL][CUDA] Use cuEventQuery to check event completion status #3544
Conversation
| } | ||
| if (ret == CUDA_ERROR_NOT_READY) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
I believe we can safely set isCompleted_ to true here. Will allow it to take a slightly faster path in case this check is made again.
There was a problem hiding this comment.
Done! I've changed both _pi_event::is_completed and _pi_event::get_execution_status to non-const.
There was a problem hiding this comment.
Oh, now I get the comment you made in the summary. It should be fine as non-const, but if it goes against your better judgement I am okay with you reverting it and keeping is_completed as const. I will leave that up to you.
Either way, LGTM! 😄 👍
There was a problem hiding this comment.
I'm going to go on a limb and say once the status is complete , users aren't likely to query it again, so optimizing this shouldn't be necessary. Reverted ;-).
There was a problem hiding this comment.
This would leave the "_pi_event" object in a malformed state where the isCompleted_ is false for a completed event. Light +1 for making this non-const.
There was a problem hiding this comment.
Another option is to rename it to hasBeenWaitedOn_ or the like. To my knowledge, isCompleted_ is only used for this.
There was a problem hiding this comment.
@smaslov-intel You are right, that is no good either. I'm still feeing a bit iffy about making an is_* function non-const though.
I like the idea of hasBeenWaitedOn_ as well. If there are no objections, I will move forward with this in a couple of hours!
psalz
force-pushed
the
fix-event-cmd-status-query
branch
3 times, most recently
from
30a0087 to
39485a3
Compare
Previously the completion status of a PI CUDA event (returned by a `get_info` query for `info::event::command_execution_status`) would only be changed to `command_execution_status::complete` once the event had been waited upon. This changes the mechanism to query the underlying event's execution status using `cuEventQuery` if the event has not been waited upon.
psalz
force-pushed
the
fix-event-cmd-status-query
branch
from
39485a3 to
eb7b20d
Compare
|
/summary:run |
Previously the completion status of a PI CUDA event (returned by a
get_infoquery forinfo::event::command_execution_status) would only be changed tocommand_execution_status::completeonce the event had been waited upon.This changes the mechanism to query the underlying event's execution status using
cuEventQueryif the event has not been waited upon. I didn't memoize the result of the query asis_completed()is markedconst(and makingisCompleted_amutablemember also doesn't seem right).I've created a corresponding PR for the SYCL CTS that tests for this.