search

gpu / JOCL

Java bindings for OpenCL
http://www.jocl.org
Other
187 stars 33 forks source link

Make nativePointer readable from the outside #26

Closed haesleinhuepf closed 4 years ago

haesleinhuepf commented 4 years ago

Dear @gpu ,

we (CC @bnorthan) recently came across the issue that we cannot access the nativePointer inside the NativePointerObject class. However, this would be very useful to bridge towards other OpenCL-libraries as discussed here: https://forum.image.sc/t/high-performance-fft-based-algorithms-deconvolution/31710/38

Would you mind merging this PR and deploying a new JOCL?

Thanks a lot in advance!

Cheers, Robert

gpu commented 4 years ago

I usually hesitate when making something public - particularly when it is such a low-level "implementation detail". But I see that there are cases for using the native pointer value - namely, for different sorts of interoperability. I haven't read the full thread that you linked to, but this also seems to be the case here: When there is another JNI-based library, there is no other way to "translate" a JOCL Pointer into the respective pointer type of the target library*.

If you added a word of caution, like this...

    /**
     * Method to obtain the native pointer value.
     *
     * Clients should usually not use this pointer value directly.
     * It is only intended for interoperability with other JNI based
     * libraries.
     *
     * @return The native pointer value
     */

then I'd merge it, and schedule an update.

* My hesitation to make things like this public is due to experience. One should really, really, really think carefully about visibility levels. Once something is public, it is more likely to cause breaking changes for clients. But... in the context of the NativePointerObject in JOCL and JCuda, I've observed the exact opposite effect: People want (or need) this value. And gosh, I've seen some crappy attempts to obtain it: People have extended the NativePointerObject class to override the method to be public. People have used reflection with blatant nativePointerField.setAccessible(true)/long x = nativePointerField.get(pointer) calls. The thread that you linked to now talks about actually parsing the value from the toString representation - ouch. This has to stop. So making it public (with an appropriate disclaimer in the JavaDoc) is far less brittle than all the alternatives...

(One could argue that there should also be a constructor that receives the long value, to allow creating a JOCL Pointer from a given third-party pointer, but that's probably a different issue)

haesleinhuepf commented 4 years ago

Hi @gpu,

great, thanks for your efforts! And thanks for your detailed explanation. We're absolutely aware of these issues and we're happy to take care of them. Making OpenCL exploitable for end users without compatibility and interoperability issues is our mission and obviously also yours. Thanks for your support and thanks for making JOCL! It's an amazing bridge enabling so many things in our tiny image processing universe!

Merry Christmas! 🎄

Cheers, Robert

gpu commented 4 years ago

Apologies for some non-PR-related chit-chat here:

I've skimmed over the projects that this related to, and many of them really look interesting. Some fairly random pointers to what I mean:

I'd have to invest more time to have a closer look at all this....

gpu commented 4 years ago

If you don't update the PR with the additional comment, I'd do this directly in the master branch ASAP.

haesleinhuepf commented 4 years ago

Hi @gpu

ah sorry, I was sleeping ;-)

I just added the comment. Thanks for your support!

Cheers, Robert

gpu commented 4 years ago

Thanks, merged it. But I still have to schedule an update for the maven release. (I'm rather busy ATM, there's an update for JCuda on the way, but I hope that I can finish that this week, so next week might be the time for an update of JOCL)

haesleinhuepf commented 4 years ago

Cool, thanks, @gpu! Keep us posted 😉

Cheers, Robert

gpu commented 4 years ago

Version 2.0.2, with NativePointerObject#getNativePointer being public, should be available in Maven Central in a few minutes, with the following coordinates:

<dependency>
    <groupId>org.jocl</groupId>
    <artifactId>jocl</artifactId>
    <version>2.0.2</version>
</dependency>

Please open an issue if you encounter any problems.

bnorthan commented 4 years ago

Thanks @gpu . The intended use case of this is to make it easy to call existing c/c++ opencl algorithms (like deconvolution) from @haesleinhuepf's CLIJ project. The idea is to make simple wrappers around existing code build on clFFT. An alternative could of been to wrap all of clFFT, however that could be a huge undertaking (I don't know for sure) plus the idea of CLIJ is to make it easy for intermediate level programmers (mainly biologists) to write GPU image processing code. I think that clFFT is a pretty obtuse API, and exposing a subset with a simplified API, would be a better fit for the intended users, than exposing the entire clFFT API.

gpu commented 4 years ago

Are you referring to the clFFT from https://github.com/clMathLibraries ? Quite a while ago, I tackled their clBLAS bindings, and have some code generator infrastructure that could cover some of the work that has to be done for clFFT as well. But the development of the AMD CL libraries seems to have stopped (and compiling* these libraries was a bit difficult back then - I think I remember some dependency hassle that I went through). So now there is https://github.com/gpu/JOCLBlast for the (more mature, simple and actively developed) https://github.com/CNugteren/CLBlast/ . Also, I think there is not sooo much demand for GPU-based FFT from Java - CLIJ and image processing being a "niche case" here.

bnorthan commented 4 years ago

Yes. It looks like clFFT is also being maintained by ArrayFire here.

In CLIJ and image processing there is a need for high performance FFT, a OpenCL BLAS implementation and (eventually) Wavelets. Thanks for the link to the BLAS implementations, those could be useful too.