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

Enable github-api to support GraalVM native images #1908

Closed
klopfdreh opened this issue Aug 15, 2024 · 18 comments
Closed

Enable github-api to support GraalVM native images #1908

klopfdreh opened this issue Aug 15, 2024 · 18 comments

Comments

@klopfdreh
Copy link
Contributor

Describe the bug
Due to some reflections you encounter errors during the runtime when github-api is used in a native image.

Example

at java.base@22.0.1/java.lang.invoke.LambdaForm$DMH/sa346b79c.invokeStaticInit(LambdaForm$DMH)\nCaused by: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `org.kohsuke.github.GHRepository`: cannot deserialize from Object value (no delegate- or property-based Creator): this appears to be a native image, in which case you may need to configure reflection for the class that is to be deserialized\n at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 2]

To Reproduce
Steps to reproduce the behavior:

  1. Build an application with Spring Boot Native and github-api
  2. Perform a native-image build
  3. Run the native application

Expected behavior
github-api should be used in a native image without any issues

Desktop (please complete the following information):
N/A

Additional context
You could add META-INF/native-image/<groupid>/<artifactid>/reflect-config.json and describe the reflection usage:

Example:

[
  {
    "name": "org.kohsuke.github.GHRepository",
    <settings for reflections>
  }
]
@klopfdreh
Copy link
Contributor Author

klopfdreh commented Aug 15, 2024

I saw a lot of other issues which are caused by the heavy usage of reflections. When I add all classes which are instantiated with reflections, like explained in the issue, I received the following error

java.lang.NullPointerException: The root GitHub reference for this instance is null. Probably caused by deserializing this class without using a GitHub instance. If you must do this, use the MappingObjectReader from GitHub.getMappingObjectReader().

Edit: I created a demo project for showcase: https://github.com/klopfdreh/github-api-native-test

Here you can run the GitHubApiNativeTestApplication in Java and everything is working fine.

When you perform a native image build (like explained in the README.md) the error above occurs.

All images are arm based - if you want to try it on x86_64 - you have to change the docker image and the liberica NIK which is downloaded - see Dockerfile.

@bitwiseman
Copy link
Member

@klopfdreh
I don't have a lot of time to commit to this project currently.

I've created a PR to create a v2.x That removes Java 8 and most of the reflection from this project. But it is already out of date and needs more attention to complete.

PRs welcome, but sounds like the changes you're asking if I would be breaking changes. So we might want to include the 2.x effort.

@klopfdreh
Copy link
Contributor Author

Hey @bitwiseman - yes it seems to be better in 2.x then.

I think it all narrows down to the way objects are read and the root value is passed into the newly created objects with InjectableValues.

Sadly I just came across this issue as we are moving all of our projects to Spring Boot Native and one of it is using this GitHub-API.

I don’t know if I have the time to fix this as we have a lot of other projects being ported.

I just created a demo project which can be used to test native image.

When there will be a near final version of 2.x then let me know - I could test it for you.

@gsmet
Copy link
Contributor

gsmet commented Aug 19, 2024

FWIW, we are heavily using this API in Quarkus projects and you might find some inspiration here:
https://github.com/quarkiverse/quarkus-github-api/blob/main/deployment/src/main/java/io/quarkiverse/githubapi/deployment/GithubApiProcessor.java#L38-L52
and here:
https://github.com/quarkiverse/quarkus-github-api/blob/main/deployment/src/main/java/io/quarkiverse/githubapi/deployment/GitHubApiDotNames.java
(yeah sorry for all the constants, it was a bad idea, I need to drop them and just initialize the lists)

You could probably use this work to generate a native-image reflection config file.

In any case, given Jackson is heavily reflection-based (as all the general purpose JSON library out there), you will end up with some reflection.
What could be a good idea for 2 is to have a marker annotation for JSON objects and use a small annotation processor to generate the native-image file.

@klopfdreh
Copy link
Contributor Author

