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

GraalVM compatibility #936

Open
mrniko opened this issue Jan 23, 2023 · 13 comments
Open

GraalVM compatibility #936

mrniko opened this issue Jan 23, 2023 · 13 comments

Comments

@mrniko
Copy link

mrniko commented Jan 23, 2023

Is your feature request related to a problem? Please describe.
Make Kryo lib GraalVM compatible

Describe the solution you'd like
Some of serializers can't be used in GraalVM - redisson/redisson#4811. They needs to be added in reflect-config.json file stored in library.

Describe alternatives you've considered
I'm adding it manually in my project now

@theigl
Copy link
Collaborator

theigl commented Jan 23, 2023

@mrniko: My understanding of GraalVM requirements is limited. What serializers would we need to add to the reflect-config.json? All of them?

The issue you linked mentions OffsetDateTimeSerializer, but I don't see how this serializer is different from any other.

@mrniko
Copy link
Author

mrniko commented Jan 23, 2023

It's arise if you use OffsetDateTime object. But you use any other object like Duration, Instant... you'll get similar error. I think all Serializers which extends ImmutableSerializer and located in TimeSerializers should be added.

@mrniko
Copy link
Author

mrniko commented Jan 23, 2023

I attached the sample project. You can use it to reproduce issue with other serializers. Read Readme.md for the build steps. Please note that OffsetDateTimeSerializer already added into \src\main\resources\META-INF\native-image\com.example\micronaut-demo-graalvm\reflect-config.json file.

micronaut-demo-graalvm-master2.zip

@mrniko
Copy link
Author

mrniko commented Jan 23, 2023

It seems all Serializers extending ImmutableSerializer should be added, not only in TimeSerializers class

@mrniko
Copy link
Author

mrniko commented Jan 24, 2023

I attached reflect-config.json file with all Serializers

reflect-config.json.zip

@theigl
Copy link
Collaborator

theigl commented Jan 24, 2023

@mrniko: Can you please create a PR with your suggested changes? Then we can discuss this further.

We could add some conditional declarations for serializers: RecordSerializer only makes sense for JDK17+, ImmutableCollectionsSerializers is for JDK11+. I'm not sure this is necessary, but the GraalVM docs recommend it.

@mrniko
Copy link
Author

mrniko commented Feb 8, 2023

Here is another issue with GraalVM redisson/redisson#4820

@theigl
Copy link
Collaborator

theigl commented Feb 8, 2023

@mrniko: We have two options here:

  1. Disable unsafe on GraalVM with -Dkryo.unsafe=false.
  2. Change the code in Kryo to use direct assignment to static final fields, so GraalVM can perform the substitutions (see https://developers.redhat.com/articles/2022/05/09/using-unsafe-safely-graalvm-native-image#how_to_fix_unsafe_offset_computations)

I don't use GraalVM in my dayjob, so I can't spend a lot of time on this issue.

If you can provide me with a very simple example project that uses GraalVM + Kryo where I can reproduce these issues, I will attempt to fix them. But it should be a project without Redisson, Quarkus etc. Ideally, just a class that initializes Kryo and does some serialization.

@theigl
Copy link
Collaborator

theigl commented Feb 8, 2023

@CGDogan
Copy link

CGDogan commented Jul 17, 2023

@mrniko thank you for your reflection-config.json, the project we use uses three serializers only so learning the correct format for listing those from your upload, namely allDeclaredConstructors, we're able to use kryo

@mrniko
Copy link
Author

mrniko commented Jul 17, 2023

@CGDogan

You don't need to add reflection-config.json since it's part of Redisson.

@CGDogan
Copy link

CGDogan commented Jul 17, 2023

No no, you helped me write as much as we needed to work with kryo and I'm not using Redisson:

{
  "name": "com.esotericsoftware.kryo.serializers.DefaultSerializers$ClassSerializer",
  "allDeclaredConstructors": true
},
{
  "name": "com.esotericsoftware.kryo.serializers.DefaultSerializers$IntSerializer",
  "allDeclaredConstructors": true
},
{
  "name": "com.esotericsoftware.kryo.serializers.DefaultSerializers$StringSerializer",
  "allDeclaredConstructors": true
},

@CGDogan
Copy link

CGDogan commented Jul 17, 2023

Sorry, Kryo makes classes by loading their bytecode, but this will work iff a generic bytecode is loaded for the class using experimental-class-define-support and then fields are initialized. All loaded bytecode must be known before compiletime hence only "templates" can be loaded. This is how Kryo could work but yes it's a lot of work. For now, it does not I think.

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

No branches or pull requests

3 participants