kenba/opencl3

It's safe to implement `Send` for most of the types

Closed this issue · 5 comments

vmx commented

I know this has been discussed before, but back then it was conflated with other topics. This issue is focused on Send.

@kenba, you've made sure that opencl3 is a safe wrapper around an OpenCL FFI. For example the types are not Copy or Clone to have these safety guarantees as good as possible. Following the Rust API Guidelines about interoperability, those types should then implement Send.

Let me take CommandQueue as an example. It is a struct with two fields, one is of type cl_command_queue, the other one is of type cl_uint. cl_uint is not of concern, as it is just a type alias for i32, which is already safe. The problem is cl_command_queue, which is a raw *mut c_void pointer. CommandQueue owns the pointer, so no one else outside of CommandQueue can manipulate it, once passed in. When using the library the pointer will stay around, you already built the library so that is the case. Given all these things, the CommandQueue qualifies as Send, hence can implement Send, despite having a raw (non-Send) pointer internally.

Another good read in regards to FFIs and Send is this answer on Stack Overflow.

kenba commented

@vmx please feel free to submit a pull request with Sends for the classes that you want to implement it.
However, I'm not convinced that Send should be implemented for Kernel, considering the comments in the OpenCL API.

vmx commented

However, I'm not convinced that Send should be implemented for Kernel, considering the comments in the OpenCL API.

Let's focus on that for now (before I do a PR), as I would want to implement Send for Kernel. I guess your concerns are about the Multiple Host Threads section, the core if it is:

clSetKernelArg, clSetKernelArgSVMPointer, clSetKernelExecInfo and clCloneKernel are safe to call from any host thread, and safe to call re-entrantly so long as concurrent calls to any combination of these API calls operate on different cl_kernel objects. The state of the cl_kernel object is undefined if clSetKernelArg, clSetKernelArgSVMPointer, clSetKernelExecInfo or clCloneKernel are called from multiple host threads on the same cl_kernel object at the same time.

With implementing Send it will still only possible for a single thread to access the cl_kernel object. It won't enable concurrent access/calls. For concurrent access by multiple threads you would need to implement Sync.

What Send enables you to do is, moving things off the main thread and do computations there. The main thread will loose its access to the object once it was sent to the other thread.

kenba commented

Thanks Volker. Since Send just enables you to move things off the main thread, you can implement it for whatever you want.

vmx commented

I went ahead an implemented Send for almost every struct. The only struct that doesn't implement is Send is cl_image_desc. I didn't spent time thinking about whether it could implement Send as I don't see great use of it being Send. I'd probably rather change the API so that it isn't even needed to be constructed when images are used (but that's surely out of scope for this PR).

kenba commented

@vmx has implemented Send for most of the types.