Mark copy events as nodiscard
#286
Conversation
stephenswat
force-pushed
the
feat/nodiscard_copy
branch
2 times, most recently
from
a2ef8b0 to
9389492
Compare
There was a problem hiding this comment.
🤔 I considered doing this a while ago. But decided that it would be too painful for the users to always have to deal with adding wait() and ignore() calls, for all vecmem::copy variants. Even all the synchronous ones.
Since forgetting to add these calls is "only" a performance penalty in the current setup, I introduced VECMEM_FAIL_ON_ASYNC_ERRORS in #270.
Now... I do not think that that solution is perfect. Not at all. But I'm very afraid of making the UI "too painful" to use. And I fear that using [[nodiscard]] on all of these functions is very painful. 😦
|
Hmm, not necessarily. I modified traccc using this PR and it only came to 20 files changed, 86 insertions(+), 69 deletions(-). So not that painful, really. |
Unfortunately, they can also be runtime errors in SYCL code if you use unordered streams. |
Using acts-project/vecmem#286 I was able to find a few more cases where we do not properly wait for or ignore events. In this PR I opted to add those missing statements according to a simple heuristic, i.e. I added waits for all SYCL and share code and I added ignores for all CUDA code. Reason for this is that CUDA events are stream-ordered, so you can't really shoot yourself in the foot too much. SYCL events, on the other hand, are unordered, so they need stricted checking.
stephenswat
force-pushed
the
feat/nodiscard_copy
branch
from
9389492 to
ccbade8
Compare
|
Unfortunately there are some HIP issues after all. 😦 (Some missing return value handling was left in.) |
stephenswat
force-pushed
the
feat/nodiscard_copy
branch
from
ccbade8 to
1d3b9d7
Compare
Currently, there are a few copy statements in the testing code which are not being properly ignored or waited for. Thus, this commit adds the appropriate code to ensure that the (potentially) asynchronous properties of these copies are properly handled.
|
Updated to pass the CI. @krasznaa |
Currently, there is no compile-time mechanism to ensure that copy objects are properly handled, i.e. either waited for or ignored. This commit adds the
nodiscardattribute to methods of the copy objects which return events to ensure that the compiler will issue a warning if the object is ignored.