FWIW, we are heavily using this API in Quarkus projects and you might find some inspiration here: https://github.com/quarkiverse/quarkus-github-api/blob/main/deployment/src/main/java/io/quarkiverse/githubapi/deployment/GithubApiProcessor.java#L38-L52 and here: https://github.com/quarkiverse/quarkus-github-api/blob/main/deployment/src/main/java/io/quarkiverse/githubapi/deployment/GitHubApiDotNames.java (yeah sorry for all the constants, it was a bad idea, I need to drop them and just initialize the lists)

You could probably use this work to generate a native-image reflection config file.

In any case, given Jackson is heavily reflection-based (as all the general purpose JSON library out there), you will end up with some reflection. What could be a good idea for 2 is to have a marker annotation for JSON objects and use a small annotation processor to generate the native-image file.

Hey - thanks a lot for the heads up! Is there anyway to translate this into something which can be used only by GraalVM and does not rely ony Quarkus / JBoss API? If yes we could provide it here to enable the project for native images in a more generic way.

@gsmet
Copy link
Contributor

gsmet commented Aug 19, 2024

Basically, you need to provide a reflection config file in JSON that lists all the reflection that is needed.

That's what Quarkus does from the classes I pointed out.

You could probably script something that generates most of the JSON file from what I pointed out. Again, with an annotation and an annotation processor, things would be a lot easier.

I adjust this code every time there is a release to make sure everything is listed (I'm a couple of micros late).

@klopfdreh
Copy link
Contributor Author

klopfdreh commented Aug 19, 2024

Thank you for the explanation. I got it working with a RuntimeHintsRegistrar for Spring Boot Native right now. 👍

See last commit: klopfdreh/github-api-native-test@8c50f6c

@bitwiseman what we could do to make this work for Spring Boot Native:

  1. Add a file META-INF/spring/aot-factories with the content
org.springframework.aot.hint.RuntimeHintsRegistrar=org.kohsuke.github.aot.GitHubRuntimeHints
  1. Add the GitHubRuntimeHints to org.kohsuke.github.aot with the following content
package org.kohsuke.github.aot;

import org.springframework.aot.hint.MemberCategory;
import org.springframework.aot.hint.RuntimeHints;
import org.springframework.aot.hint.RuntimeHintsRegistrar;
import org.springframework.aot.hint.TypeReference;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.util.Arrays;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;
import java.util.logging.Level;
import java.util.logging.Logger;

public class GitHubRuntimeHints implements RuntimeHintsRegistrar {

    private static final Logger LOGGER = Logger.getLogger(GitHubRuntimeHints.class.getName());

    @Override
    public void registerHints(RuntimeHints hints, ClassLoader classLoader) {
        Arrays.stream(System.getProperty("java.class.path").split(File.pathSeparator)).forEach(classpathEntry -> {
            // If the classpathEntry is no jar skip it
            if (!classpathEntry.endsWith(".jar")) {
                return;
            }

            try (JarInputStream is = new JarInputStream(new FileInputStream(classpathEntry))) {
                JarEntry entry;
                while ((entry = is.getNextJarEntry()) != null) {
                    if (entry.getName().endsWith(".class") && entry.getName().startsWith("org/kohsuke/github")) {
                        String githubApiClassName = entry.getName().replace("/", ".");
                        String githubApiClassNameWithoutClass = githubApiClassName.substring(0, githubApiClassName.length() - 6);
                        LOGGER.log(Level.INFO, "Registered class " + githubApiClassNameWithoutClass + " for reflections and serialization.");
                        hints.reflection().registerType(TypeReference.of(githubApiClassNameWithoutClass), MemberCategory.values());
                        hints.serialization().registerType(TypeReference.of(githubApiClassNameWithoutClass));
                    }
                }
            } catch (IOException e) {
                LOGGER.log(Level.INFO, "Error while reading jars", e);
            }
        });
    }
}

When ever GitHub-API is used as jar in a Spring Boot Native project it should be caught up automatically all serialization and reflection hints during the aot-processing. I think the spring dependency could be introduced as optional as this RuntimeHintsRegistrar is only executed when used within a Spring project - for logging I used java.util.logging.

See like it works in a Spring Project (In this case in Spring Batch):

https://github.com/spring-projects/spring-batch/blob/main/spring-batch-core/src/main/java/org/springframework/batch/core/aot/CoreRuntimeHints.java

https://github.com/spring-projects/spring-batch/blob/main/spring-batch-core/src/main/resources/META-INF/spring/aot.factories

What I did to make this future proof is to iterate through all classes and get all in the org/kohsuke/github package. So everything in there is handled like in Java in which you can access all with reflections.

@klopfdreh
Copy link
Contributor Author

@bitwiseman - if I should I create a PR let me know. 👍

@gsmet
Copy link
Contributor

gsmet commented Aug 19, 2024

I think it would need to be a separate artifact. Having a dependency on Spring in such a low level library looks odd.

@klopfdreh
Copy link
Contributor Author

I think it would need to be a separate artifact. Having a dependency on Spring in such a low level library looks odd.

So you suggest to add a new repository to hub4j called „github-api-spring-boot“ and if you want to build native applications you need to add both of them? (I let the native information out of the name as this dependency may also provide other Spring related integrations)

Would be fine for me, too. 👍

@gsmet
Copy link
Contributor

gsmet commented Aug 19, 2024

It’s really for @bitwiseman to decide :)

