-
Notifications
You must be signed in to change notification settings - Fork 59
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
chore: prepare showcase for Graal SDK 23.x #3574
Conversation
@@ -0,0 +1,3 @@ | |||
Args = --initialize-at-build-time=io.grpc.internal.TimeProvider,\ |
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.
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?
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.
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.
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 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
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.
Done
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 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?
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.
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?
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'll look into commits and release notes of the Graal SDK.
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.
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.
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 is the parent feature: oracle/graal#4684
This was followed by the new strategy by default: oracle/graal#7474
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.
Perfect, thanks! Do you mind adding these info to the description? Also be mindful that this change may affect handwritten libraries as well.
|
|
Approach
Register
java.time.Instant
to the reflection configuration of gax-grpc (see grpc code interacting with this class)This comes from a behavior change in Graal for JDK 22, which introduced a new class initialization strategy called strict image heap, which is enforced by default since then.
Proof: