Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Provide GraalVM metadata and substitutions #1357
Provide GraalVM metadata and substitutions #1357
Changes from 4 commits
e249c02
61c0127
f7ffe8a
db998ac
92ac5ae
7b10c60
18a1496
261841a
fe51f0e
7c8b08a
3929143
77cb614
a244a63
3bc66d2
be64442
faaba04
841f67b
c3ab8da
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 inMongoCompressor
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, something you really want this feature.... I follow the same pattern very (too) often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 forMongoCompressorSubstitution.createSnappyCompressor
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
--initialize-at-build-time=com.mongodb.internal.connection.SnappyCompressor
P.S. Given that snappy compression works with GraalVM, I removed snappy-related substitutions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not have these in Quarkus. I need to try with that. It might fix the mongo crypt issues we have in native.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should all be removed now that they are in mongodb-crypt, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving this unresolved since we agreed to keep this PR open until after mongodb-crypt release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a slightly different set of reflection registrations. But, I believe yours are more accurate.