Integrate Native Image API changes in GraalVM 22.2+

[galderz]

Description

Adjust Quarkus to upcoming Native Image API changes. An initial discussion happened during the Native Image Committer Community Meeting on 2nd of June.

Analysis

The following required changes were noted during the discussion:

• [x] Switch from AutomaticFeature to Feature and then pass in features in use via --feature.
• [ ] AlwaysInline won't be exposed in the API so we should stop using it. io/quarkus/runtime/graal/ConfigurationSubstitutions and io/quarkus/jdbc/h2/runtime/graal/RemoteOnly are the only remaining users.
• [ ] -H: options are not public API. During the meeting @christianwimmer said some of those should become API (e.g. --blah option) and others might no longer be used (e.g. We should stop disabling ParseOnce with 22.2 because memory footprint there has been improved, see https://github.com/quarkusio/quarkus/issues/25944 for more details). He said he'd investigate Quarkus' use of those are create an issue with recommendations.

FYI @zakkak @jerboaa @n1hility @Sanne @geoand

[jerboaa]

There are two cases of internal API usage which recently popped up breaking a potential future API contract (worked-around for now by this PR):

1. org.graalvm.nativeimage.impl.ConfigurationCondition: Used here, for example: https://github.com/quarkusio/quarkus/blob/5f27683e98e4f17c83d12eb75fa9e8adc352cc1b/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageFeatureStep.java#L300
2. org.graalvm.nativeimage.impl.RuntimeSerializationSupport: Used here, for example: https://github.com/quarkusio/quarkus/blob/5f27683e98e4f17c83d12eb75fa9e8adc352cc1b/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageFeatureStep.java#L163

[galderz]

There are two cases of internal API usage which recently popped up breaking a potential future API contract (worked-around for now by this PR):

1. org.graalvm.nativeimage.impl.ConfigurationCondition: Used here, for example: https://github.com/quarkusio/quarkus/blob/5f27683e98e4f17c83d12eb75fa9e8adc352cc1b/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageFeatureStep.java#L300

Unless I'm mistaken, ConfigurationCondition was first introduced by @zakkak in this commit to support resource registration.

2. org.graalvm.nativeimage.impl.RuntimeSerializationSupport: Used here, for example: https://github.com/quarkusio/quarkus/blob/5f27683e98e4f17c83d12eb75fa9e8adc352cc1b/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageFeatureStep.java#L163

Yeah, that's used to enable registering lambdas for serialization.

I'll investigate these 2 use case to see if there are ways to do the same via public APIs, otherwise I'll create issue(s) in oracle/graal.

[galderz]

I think these ^ fall within the "JNI / Resource / Proxy / Serialization registration" section in the discussion. A public API version to register resources, serialization and others are required. The discussion already highlights this issue, so it should be solved with the API work.

[jerboaa]

https://github.com/oracle/graal/pull/4783 has some more details about what replacements are for some currently internal API usage. Particularly: ReflectionUtil (replace with own) => https://github.com/oracle/graal/pull/4783#issuecomment-1208138188 LocalizationFeature => https://github.com/oracle/graal/pull/4783#issuecomment-1208160638

[jerboaa]

The annotations (except AlwaysInline as promised) will move to a different artifact (sdk), but remain in the same package: https://github.com/oracle/graal/pull/4683

[olpaw]

We should stop disabling ParseOnce

@jerboaa, @galderz while at it you could also remove -H:+JNI since this is always enabled anyway (since Oct 10, 2018, see https://github.com/oracle/graal/commit/6346801d0b129355dfd95a8f7cb112ed0a8155a4)

[zakkak]

@jerboaa, @galderz while at it you could also remove -H:+JNI since this is always enabled anyway (since Oct 10, 2018, see oracle/graal@6346801)

Thanks for pointing this out @olpaw. https://github.com/quarkusio/quarkus/pull/27267 takes care of it.

[galderz]

Re: ParseOnce. At the end of June I carried some tests to verify its effects on 22.2 and indeed the memory usage had improved, so it no longer resulted in a max RSS penalty. So I think it's safe to remove disabling it.

[zakkak]

Re: ParseOnce. At the end of June I carried some tests to verify its effects on 22.2 and indeed the memory usage had improved, so it no longer resulted in a max RSS penalty. So I think it's safe to remove disabling it.

FYI I replied to this in https://github.com/quarkusio/quarkus/issues/25944#issuecomment-1217771007 to move the discussion under the more specific issue.

[jerboaa]

With #27960 there would be no more usage of @NeverInline in Quarkus as far as I can see. As far as @AlwaysInline is concerned, I see these remaining usages:

$ grep -rn '@AlwaysInline'
extensions/jdbc/jdbc-mssql/runtime/src/main/java/io/quarkus/jdbc/mssql/runtime/graal/com/microsoft/sqlserver/jdbc/SQLServerJDBCSubstitutions.java:94:    @AlwaysInline("We need this to be constant folded")
extensions/jdbc/jdbc-h2/runtime/src/main/java/io/quarkus/jdbc/h2/runtime/graal/RemoteOnly.java:17:    @AlwaysInline("Method org.h2.engine.SessionRemote.connectEmbeddedOrServer must be able to realize it's only ever going remote")
core/runtime/src/main/java/io/quarkus/runtime/graal/ConfigurationSubstitutions.java:26:    @AlwaysInline("trivial")

[Sanne]

@dmlloyd could you comment in regards to Target_io_smallrye_config_SmallRyeConfigProviderResolver#getConfig ? Assuming tests pass w/o the @AlwaysInline("trivial") can we remove it safely?

[dmlloyd]

Yes, the semantics would not change so it is okay to remove.

[Sanne]

@galderz is it worth to work on "-H: options are not public API" in this cycle? As I commented above I see no big value in it - especially no need to rush it. I certainly don't mind if you all want to keep trying that but I'm wondering if it could be broken out into a separate issue, so to allow deferring it, or at least prioritize differently.

[jerboaa]

@galderz is it worth to work on "-H: options are not public API" in this cycle? As I commented above I see no big value in it - especially no need to rush it. I certainly don't mind if you all want to keep trying that but I'm wondering if it could be broken out into a separate issue, so to allow deferring it, or at least prioritize differently.

It's not very realistic that this can be completed in it's entirety. For example -H:BuildOutputJSONFile is a new option in 22.3. Also some of the diagnostic ones are unlikely to get public equivalents. So +1 on this being best effort.

[zakkak]

+1 IMHO we should just try and minimize the use of -H: options and more importantly avoid introducing new Quarkus options mapping 1to1 to -H: options. We should instead mention such options in the documentation where needed and pass them through quarkus.native.additional-build-args.

[galderz]

I'll split the -H: part into a different issue.

[galderz]

I've split -H: work into https://github.com/quarkusio/quarkus/issues/28148. We might need individual issues in the future to address individual -H: options, rather than a single issue. Feel free to close the new issue if needed.

[gsmet]

I think this has been already handled. If there is remaining work, better open new issues as title would be misleading.