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

Add native-image support #510

Merged
merged 5 commits into from
May 30, 2023
Merged

Add native-image support #510

merged 5 commits into from
May 30, 2023

Conversation

violetagg
Copy link
Member

Motivation:

In order to support native-image, the project has to provide the corresponding configuration for reflection, JNI and resources.

Modifications:

  • Module codec-native-quic: Add profile native-image-agent for generating native-image configuration
  • New module testsuite-native-image: Add profiles native-image-quic-server/native-image-quic-client for testing the native-image configuration

Result:
Fixes #464

@violetagg
Copy link
Member Author

I would like to outline the following:

  • We need to exclude the new module testsuite-native-image from the deployment
  • I think Graal still does not support Apple M1 because of that I haven't added any configuration about it Support for Apple M1 (darwin-aarch64) oracle/graal#2666
  • It will be good if we can enable a regular check on the CI (similar to what we have in Netty)

@violetagg
Copy link
Member Author

@normanmaurer @gradinac PTAL 🙏

@normanmaurer normanmaurer added this to the 0.0.43.Final milestone May 23, 2023
@normanmaurer
Copy link
Member

@violetagg I wonder if there is a way to generate these files ? Otherwise I think there is a risk we break things again in the future .

@violetagg
Copy link
Member Author

@violetagg I wonder if there is a way to generate these files ? Otherwise I think there is a risk we break things again in the future .

@normanmaurer they are generated with ./mvnw -Pnative-image-agent -pl codec-native-quic test. I did that on my Mac OS. The generated files are in codec-native-quic/target/native/agent-output/test

import java.net.InetSocketAddress;
import java.util.concurrent.TimeUnit;

// The class is copied from "codec-native-quic" module
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we couldn't somehow do this as part of the build so we not have to duplicate code . Same for above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah let me check this one

testsuite-native-image/pom.xml Outdated Show resolved Hide resolved
violetagg added 4 commits May 29, 2023 11:14
Motivation:

In order to support `native-image`, the project has to provide the corresponding configuration
for reflection, JNI and resources.

Modifications:
- Module `codec-native-quic`: Add profile `native-image-agent` for generating `native-image` configuration
- New module `testsuite-native-image`: Add profiles `native-image-quic-server`/`native-image-quic-client` for testing the `native-image` configuration

Result:
Fixes #464
@violetagg
Copy link
Member Author

@normanmaurer PTAL

Comment on lines +4 to +7
{
"condition":{"typeReachable":"io.netty.incubator.codec.quic.Quiche"},
"pattern":"\\QMETA-INF/native/libnetty_quiche_osx_x86_64.jnilib\\E"
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{
"condition":{"typeReachable":"io.netty.incubator.codec.quic.Quiche"},
"pattern":"\\QMETA-INF/native/libnetty_quiche_osx_x86_64.jnilib\\E"
},
{
"condition":{"typeReachable":"io.netty.incubator.codec.quic.Quiche"},
"pattern":"\\QMETA-INF/native/libnetty_quiche_osx_x86_64.jnilib\\E"
},
{
"condition":{"typeReachable":"io.netty.incubator.codec.quic.Quiche"},
"pattern":"\\QMETA-INF/native/libnetty_quiche_osx_aarch_64.jnilib\\E"
},

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work only when GraalVM supports it oracle/graal#2666

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment for this...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I will add the resource so that we don't need to add it later and in addition I'll add a comment that this is still not supported.

{
"condition":{"typeReachable":"io.netty.incubator.codec.quic.Quiche"},
"pattern":"\\QMETA-INF/native/libnetty_quiche_osx_aarch_64.jnilib\\E",
"//note": "Apple M1 is still not supported https://github.com/oracle/graal/issues/2666"
Copy link
Member Author

@violetagg violetagg May 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GraalVM will ignore this note

Warning: Unknown attribute(s) [//note] in resource and resource bundle descriptor object

@normanmaurer normanmaurer merged commit 7dec238 into netty:main May 30, 2023
@normanmaurer
Copy link
Member

@violetagg thanks a lot! You want to do another PR to run the test suite regularly as well ?

@violetagg
Copy link
Member Author

@violetagg thanks a lot! You want to do another PR to run the test suite regularly as well ?

sure

@violetagg violetagg deleted the native-image-support branch May 30, 2023 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants