Provide GraalVM metadata and substitutions #1357

stIncMale · 2024-04-03T03:39:13Z

I recommend reviewing the first four commits in this PR one at a time, as they are logically independent and don't conflict with each other.

Moved native-image.properties to src/main/resources/META-INF/native-image/native-image.properties. Note how now they are immediately under the native-image folder.
Ensured that MongoClient uses custom com.mongodb.spi.dns.DnsClientProviders in GraalVM via ServiceLoader.
Moved driver-specific reachability metadata from :graalvm-native-image-app into the relevant driver JARs.
Introduced GraalVM substitutions based on MongoClientSubstitutions in Quarkus. The introduced substitutions achieve the following:
4.1. Assert that substitutions are indeed used via Substitutions.assertUsed, which is called from NativeImageApp.
4.2. Explicitly throw at run time if UnixServerAddress is used. It allegedly does not work with GraalVM, we'll figure this out (or not) in JAVA-5400.
4.2. ~~Explicitly throw at run time if the Snappy compressor is used. I don't know why it allegedly does not work with GraalVM, we'll figure this out in JAVA-5383.~~

Note that I omitted some of Quarkus substitutions for various reasons:

We should not substitute ConnectionString.buildCompressors such that it ignores the Snappy compressor. This method calls MongoCompressor.createSnappyCompressor, which is substituted and throws an exception. Throwing an exception should be our reaction to the Snappy compressor in all scenarios.
We should not substitute InternalStreamConnection.createCompressor, as it can never be requested to create a Snappy compressor given that MongoCompressor.createSnappyCompressor is substituted and throws in GraalVM native image. Moreover, the Quarkus substitution makes it always throw, thus breaking all compressors in GraalVM.
There is no need for substitutions related to Snappy compression because it works in GraalVM, provided that the corresponding reachability metadata and dependency is present. We'll add that metadata to our JARs in JAVA-5383.
There is no need for an alias for UnixServerAddress.
Instead of having multiple substitutions related to UnixServerAddress: a substititution for SocketStreamFactory.create, a deletion of UnixSocketChannelStream, a substitution for ServerAddressHelper.createServerAddress, we can substitute only the constructor of UnixServerAddress such that it throws.
We should not have a substitution for com.sun.jna.Native.getWebStartLibraryPath.

JAVA-5219

THe added substitutions are based on https://github.com/quarkusio/quarkus/blob/main/extensions/mongodb-client/runtime/src/main/java/io/quarkus/mongodb/runtime/graal/MongoClientSubstitutions.java JAVA-5219

stIncMale · 2024-04-03T04:05:51Z

driver-core/src/main/com/mongodb/internal/connection/SocketStreamFactory.java

    @Override
    public Stream create(final ServerAddress serverAddress) {
+        return createInternal(serverAddress);


There is no way to call the original implementation of a method that is being substituted. I had to introduce createInternal as a work-around.

Yeah, something you really want this feature.... I follow the same pattern very (too) often.

stIncMale · 2024-04-03T04:08:27Z

driver-core/src/main/com/mongodb/UnixServerAddress.java


 import static com.mongodb.assertions.Assertions.isTrueArgument;
 import static com.mongodb.assertions.Assertions.notNull;

 /**
 * Represents the location of a MongoD unix domain socket.
+ * It is {@linkplain SocketStreamFactorySubstitution#create(ServerAddress) not supported in GraalVM native image}.


Note that because internal classes are excluded from our API docs, this linkplain produces plain text "not supported in GraalVM native image" in the generated API docs. But in an IDE we still have a working link, which is convenient. The same goes for the change in MongoCompressor.

stIncMale · 2024-04-03T04:53:25Z

Hi, @cescoffier. I am unable to add you as a reviewer of this PR, but your review may be helpful. I am inviting you to take a look at this PR if you have time.

stIncMale · 2024-04-03T04:58:22Z

...core/src/main/com/mongodb/internal/graalvm/substitution/SocketStreamFactorySubstitution.java

+    @Substitute
+    public Stream create(final ServerAddress serverAddress) {
+        if (serverAddress instanceof UnixServerAddress) {
+            throw new UnsupportedOperationException("UnixServerAddress is not supported in GraalVM native image");


Assuming UnixServerAddress is not working properly in GraalVM native image, this is not a breaking change. But this assumptions is based solely on the substitutions made in Quarkus. The same is true for MongoCompressorSubstitution.createSnappyCompressor.

So, it might be possible to support it now. These substitutions are kind of old, like 4/5 years old and GraalVM evolved a lot in between.

Note that the class will need to be marked as a runtime only class (cannot be initialized at build time)

Note that the class will need to be marked as a runtime only class (cannot be initialized at build time)

Could you please help me understand why? I made the snappy compressor work without any tricks, just by collecting the reachability metadata with the tracing agent via the org.graalvm.buildtools.native Gradle plugin. And the compression works regardless of whether I specify

--initialize-at-run-time=com.mongodb.internal.connection.SnappyCompressor

or --initialize-at-build-time=com.mongodb.internal.connection.SnappyCompressor

P.S. Given that snappy compression works with GraalVM, I removed snappy-related substitutions.

JAVA-5219

cescoffier

The config makes a lot of sense. There are a few differences with what we do in Quarkus.

My main concern is around JNA/JNI, and the cost in term of image size. Maybe it can be made optional.

cescoffier · 2024-04-05T09:11:41Z

driver-core/src/main/com/mongodb/internal/connection/SocketStreamFactory.java

    @Override
    public Stream create(final ServerAddress serverAddress) {
+        return createInternal(serverAddress);


Yeah, something you really want this feature.... I follow the same pattern very (too) often.

cescoffier · 2024-04-05T09:15:56Z

driver-core/src/main/com/mongodb/internal/graalvm/substitution/MongoCompressorSubstitution.java

+public final class MongoCompressorSubstitution {
+    @Substitute
+    public static MongoCompressor createSnappyCompressor() {
+        throw new UnsupportedOperationException("Snappy compressor is not supported in GraalVM native image");


Not for this PR, but I got Snappy working in native with some nasty work (for kafka and netty). You could reproduce the same kind of thing.

See:

https://github.com/quarkusio/quarkus/blob/main/extensions/kafka-client/deployment/src/main/java/io/quarkus/kafka/client/deployment/KafkaProcessor.java#L282-L308

https://github.com/quarkusio/quarkus/blob/main/extensions/kafka-client/runtime/src/main/java/io/quarkus/kafka/client/runtime/SnappyRecorder.java

https://github.com/quarkusio/quarkus/blob/main/extensions/kafka-client/deployment/src/main/java/io/quarkus/kafka/client/deployment/SnappyUtils.java

cescoffier · 2024-04-05T09:17:23Z

...core/src/main/com/mongodb/internal/graalvm/substitution/SocketStreamFactorySubstitution.java

+    @Substitute
+    public Stream create(final ServerAddress serverAddress) {
+        if (serverAddress instanceof UnixServerAddress) {
+            throw new UnsupportedOperationException("UnixServerAddress is not supported in GraalVM native image");


So, it might be possible to support it now. These substitutions are kind of old, like 4/5 years old and GraalVM evolved a lot in between.

Note that the class will need to be marked as a runtime only class (cannot be initialized at build time)

cescoffier · 2024-04-05T09:18:51Z

driver-core/src/main/resources/META-INF/native-image/jni-config.json

@@ -0,0 +1,22 @@
+[


We do not have these in Quarkus. I need to try with that. It might fix the mongo crypt issues we have in native.

cescoffier · 2024-04-05T09:20:45Z

driver-core/src/main/resources/META-INF/native-image/reflect-config.json

@@ -0,0 +1,96 @@
+[
+{
+  "name":"com.mongodb.BasicDBObject",


We have a slightly different set of reflection registrations. But, I believe yours are more accurate.

driver-core/src/main/resources/META-INF/native-image/resource-config.json

- Improve logging - Use `clean` instead of `:graalvm-native-image-app:clean` in `readme.md` JAVA-5219

Given that the snappy compressor works in GraalVM, we don't need any substitutions for it. JAVA-5219

Having these entries results in a larger native image, even if the shared libraries are not used by the application. Therefore, we won't supply the entries with our JARs, but will document this for users and propose the `resource-config.json` in :graalvm-native-image-app as an example where users may pick the necessary entries from if need be. JAVA-5219

stIncMale · 2024-04-07T09:18:09Z

graalvm-native-image-app/src/main/resources/META-INF/native-image/resource-config.json

Moved entries about the shared libraries before any other entries, anticipating that we'll reference this file from our docs as an example.

…-core` Without this metadata an application using the driver won't work, and there is no other place this metadata may come from, unless it is gathered via the GraalVM tracing agent. It's weird that we have to include reachability metadata specific to JDK, but there is nothing to do about it, other than saying that users are to collect the metadata for their applications themselves. The latter actually seems like the only right way to do this, but it conflicts with the fact that there is Oracle's reachability metadata repository (https://github.com/oracle/graalvm-reachability-metadata). JAVA-5219

Throwing from the constructor of `UnixServerAddress` seems better than from `SocketStreamFactory.create`. Not only this is more natural, but it also prevents the creation of a `MongoClient`, which wasn't he case previously. JAVA-5219

Entries were removed from `graalvm-native-image-app/src/main/resources/META-INF/native-image/reflect-config.json` as `NativeImageApp` works without them JAVA-5219

stIncMale · 2024-04-22T06:39:56Z

...r-core/src/main/com/mongodb/internal/graalvm/substitution/UnixServerAddressSubstitution.java

+public final class UnixServerAddressSubstitution {
+    @Substitute
+    private static void checkNotInGraalVmNativeImage() {
+        throw new UnsupportedOperationException("UnixServerAddress is not supported in GraalVM native image");


This is actually achievable also via org.graalvm.nativeimage.ImageInfo.inImageRuntimeCode instead of the substitutions mechanism. However, I decided to still go with the substitutions

It's more powerful (not strictly so).

It allows us to keep the GraalVM-related code in a separate package, though we still refer to in internal API documentation comments.

bson-record-codec/src/main/resources/META-INF/native-image/reflect-config.json

...r-core/src/main/com/mongodb/internal/graalvm/substitution/UnixServerAddressSubstitution.java

jyemin · 2024-04-24T13:22:36Z

driver-core/src/main/resources/META-INF/native-image/jni-config.json

@@ -0,0 +1,180 @@
+[


This should all be removed now that they are in mongodb-crypt, right?

We have a ticket for that https://jira.mongodb.org/browse/JAVA-5408. But we can do it only when a new version of mongodb-crypt is released, and the driver depends on that new version.

Leaving this unresolved since we agreed to keep this PR open until after mongodb-crypt release

driver-core/src/main/resources/META-INF/native-image/resource-config.json

driver-core/src/main/resources/META-INF/native-image/reflect-config.json

JAVA-5219

Goes to mongodb-crypt from driver-core/src/main/resources/META-INF/native-image/reflect-config.json: { "name":"boolean", "fields":[{"name":"OPTIONS"}, {"name":"STRING_ENCODING"}, {"name":"STRUCTURE_ALIGNMENT"}, {"name":"TYPE_MAPPER"}] }, { "name":"com.sun.crypto.provider.AESCipher$General", "methods":[{"name":"<init>","parameterTypes":[] }] }, { "name":"com.sun.crypto.provider.HmacCore$HmacSHA256", "methods":[{"name":"<init>","parameterTypes":[] }] }, { "name":"com.sun.crypto.provider.HmacCore$HmacSHA512", "methods":[{"name":"<init>","parameterTypes":[] }] }, { "name":"int", "fields":[{"name":"OPTIONS"}, {"name":"STRING_ENCODING"}, {"name":"STRUCTURE_ALIGNMENT"}, {"name":"TYPE_MAPPER"}] }, { "name":"java.lang.Throwable", "methods":[{"name":"addSuppressed","parameterTypes":["java.lang.Throwable"] }] }, { "name":"java.lang.reflect.Method", "methods":[{"name":"isVarArgs","parameterTypes":[] }] }, { "name":"java.nio.Buffer" }, { "name":"long", "fields":[{"name":"OPTIONS"}, {"name":"STRING_ENCODING"}, {"name":"STRUCTURE_ALIGNMENT"}, {"name":"TYPE_MAPPER"}] }, // NativePRNG also stays in driver-core { "name":"sun.security.provider.NativePRNG", "methods":[{"name":"<init>","parameterTypes":[] }, {"name":"<init>","parameterTypes":["java.security.SecureRandomParameters"] }] }, { "name":"sun.security.provider.SHA2$SHA256", "methods":[{"name":"<init>","parameterTypes":[] }] }, { "name":"sun.security.provider.SHA5$SHA512", "methods":[{"name":"<init>","parameterTypes":[] }] }, { "name":"void", "fields":[{"name":"OPTIONS"}, {"name":"STRING_ENCODING"}, {"name":"STRUCTURE_ALIGNMENT"}, {"name":"TYPE_MAPPER"}] }, Goes to mongodb-crypt from graalvm-native-image-app/src/main/resources/META-INF/native-image/reflect-config.json { "name":"org.slf4j.Logger" }, Goes to graalvm-native-image-app/build/native/agent-output/run/reflect-config.json from driver-core/src/main/resources/META-INF/native-image/reflect-config.json { "name":"com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl", "methods":[{"name":"<init>","parameterTypes":[] }] }, { "name":"java.io.FilePermission" }, { "name":"java.lang.RuntimePermission" }, { "name":"java.net.NetPermission" }, { "name":"java.net.SocketPermission" }, { "name":"java.net.URLPermission", "methods":[{"name":"<init>","parameterTypes":["java.lang.String","java.lang.String"] }] }, { "name":"java.security.AllPermission" }, { "name":"java.security.SecurityPermission" }, { "name":"java.util.PropertyPermission" }, { "name":"java.util.concurrent.atomic.AtomicBoolean", "fields":[{"name":"value"}] }, { "name":"java.util.concurrent.atomic.AtomicReference", "fields":[{"name":"value"}] }, { "name":"javax.smartcardio.CardPermission" }, Goes to bson/src/main/resources/META-INF/native-image/reflect-config.json from driver-core/src/main/resources/META-INF/native-image/reflect-config.json { "name":"java.lang.Object", "queryAllDeclaredMethods":true }, Goes to bson/src/main/resources/META-INF/native-image/reflect-config.json from graalvm-native-image-app/src/main/resources/META-INF/native-image/reflect-config.json { "name":"org.slf4j.Logger" }, Goes to driver-core/src/main/resources/META-INF/native-image/reflect-config.json from graalvm-native-image-app/src/main/resources/META-INF/native-image/reflect-config.json { "name":"org.slf4j.Logger" }, JAVA-5219

jyemin · 2024-05-02T17:47:37Z

LGTM except for the one that requires waiting for the next mongodb-crypt release.

jyemin · 2024-05-07T22:19:31Z

Converted to Draft PR while we await the mongodb-crypt release.

stIncMale added 4 commits April 2, 2024 20:40

Simplify the locations of native-image.properties

e249c02

JAVA-5219

Update DnsSpi to check that DnsClientProvider is used

61c0127

JAVA-5219

Move relevant reachability metadata into our JARs

f7ffe8a

JAVA-5219

Add GraalVM substitutions

db998ac

THe added substitutions are based on https://github.com/quarkusio/quarkus/blob/main/extensions/mongodb-client/runtime/src/main/java/io/quarkus/mongodb/runtime/graal/MongoClientSubstitutions.java JAVA-5219

stIncMale self-assigned this Apr 3, 2024

stIncMale commented Apr 3, 2024

View reviewed changes

stIncMale requested a review from jyemin April 3, 2024 04:48

stIncMale commented Apr 3, 2024

View reviewed changes

stIncMale added 2 commits April 3, 2024 08:10

Use parameterized logging when logging an exception

92ac5ae

JAVA-5219

Alleviate the race condition in DnsSpi.useDnsClientProvider

7b10c60

JAVA-5219

cescoffier reviewed Apr 5, 2024

View reviewed changes

stIncMale added 3 commits April 5, 2024 16:22

Do minor improvements

18a1496

- Improve logging - Use `clean` instead of `:graalvm-native-image-app:clean` in `readme.md` JAVA-5219

Remove Snappy-related GraalVM substitutions

261841a

Given that the snappy compressor works in GraalVM, we don't need any substitutions for it. JAVA-5219

stIncMale requested a review from cescoffier April 7, 2024 09:15

stIncMale commented Apr 7, 2024

View reviewed changes

stIncMale added 3 commits April 8, 2024 09:07

Refactor UnixServerAddress substitutions

3929143

Throwing from the constructor of `UnixServerAddress` seems better than from `SocketStreamFactory.create`. Not only this is more natural, but it also prevents the creation of a `MongoClient`, which wasn't he case previously. JAVA-5219

Remove reactor.core/util entries

77cb614

Entries were removed from `graalvm-native-image-app/src/main/resources/META-INF/native-image/reflect-config.json` as `NativeImageApp` works without them JAVA-5219

stIncMale commented Apr 22, 2024

View reviewed changes

jyemin reviewed Apr 24, 2024

View reviewed changes

stIncMale added 2 commits April 30, 2024 19:44

Improve the tracing agent configuration

a244a63

JAVA-5219

stIncMale force-pushed the JAVA-5219 branch from a15d29f to 3bc66d2 Compare May 1, 2024 20:05

stIncMale requested a review from jyemin May 1, 2024 20:38

jyemin marked this pull request as draft May 7, 2024 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide GraalVM metadata and substitutions #1357

Provide GraalVM metadata and substitutions #1357

stIncMale commented Apr 3, 2024 •

edited

stIncMale Apr 3, 2024

cescoffier Apr 5, 2024

stIncMale Apr 3, 2024

stIncMale commented Apr 3, 2024

stIncMale Apr 3, 2024 •

edited

cescoffier Apr 5, 2024

stIncMale Apr 7, 2024 •

edited

cescoffier left a comment

cescoffier Apr 5, 2024

cescoffier Apr 5, 2024

cescoffier Apr 5, 2024

cescoffier Apr 5, 2024

cescoffier Apr 5, 2024

stIncMale Apr 7, 2024

stIncMale Apr 22, 2024

jyemin Apr 24, 2024

stIncMale Apr 30, 2024

jyemin May 2, 2024

jyemin commented May 2, 2024

jyemin commented May 7, 2024

Provide GraalVM metadata and substitutions #1357

Are you sure you want to change the base?

Provide GraalVM metadata and substitutions #1357

Conversation

stIncMale commented Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stIncMale commented Apr 3, 2024

stIncMale Apr 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stIncMale Apr 7, 2024 • edited

Choose a reason for hiding this comment

cescoffier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyemin commented May 2, 2024

jyemin commented May 7, 2024

stIncMale commented Apr 3, 2024 •

edited

stIncMale Apr 3, 2024 •

edited

stIncMale Apr 7, 2024 •

edited