Skip to content

Expose findAnnotated* in public API #4925

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

Closed
SergejIsbrecht opened this issue Sep 11, 2022 · 9 comments
Closed

Expose findAnnotated* in public API #4925

SergejIsbrecht opened this issue Sep 11, 2022 · 9 comments

Comments

@SergejIsbrecht
Copy link

SergejIsbrecht commented Sep 11, 2022

Feature request

I would like to implement a Feature to resolve all classes, which are marked with a special Annotation. As far as I see it there is straight forward way to get all classes / interfaces, which would provide such capabilities.

    implementation("org.graalvm.nativeimage:svm:22.2.0")

Describe the solution you'd like.
After some searching I found following public methods, which are implemented in the Feature-Impl. I would like to make following methods to be publicly available in the Feature-Interface

FeatureImpl.FeatureAccessImpl

        public List<Class<?>> findAnnotatedClasses(Class<? extends Annotation> annotationClass) {
            return imageClassLoader.findAnnotatedClasses(annotationClass, false);
        }

        public List<Method> findAnnotatedMethods(Class<? extends Annotation> annotationClass) {
            return imageClassLoader.findAnnotatedMethods(annotationClass);
        }

        public List<Field> findAnnotatedFields(Class<? extends Annotation> annotationClass) {
            return imageClassLoader.findAnnotatedFields(annotationClass);
        }

Describe who do you think will benefit the most.
Devs, which implement Features based upon certain Annotations

Describe alternatives you've considered.
Accessing the impl classes by casting / reflection does not work due to not being exported

java.lang.IllegalAccessError: class substratevm.features.FeatX (in unnamed module @0x5151a94d) cannot access class com.oracle.svm.hosted.FeatureImpl$FeatureAccessImpl (in module org.graalvm.nativeimage.builder) because module org.graalvm.nativeimage.builder does not export com.oracle.svm.hosted to unnamed module @0x5151a94d
	at substratevm.features.FeatX.duringAnalysis(FeatX.java:17)

An alternative would be using an external library, which seems to work

    ClassLoader applicationClassLoader = access.getApplicationClassLoader();
    try {
      Reflections reflections = new Reflections(
          new ConfigurationBuilder()
              .forPackage("package", applicationClassLoader));
      Set<Class<?>> typesAnnotatedWith = reflections.getTypesAnnotatedWith(Annoclass);

Additional context.
/

Express whether you'd like to help contributing this feature
I could make them public in a backwards compatible way, but it is up to the team whether my proposal has some merits. It seems it was a concise decision to NOT expose it via the interface.

If there is another way to get all classes, fields, methods, which are marked with a Annotation please provide an alternative solution and close this issue.

@kristofdho
Copy link

Something similar is being discussed in this issue: #4862
It could also be considered as part of the move away from the svm jar?
/cc @olpaw

@olpaw
Copy link
Member

olpaw commented Jan 29, 2024

@Berstanio is this issue still relevant. Would native-image support for libgdx benefit from exposing findAnnotated{Classes,Methods,Fields} as public API (lifting the methods to FeatureAccess) ?

@Berstanio
Copy link

Hi @olpaw,
yes, this is still relevant and the public API would help!
In general, we utilise annotations for specific reflection configuration. libGDX can deserialize classes via reflection, and marker annotations can be used to mark a class for deserialization.

On a related note, libGDX also currently depends on the findSubclasses internal API. This is used if base type A can be deserialized, all descendents of type A should be deserializable too.

@kristofdho
Copy link

I thought classpath scanning was going to be removed, which would make these internal calls impossible and they would be removed? And that's why they would not be exposed to the public API.
Has anything changed in this regard?

Our use-case is similar to that of @Berstanio in that we have marker annotations that require reflection configuration, including all subclasses so for us as well, the findSubclasses would be useful.
Currently we're relying on the Reflections library to do this but we'd love to get rid of this dependency.

@olpaw
Copy link
Member

olpaw commented Jan 30, 2024

I thought classpath scanning was going to be removed, which would make these internal calls impossible and they would be removed? And that's why they would not be exposed to the public API.

That is correct. In the long run classpath scanning will be removed and thus we cannot expose findAnnotated{Classes,Methods,Fields}. But we do provide

org.graalvm.nativeimage.AnnotationAccess

which allows users to query annotation information without causing side-effecting changes that impact the analysis results. Together with

org.graalvm.nativeimage.hosted.Feature.FeatureAccess#getApplicationClassPath
org.graalvm.nativeimage.hosted.Feature.FeatureAccess#getApplicationModulePath

feature implementers can implement their own variants of findAnnotated{Classes,Methods,Fields}.

@olpaw olpaw closed this as completed Jan 30, 2024
@olpaw
Copy link
Member

olpaw commented Jan 31, 2024

On a related note, libGDX also currently depends on the findSubclasses internal API. This is used if base type A can be deserialized, all descendents of type A should be deserializable too.

@Berstanio you can replace that with something even better. We have subtype reachability handlers. See:
https://www.graalvm.org/sdk/javadoc/org/graalvm/nativeimage/hosted/Feature.BeforeAnalysisAccess.html
There you find registerSubtypeReachabilityHandler. This is even better because it will only make you register descendants of a given base type as serializable when they are actually detected as reachable by the static analysis.

@Berstanio
Copy link

Berstanio commented Jan 31, 2024

@olpaw

when they are actually detected as reachable by the static analysis.

That is sadly the crucial downside, that makes this unsuitable.

Imagine this case:

class Animation {
    private AnimationTransformer transformer;
}

AnimationTransformer is a abstract base class/or interface, that only defines a method transform(Location x). Now we have a lot of subclasses like LinearTransform or RotationalTransform. If we now want to deserialize a Animation from a file, all the subtypes of AnimationTransformer will not be considered reachable by static analysis at any point, so the deserialization fails.
findSubclasses fixes this issue for us, since it does not filter out "unreachable" classes.

@olpaw
Copy link
Member

olpaw commented Jan 31, 2024

findSubclasses fixes this issue for us, since it does not filter out "unreachable" classes.

I see. The static analysis will not see the Animation subclasses to ever get instantiated because they only come into existence via de-serialization.

Ideally there would it be a static method in libgdx that returns all Animation subclasses. With that, your feature could use it to call org.graalvm.nativeimage.hosted.RuntimeSerialization#register for all those classes. But I see how using findSubclasses is just more convenient for you. cc @christianwimmer

@kristofdho
Copy link

kristofdho commented Jan 31, 2024

Would it be possible to expose these calls anyway, but make it lazilly initialized?
So by default there would be no classpath scanning, so if native-image no longer uses it by default, it's never executed.
But if any features calls one of these calls that does require classpath scanning, only then the scan is executed?

That way we can rely on GraalVM to do the actual scan, and avoid issues with e.g. the AnnotationAccess stuff, because using the Reflections library (or similar libraries) for this will just use default reflection and not the AnnotationAccess provided by native-image to avoid unnecessary side effects.

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

No branches or pull requests

5 participants