-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Create OSGi bundles for ND4J #8083
base: master
Are you sure you want to change the base?
Conversation
* Minimal changes to the module build, although it appears that this could be simplified * All Jackson packages exported at the jackson.version Signed-off-by: Tim Ward <timothyjward@apache.org>
This is the least invasive option for creating an OSGi enabled ND4J bundle, but it could be improved * Multiple API jars are combined to avoid split package problems, ideally there would be no split packages * API packages and their versions are determined here, ideally this would be done using annotations in the packages themselves * Some third party code is included because it is not packaged in OSGi bundles * Some third party code leaks out through the ND4J API Signed-off-by: Tim Ward <timothyjward@apache.org>
This is the least invasive option for creating an ND4J backend * The Native Java API, Java Implementation and Native code are all combined into one module * Each platform creates its own specific OSGi bundle complete with native dependencies * The Frontend has a requirement on at least one backend being present * The Frontend logic has been improved to handle backend discovery/loading when it's a separate module * A simple OSGi sniff test has been added to demonstrate loading an ND Array Signed-off-by: Tim Ward <timothyjward@apache.org>
I don't see these changes as "minimally invasive". What prevents us from creating an nd4j-osgi module like the nd4j-uberjar module that doesn't require any changes to the other modules? |
So in this PR:
Of these the only ones which affect existing modules are the internal |
Both backends can be built in the same uber JAR, that's not an issue. If we need to add a dependency on OSGi like that, we'll need to make sure it doesn't break anything anywhere, on Android and Spark among things... Is this something you would have the time to test? :) |
I'm happy to help, but I'm not sure I have the appropriate infrastructure available. Unless of course you have solved this problem already? |
@timothyjward No, not really unfortunately. It's pretty manual running examples from dl4j-examples for most such integration tests. |
@timothyjward @saudet with java 17 LTS forcing module-info I added a semi related PR: #9626 I think I've at least solved part of the problem for modularization. I'm playing with how to add a meta backend that allows 1 or more backends to be present. We also have classloader overriding now. It might allow us to introduce a better way of introducing OSGI support. Any suggestions even if you dont' have time to work on this? |
The biggest issues for OSGi were:
I'm guessing that item 1 has been solved now as JPMS has a similar requirement. Item 2 has hopefully improved, but isn't necessarily a deal breaker. Item 3 is a big deal, but depending on what the "classloader overriding" support you've added offers it may be sufficient. I'd suggest adding the
These issues can then be worked through one at a time as they are identified until the basic OSGi tests start passing. My guess is that there probably won't be many issues left if the split packages are already gone. |
@timothyjward thanks for the comments that helps a lot. I'll try to address what I can in the upgrade. Regarding 2 and 3:
I'll use this PR for reference and see what I can do about adding proper OSGI metadata. Thanks a lot for your comments. |
What changes were proposed in this pull request?
As described in #3022 it is necessary for Deeplearning4J to be provided as OSGi bundles in order for it to be used embedded in Eclipse. OSGi has a very different class loader structure which impacts how DL4J loads its backend implementation and its various plugins.
This PR makes the necessary additions/fixes to provide ND4J as OSGi bundles (the first step on the road to running the full DL4J on OSGi. Ideally the existing ND4J jars would have been enhanced with OSGi metadata, but this is not possible because:
The PR therefore creates an nd4j-osgi bundle which wraps up the "frontend api" modules into a single OSGi bundle JAR. This has a requirement for a "backend". There is also an nd4j-native-osgi bundle providing the backend - this is an OSGi fragment which attaches to the frontend API to provide a working backend.
The overall approach is minimally invasive to the existing ND4J code, there are no API changes and all of the existing jars are still produced as before.
How was this patch tested?
There is an automated OSGi sniff test which has been put in the nd4j-tests-osgi module
Quick checklist
The following checklist helps ensure your PR is complete: