-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Initialize ObjectId, ClientMetadataHelper at runtime, GraalVM #1078
Conversation
@@ -13,4 +13,4 @@ | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
# | |||
Args = --initialize-at-run-time=com.mongodb.UnixServerAddress,com.mongodb.internal.connection.SnappyCompressor |
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.
I saw that we had JAVA-3580 open to test UnixServerAddress and SnappyCompressor, which were proposed in #527. I do not know how we might test these, since the original had no reproducer. I do not understand why these were originally added, since they seem to never have had statics (initialized via --initialize-at-build-time
). Either these were never needed, or there is some other form of initialization we should check our codebase for. Any ideas?
In any case, my testing shows that classes are being picked up from native-image.properties
, so I think we should consider these sufficiently tested.
@@ -13,4 +13,4 @@ | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
# | |||
Args = --initialize-at-run-time=com.mongodb.UnixServerAddress,com.mongodb.internal.connection.SnappyCompressor | |||
Args = --initialize-at-run-time=com.mongodb.UnixServerAddress,com.mongodb.internal.connection.SnappyCompressor,org.bson.types.ObjectId,com.mongodb.internal.connection.ClientMetadataHelper |
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.
I'm a bit surprised this works because ObjectId is in the bson jar not driver-core, but I guess the native image compiler doesn't care.
But for anyone that is using the bson library directly, it seems like this more properly belongs in the bson jar file.
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.
I added ObjectId to bson. Tested by replacing mongodb-driver-sync with bson as a dependency, and confirming that the oid did not change between runs, and then changed after the fix.
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 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.
Regarding resources? Seems we adopted in #527 a non-typical structure for resources in driver-core: src/resources
as opposed to the default location: src/main/resources
https://docs.gradle.org/current/userguide/java_plugin.html
@katcharov used the default structure so no build changes needed.
Let's take the opportunity here to make them consistent. If there's no reason for the non-standard location of the driver-core resources, we can move that one to the default location. |
Moved resources, and repeated all prior manual tests. |
JAVA-4856
also:
JAVA-3580
JAVA-4864
Manually tested, via something like:
Running the jar, and then two runs of the native image:
Generates the same oid for the latter two, and reports GraalVM.
After changes:
oids are different, and the platform is no longer GraalVM.