bkietz commented on code in PR #37365:
URL: https://github.com/apache/arrow/pull/37365#discussion_r1307704548
##########
cpp/src/arrow/gpu/cuda_context.cc:
##########
@@ -293,11 +334,32 @@ std::shared_ptr<CudaDevice>
CudaMemoryManager::cuda_device() const {
return checked_pointer_cast<CudaDevice>(device_);
}
+Result<std::shared_ptr<Device::SyncEvent>>
CudaMemoryManager::MakeDeviceSyncEvent() {
+ ARROW_ASSIGN_OR_RAISE(auto context, cuda_device()->GetContext());
+ ContextSaver set_temporary((CUcontext)(context.get()->handle()));
+
+ // TODO: event creation flags??
+ CUevent ev;
+ CU_RETURN_NOT_OK("cuEventCreate", cuEventCreate(&ev, CU_EVENT_DEFAULT));
+
+ return std::shared_ptr<Device::SyncEvent>(
+ new CudaDevice::SyncEvent(context, new CUevent(ev), [](void* ev) {
+ auto typed_event = reinterpret_cast<CUevent*>(ev);
+ auto result = cuEventDestroy(*typed_event);
+ if (result != CUDA_SUCCESS) {
+ // should we throw? I think that would automatically terminate
+ // if you throw in a destructor. What should we do with this error?
Review Comment:
log at warning level, please. This is what we do for other [errors in
destructors](https://github.com/bkietz/arrow/blob/ccae4488e1a0abfc383a2b22f012636ebd47ee21/cpp/src/arrow/util/io_util.cc#L771-L775)
##########
cpp/src/arrow/device.h:
##########
@@ -117,6 +117,9 @@ class ARROW_EXPORT Device : public
std::enable_shared_from_this<Device>,
/// event is completed without blocking the CPU.
virtual Status WaitEvent(const SyncEvent&) = 0;
+ /// \brief Blocks until a stream's remaining tasks are completed
Review Comment:
```suggestion
/// \brief Blocks the current thread on the CPU until a stream's
remaining tasks are completed
```
##########
cpp/src/arrow/gpu/cuda_context.cc:
##########
@@ -281,6 +279,49 @@ Result<std::shared_ptr<CudaDevice>> AsCudaDevice(const
std::shared_ptr<Device>&
}
}
+Status CudaDevice::Stream::WaitEvent(const Device::SyncEvent& event) {
+ auto cuda_event =
+ checked_cast<const CudaDevice::SyncEvent*, const
Device::SyncEvent*>(&event);
+ if (!cuda_event) {
+ return Status::Invalid("CudaDevice::Stream cannot Wait on non-cuda event");
+ }
+
+ auto cu_event = cuda_event->value();
+ if (!cu_event) {
+ return Status::Invalid("Cuda Stream cannot wait on null event");
+ }
+
+ ContextSaver set_temporary((CUcontext)(context_.get()->handle()));
Review Comment:
```suggestion
ContextSaver
set_temporary(reinterpret_cast<CUcontext>(context_.get()->handle()));
```
Sorry for the confusion; seems we have a few other c-style casts in this
file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
