quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.82k stars 2.69k forks source link

Refactoring the way we register classes for reflection, JNI, etc. #25975

Open zakkak opened 2 years ago

zakkak commented 2 years ago

Description

Quarkus currently relies on gizmo to generate a rather large AutomaticFeature for performing a number of things by often invoking GraalVM internal APIs instead of following the "standard" approach of generating json configuration files for GraalVM to consume.

Namely, Quarkus, at the time of writing, seems to rely on the following internal APIs:

  1. com.oracle.svm.util.ReflectionUtil
  2. com.oracle.svm.core.jni.JNIRuntimeAccess
  3. com.oracle.svm.core.jdk.proxy.DynamicProxyRegistry
  4. com.oracle.svm.core.jdk.LocalizationFeature
  5. com.oracle.svm.core.jdk.localization.LocalizationFeature
  6. com.oracle.svm.core.configure.ResourcesRegistry
  7. com.oracle.svm.reflect.serialize.hosted.SerializationFeature
  8. com.oracle.svm.core.jdk.serialize.SerializationRegistry
  9. com.oracle.svm.reflect.serialize.SerializationSupport$StubForAbstractClass
  10. com.oracle.svm.reflect.serialize.SerializationSupport

To me this approach has the following issues:

  1. It makes it harder to add new quarkus features or fix bugs that require tampering with the generation of the AutoFeature. First you need to write code by using an assembler... which you then need to debug by extracting the generated by gizmo class and disassembling it to see if everything looks as expected or not.
  2. We rely on internal APIs with all the issues that brings (e.g. harder to maintain our code because of unexpected changes to internal API, need to ensure the internal APIs are somehow exposed to us, etc.)

Implementation ideas

I propose we start:

  1. Moving as much configuration as possible out of the AutomaticFeature and generating json files instead (which one can easily inspect and alter)
  2. Request upstream Graal to make public any internal APIs that we really need access to.

WDYT?

zakkak commented 2 years ago

Regarding point 2 note also that in the Native Image Committer Community Meeting 2022-06-02 it was stated that:

JNI / Resource / Proxy / Serialization registration

Currently not exposed as public API, but really for no good reason. We have the reflection registration already in the API.

geoand commented 2 years ago

@zakkak has this now been implemented?

jerboaa commented 2 years ago

2. Request upstream Graal to make public any internal APIs that we really need access to.

I think most APIs in use by quarkus are publicly accessible ones in upcoming graal 22.3.

zakkak commented 2 years ago

@zakkak has this now been implemented?

No. We have indeed switched to using mostly public APIs, but we still:

  1. rely on some internal ones (see https://github.com/oracle/graal/issues/5013)
  2. rely on gizmo to generate a Feature where we could instead generate json files.

Note, that there are still cases where we might not be able to avoid any of the above. But I still believe it would be good to try and minimize the generated bytecode.

zakkak commented 1 year ago
  1. rely on gizmo to generate a Feature where we could instead generate json files.

This is now fixed by:

So at this point the only non-API call we appear to still use is org.graalvm.nativeimage.impl.RuntimeClassInitializationSupport#rerunInitialization. Since there is no alternative to this on the Quarkus side, we need some upstream changes to fix it.

zakkak commented 1 year ago

https://github.com/oracle/graal/commit/4880346c9d3491b7b367fcd1ca6bfc5c3fc3fa7f#diff-2f2b04e4347237de74c38065683de2f463234ce2e8e3e37b56bbc66cf2d2d068 enabled by default the "new class initialization strategy that allows all classes to be used at image build time". As a result starting with Mandrel and GraalVM 23.1 Quarkus should no longer need to explicitly set classes to be runtime reinitialized.

I will prepare a patch to test this out.

zakkak commented 9 months ago

Well that took longer than expected...

The conclusion is that the new class initialization doesn't really affect Quarkus since we explicitly set classes to be build time initialized and request their re-initialization explicitly when necessary.

The new class initialization strategy only affects classes that are not explicitly marked for build time initialization, but happen to be build time initialized because they are dependencies of some other build time initialized class.

As a result, we still need some upstream changes to fix it.

jerboaa commented 9 months ago

As a result, we still need some upstream changes to fix it.

For clarity, Quarkus would need an API version for org.graalvm.nativeimage.impl.RuntimeClassInitializationSupport#rerunInitialization. AFAIK, that's the only non-API method still in use. Correct @zakkak?

zakkak commented 9 months ago

Correct.

zakkak commented 9 months ago

According to https://github.com/oracle/graal/issues/5013#issuecomment-1944747803:

@zakkak with the new class initialization policy, RuntimeClassInitializationSupport#rerunInitialization is the same as initializeAtRunTime.

With https://github.com/oracle/graal/pull/8323/files#diff-3c452e61cb9ddfbab251a9aa0a134b4c0be47a0ec39020eee896e97a3ef5e2f1R55 rerunInitialization will be deprecated and hard-code to initializeAtRunTime.

That means that we can just switch to the public API initializeAtRunTime (this is implemented in https://github.com/zakkak/quarkus/tree/2024-02-15-no-runtime-rerun and being tested in https://github.com/zakkak/quarkus/actions/runs/7916551827). However, this only works for 23.1 and onwards. I considered adding some conditional code in the Feature generation to check the Mandrel/GraalVM version and decide which method to use accordingly, but this would introduce a dependency on org.graalvm.home.Version to get the Mandrel/GraalVM version which is undesired.

I think for the time being we should stick to the non-API rerunInitialization method and switch to initializeAtRunTime once we drop support for Mandrel/GraalVM < 23.1.