I know that for Quarkus, we are dealing with this in extensions and we avoid cluttering the upstream projects.

I would have a similar approach here.

@gsmet
Copy link
Contributor

gsmet commented Aug 19, 2024

Note that on the opposite side, if we go the low level GraalVM metadata route, it could make sense to include them upstream.

@klopfdreh
Copy link
Contributor Author

It’s really for @bitwiseman to decide :)

I know that for Quarkus, we are dealing with this in extensions and we avoid cluttering the upstream projects.

I would have a similar approach here.

Yes that’s true, but it is very nice that you share your experiences here to find the right way to go. 😃

@klopfdreh
Copy link
Contributor Author

Note that on the opposite side, if we go the low level GraalVM metadata route, it could make sense to include them upstream.

I think this depends on how often new classes which require reflections are introduced.

As there will be a 2.x soon it might be more future proof to introduce projects for Spring / Quarkus - but let’s see how @bitwiseman decides. 👍

@bitwiseman
Copy link
Member

I currently have very little time to devote to this project. I would rather not fork to a separate repository - but creating a separate release branch that generates a separate artifact is relatively low cost.

See https://github.com/hub4j/github-api/tree/release/v1.x-unbridged

If there's a way to do this without adding a SpringBoot dependency that would be better.

@klopfdreh klopfdreh mentioned this issue Aug 23, 2024
11 tasks
@klopfdreh
Copy link
Contributor Author

klopfdreh commented Aug 23, 2024

@bitwiseman in this case there is no need for a separate release branch. I created a PR which covers all reflection and serialization hints generated by the RuntimeHintsRegistrar I posted above.

I also double checked if there is no new class requires reflection hints in the main branch compared to the latest release.

@bitwiseman
Copy link
Member

bitwiseman commented Aug 27, 2024

For reference/thought:

As noted by @gsmet, Quarkus goes the route of manually registering classes for reflection.
https://quarkus.io/guides/writing-native-applications-tips#registering-for-reflection

The GraalVM docs discuss how to generate these files based on tracing:
https://www.graalvm.org/latest/reference-manual/native-image/metadata/AutomaticMetadataCollection/

Since we enforce code coverage on our data classes, it might be possible to generate these files based on execution of all the tests.

But I think I've made reasonable suggestion for how to make #1914 a workable solution without either of the above.

@klopfdreh
Copy link
Contributor Author

In the PR a suggestion has been implemented. It is without generate reachable meta data, but provides a way to add GraalVM without Spring / Quarkus classes during runtime.

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

3 participants