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

Initialize ObjectId, ClientMetadataHelper at runtime, GraalVM #1078

Merged
merged 3 commits into from
Feb 6, 2023

Conversation

katcharov
Copy link
Contributor

JAVA-4856

also:

JAVA-3580
JAVA-4864

Manually tested, via something like:

        System.out.println("oid: " + new ObjectId(Date.from(Instant.ofEpochSecond(99, 99))));
        System.out.println(ClientMetadataHelper.CLIENT_METADATA_DOCUMENT);

Running the jar, and then two runs of the native image:

oid: 0000006343ef8f0fabeae35b
{"driver": ...
=====
oid: 00000063886af5402f11180f
{"driver": ... "platform": "Java/GraalVM Community/17.0.6+10-jvmci-22.3-b13"}
=====
oid: 00000063886af5402f11180f
{"driver": ... "platform": "Java/GraalVM Community/17.0.6+10-jvmci-22.3-b13"}

Generates the same oid for the latter two, and reports GraalVM.

After changes:

oid: 00000063113a125a8556fcb1
{"driver": ...
=====
oid: 0000006341ef117b012d53d8
{"driver": ... "platform": "Java/Oracle Corporation/unknown-version"}
=====
oid: 00000063b0e5d506f6beef95
{"driver": ... "platform": "Java/Oracle Corporation/unknown-version"}

oids are different, and the platform is no longer GraalVM.

@@ -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
Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at 9c3d626. I'm surprised this works without the same change to bson/build.gradle as was made to driver-core/build.gradle in that commit. Any ideas, @rozza ?

Copy link
Member

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.

@katcharov katcharov requested a review from jyemin February 3, 2023 23:46
@jyemin
Copy link
Contributor

jyemin commented Feb 6, 2023

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.

@katcharov
Copy link
Contributor Author

Moved resources, and repeated all prior manual tests.

@katcharov katcharov merged commit 493b248 into mongodb:master Feb 6, 2023
@katcharov katcharov deleted the JAVA-4856 branch February 6, 2023 19:54
ispringer pushed a commit to evergage/mongo-java-driver that referenced this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants