Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make nativePointer readable from the outside #26

Merged
merged 3 commits into from
Jan 16, 2020
Merged

Conversation

haesleinhuepf
Copy link
Contributor

@haesleinhuepf haesleinhuepf commented Dec 23, 2019

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

haesleinhuepf added 2 commits December 23, 2019 23:13
by chaning its access to public
@gpu
Copy link
Owner

gpu commented Dec 24, 2019

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
Copy link
Contributor Author

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
Copy link
Owner

gpu commented Dec 25, 2019

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:

  • ImageJ in general. I'm really not up to date here, but remember some hiccups around ImageJ back when I created the JCuda ImageJ example at https://github.com/jcuda/jcuda-imagej-example , with a transition from "ImageJ 1" to "ImageJ 2" and some changes in the plugin concept...

  • The general image processing libraries: From a very short glance at the landing pages of some projects, some of them seem to do "3D image visualization" (as in "3D textures"). In fact, for some completely unrelated reason, I recently thought about porting the "Volume Rendering" JCuda/CUDA sample from https://github.com/jcuda/jcuda-samples/blob/master/JCudaSamples/src/main/java/jcuda/driver/gl/samples/JCudaDriverVolumeRendererJOGL.java to JOCL - it shouldn't be sooo hard, and I just like the results:

    image

  • Once I attempted to create bindings for NVIDIA NPP. The image-related part is particularly relevant here: https://docs.nvidia.com/cuda/npp/group__nppi.html . But this has been dormant for a while, for several reasons. (E.g. 1. NPP is huge, and even though I can generate much of the JNI code automatically, the generator would have to be extended considerably. 2. NPP is always specific for NVIDIA. 3.: NPP is far from trivial to use. 4. The effort-to-benefit ratio for maintaining such a lib as a 1:1 binding seems to be too high)

  • The things that are not specifically about image processing, but related to SciJava: I noticed that KNIME is listed as one of the SciJava-related projects. I've also once spent a considerable amount of time for things like https://github.com/javagl/Flow - of course, this is not nearly in the same league as KNIME or RapidMiner (and I was forced to publish it in a state that I'd consider to be too early for publishing), but I have worked on many related projects "under the hood" since then (although they they are not public)

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

@gpu
Copy link
Owner

gpu commented Jan 10, 2020

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

@haesleinhuepf
Copy link
Contributor Author

Hi @gpu

ah sorry, I was sleeping ;-)

I just added the comment. Thanks for your support!

Cheers,
Robert

@gpu gpu merged commit e9a1d66 into gpu:master Jan 16, 2020
@gpu
Copy link
Owner

gpu commented Jan 16, 2020

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
Copy link
Contributor Author

Cool, thanks, @gpu! Keep us posted 😉

Cheers,
Robert

@gpu
Copy link
Owner

gpu commented Jan 30, 2020

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
Copy link

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
Copy link
Owner

gpu commented Jan 31, 2020

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
Copy link

bnorthan commented Feb 2, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants