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.

Sign up for GitHub

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

chore: prepare showcase for Graal SDK 23.x #3574

Merged
merged 6 commits into from
Jan 24, 2025
Merged

chore: prepare showcase for Graal SDK 23.x #3574

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0796cda
chore: prepare showcase for Graal SDK 23.x
diegomarquezp Jan 20, 2025
d48ecf9
move config to gax module
diegomarquezp Jan 21, 2025
81c1a27
correct whitespace in native properties of gax
diegomarquezp Jan 21, 2025
98eaf9f
add reflection config for java.time.Instant
diegomarquezp Jan 23, 2025
7fd94d9
restore config
diegomarquezp Jan 23, 2025
0c0ac6d
Merge branch 'main' into showcase-graal-23
diegomarquezp Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .../main/resources/META-INF/native-image/com.google.showcase.v1beta1/native-image.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Args = --initialize-at-build-time=io.grpc.internal.TimeProvider,\
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know why they have to be added? Are they showcase specific or we have to add them to common modules like Gax as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's definitely at the grpc level. I'll test with another library to see if it happens and possible move this config to gax.

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 obtained this from trying the native tests in java-firestore

JUnit Vintage:SpannerClientTest
    ClassSource [className = 'com.google.cloud.spanner.v1.SpannerClientTest', filePosition = null]
    => org.opentest4j.MultipleFailuresError: Multiple Failures (2 failures)
        java.lang.NoClassDefFoundError: Could not initialize class io.grpc.internal.TimeProvider
        java.lang.NullPointerException: <no message>
         io.grpc.internal.CallTracer$1.create(CallTracer.java:74)
         io.grpc.internal.ServerImpl.<init>(ServerImpl.java:164)
         io.grpc.internal.ServerImplBuilder.build(ServerImplBuilder.java:245)
         io.grpc.ForwardingServerBuilder.build(ForwardingServerBuilder.java:201)
         com.google.api.gax.grpc.testing.MockServiceHelper.<init>(MockServiceHelper.java:62)
         [...]
         com.google.cloud.spanner.v1.SpannerClientTest.stopServer(SpannerClientTest.java:112)
         java.base@23.0.1/java.lang.reflect.Method.invoke(Method.java:580)

I'll move this to gax

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

Choose a reason for hiding this comment

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

So if I understand correctly, TimeProvider and InstantTimeProvider are now used during client initialization by grpc-java, hence we have to add them to native-image.properties? This is not due to GraalVM upgrade but rather a grpc upgrade?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, Diego! I think the solution makes sense.
However, looking at the change in grpc, we should've run into this error since two months ago, do we know why only GraalSDK 23.x triggered this error?

Copy link
Contributor Author

@diegomarquezp diegomarquezp Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'll look into commits and release notes of the Graal SDK.

Copy link
Contributor Author

@diegomarquezp diegomarquezp Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like Graal SDK 22 introduced a new class initialization strategy called strict image heap, which is enforced by default.

Since this is a behavior change affecting which classes are initialized, I'm leaning towards this new strategy being the cause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the parent feature: oracle/graal#4684
This was followed by the new strategy by default: oracle/graal#7474

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perfect, thanks! Do you mind adding these info to the description? Also be mindful that this change may affect handwritten libraries as well.

io.grpc.internal.InstantTimeProvider,

Loading