graal
graal copied to clipboard
[GR-40198] Provide public API for feature-based JNI / Resource / Proxy / Serialization registration.
This PR adds the missing APIs that will allow our users to not rely on our svm-internal APIs anymore.
@olpaw Thanks for adding me to this. Is this work being planned for 22.3? As this would be related to https://github.com/oracle/graal/discussions/4792 do you have some details on what exactly is svm-internal API and, thus, will allow us to determine what will move?
Is this work being planned for 22.3?
Yes. If you can help make sure that everything you need is in the first batch, that would speed up the overall migration process for all of us. :)
As this would be related to https://github.com/oracle/graal/discussions/4792 do you have some details on what exactly is svm-internal API and, thus, will allow us to determine what will move?
The migration should happen in a backward compatible way, so we don't plan to move anything just yet. This PR adds a new API to the Native Image part of the GraalVM SDK that frameworks are supposed to use in the future. Everything in com.oracle.svm is considered internal, while org.graalvm.nativeimage is the public API. So essentiall, we need to make sure, grep -R "com.oracle.svm" /the/quarkus/codebase does not return any results. :)
@olpaw Thanks for adding me to this. Is this work being planned for 22.3?
Yes
As this would be related to #4792 do you have some details on what exactly is svm-internal API
Everything that is part of our SVM codebase that currently requires a --add-exports when the image-builder is running on module path (with GraalVM 22.2) (i.e. when the USE_NATIVE_IMAGE_JAVA_PLATFORM_MODULE_SYSTEM=false workaround is NOT used) is considered internal API. See add-exports in https://github.com/oracle/graal/blob/master/java-benchmarks/mx.java-benchmarks/mx_java_benchmarks.py#L374..L380
and, thus, will allow us to determine what will move?
As you can see this PR only adds API but does not remove anything. To be on the safe side projects that currently use svm-internal API should switch to using public API only (for example instead of using com.oracle.svm.core.jni.JNIRuntimeAccess.register use org.graalvm.nativeimage.hosted.RuntimeJNIAccess.register). Users that will continue using non-public API are at risk of experiencing breaking changes as we might modify non-public API as we see fit.
One major package that I see missing from this batch is
com.oracle.svm.core.annotate, not sure if you plan to make it public API in a different PR.
@zakkak, yes making the annotations available as public API is a different PR: https://github.com/oracle/graal/pull/4683
Here I suggest to just copy our implementation (as a starting point). This is a simple helper-method that itself only depends on JDK code. Thus you are better off having your own variant of it in your codebase (i.e. you might want to use such a method in places that otherwise do not require native-image at all).
9.
com.oracle.svm.hosted.reflect.serialize.SerializationFeature, this is added as a required feature in one of the quarkus features.
Here we should make sure that all SVM internal feature handlers have priority (i.e. are executed before) any user-provided feature handler (e.g. Quarkus features). This way your code that specifies SerializationFeature as a required feature becomes obsolete and can be removed. cc @christianwimmer
6. com.oracle.svm.core.jdk.localization.LocalizationFeature#prepareBundle
No need to use the internal prepareBundle. Use org.graalvm.nativeimage.hosted.RuntimeResourceAccess#addResourceBundles instead.
com.oracle.svm.core.jdk.UnsupportedFeatureErrorto report a more quarkus-friendly error
I agree. We should discuss a custom solution for this use of com.oracle.svm.core.jdk.UnsupportedFeatureError in quarkus.
BTW, thanks for the detailed list above @zakkak !
9.
com.oracle.svm.hosted.reflect.serialize.SerializationFeature, this is added as a required feature in one of the quarkus features.
@zakkak if you stop using @AutomaticFeature (by using --features <list of FQNs of quarkus features> instead) you should be able to get rid of requiring your feature to specify depending on SerializationFeature because @AutomaticFeatures are always registered before non-automatic ones and thus their hooks are always executed before the ones from user provided features.
- com.oracle.svm.hosted.reflect.serialize.SerializationFeature, this is added as a required feature in one of the quarkus features.
@zakkak I'm quite curious why you need this feature dependency at all.
@zakkak if you stop using
@AutomaticFeature(by using--features <list of FQNs of quarkus features>instead) you should be able to get rid of requiring your feature to specify depending onSerializationFeaturebecause@AutomaticFeatures are always registered before non-automatic ones and thus their hooks are always executed before the ones from user provided features.
Cool, we have already done that in https://github.com/quarkusio/quarkus/pull/25976 so in theory we no longer need this.
@zakkak I'm quite curious why you need this feature dependency at all.
The main reason according to https://github.com/quarkusio/quarkus/pull/15380 was that:
If serialization requirement is detected, generated AutoFeature depends on SerializationFeatue (there is a "cache" of registered serialized classes, which is required in runtime.
Another reason is discussed in https://github.com/quarkusio/quarkus/pull/25861#issuecomment-1146514806 and https://github.com/quarkusio/quarkus/pull/25861#issuecomment-1147127342.
Given that @AutomaticFeatures are always registered before non-automatic ones I will prepare a PR removing the dependency.
Update: The PR removing the dependency is https://github.com/quarkusio/quarkus/pull/27263
Thank you @olpaw and @christianwimmer
We have some usages that I think are not yet covered by this (workarounds are very welcome):
com.oracle.svm.core.SubstrateOptions- Used to get the value of
SubstrateOptions.Class, as we have multiple entrypoints within a module, and need to switch config based on which one is used.
- Used to get the value of
com.oracle.svm.core.jdk.NativeLibrarySupport,com.oracle.svm.hosted.c.NativeLibrariesandcom.oracle.svm.core.jdk.JNIRegistrationUtil- Used to write library support in a similar way to
JNIRegistrationJava. - The JNIRegistrationUtil superclass can be substituted by our own implementation, but without the other classes we have no way to do the static linking ourselves.
- Used to write library support in a similar way to
com.oracle.svm.core.jdk.Resources#registerResource(java.lang.String, java.lang.String, java.io.InputStream)- Used in our custom support for the fontmanager library, to add the
fontconfig.bfcfile to the resources, to be used from the substitution. - We can possibly work around this by loading the resource in a static byte array instead in our substitutions.
- Used in our custom support for the fontmanager library, to add the
com.oracle.svm.hosted.FeatureImpl.DuringSetupAccessImpl#registerClassReachabilityListener- Used to add reachability handlers based on conditions not currently available. For us specifically a listener that fires for all classes with a specific annotation.
com.oracle.svm.hosted.classinitialization.ConfigurableClassInitialization- Hack to conditionally add resource bundles during analysis, used in the same way as here.
- Any workaround suggestions to add simple resource-bundles on the fly would be much appreciated.
Hi @kristofdho
Thanks for your feedback!
This PR is in its final stages but there can be followup PRs to address the issues on your list if necessary.
We have some usages that I think are not yet covered by this (workarounds are very welcome):
1. `com.oracle.svm.core.SubstrateOptions` * Used to get the value of `SubstrateOptions.Class`, as we have multiple entrypoints within a module, and need to switch config based on which one is used.
I see your point. Note that the use of SubstrateOptions.Class is not safe for what you are doing anyway. Because what will be the mainClass depends on more than just SubstrateOptions.Class. See com.oracle.svm.hosted.NativeImageGeneratorRunner#buildImage at if (className.isEmpty()) { ....
But since it's you providing the arguments to the native-image command what prevents you from using native-image ... foo.MyMainClass -Dkristof.mainclass=foo.MyMainClass and then in the feature you use System.getProperty("kristof.mainclass") to switch the configs.
I will think a bit more about the other points you raised and get back to you next week.
@olpaw Do I need to create a new issue with these remaining usages?
@olpaw Do I need to create a new issue with these remaining usages?
That would be a good idea. Then it's easier to keep track of what is still open.