Upgrade to JC Tools 3.0 changes queue behavior when compiled natively on GraalVM. #10376
Comments
|
Hi, just a qq: why not use |
|
Thanks @michael-simons for the ping! Yes, we will have to update the native configuration for Netty when we update. |
|
... we already using 4.1.49, so it needs to be updated. |
|
Hi @franz1981. Thanks for chiming in. I was there as well in my research and found this piece of code in Netty: netty/common/src/main/java/io/netty/util/internal/PlatformDependent.java Lines 894 to 937 in 8c5b72a I think that should actually already do this. We also set To be clear: I have no interested in using anything from JCTools directly at all :) We are not calling those anywhere. |
I think so, yes! |
…vely and the unsafe configuration must be adjusted. Follow up of the discussion from netty/netty#10376
|
@michael-simons let me know if forcing with this @TargetClass(className = "io.netty.channel.nio.NioEventLoop")
final class Target_io_netty_channel_nio_NioEventLoop {
@Substitute
private static Queue<Runnable> newTaskQueue0(int maxPendingTasks) {
if (maxPendingTasks == Integer.MAX_VALUE)
return new MpscUnboundedAtomicArrayQueue<T>(1024);
final int capacity = Math.max(Math.min(maxPendingTasks, 1073741824), 2048);
return new MpscChunkedAtomicArrayQueue<>(1024, capacity);
}
}help. I see too that the original code static <T> Queue<T> newMpscQueue(final int maxCapacity) {
// Calculate the max capacity which can not be bigger then MAX_ALLOWED_MPSC_CAPACITY.
// This is forced by the MpscChunkedArrayQueue implementation as will try to round it
// up to the next power of two and so will overflow otherwise.
final int capacity = Math.max(Math.min(maxCapacity, 1073741824), 2048);
return USE_MPSC_CHUNKED_ARRAY_QUEUE ? new MpscChunkedArrayQueue<T>(MPSC_CHUNK_SIZE, capacity)
: new MpscGrowableAtomicArrayQueue<T>(MPSC_CHUNK_SIZE, capacity);
}isn't "right": |
|
@michael-simons OOps, I've missed the
|
|
I hope that didn’t came across ignorant. Should not be. I use it only through Netty. But I’ll see that I create a reproducer for pure JCtools. |
|
It seems that something just doesn't care about me setting After fiddling around to get JDK logging out of Graal, I see: |
…vely and the unsafe configuration must be adjusted. Follow up of the discussion from netty/netty#10376
|
@michael-simons @franz1981 WRT If the program is AOT compiled the flag should be set during the build, the runtime value will be ignored IIUC. Is that the case here? |
|
Same here, I need to either set noUnsafe on the binary (which is highly unexpected and was not the case before since netty also test if running in a native image) or way more finely tune the reflection: (I'm using Geronimo Arthur as graalvm wrapper). Side note I'm using 21.0.0.2.r11 of graalvm and only change is a netty upgrade. |
|
FWIW it looks like |
…g the native app. These were likely brought on by the Netty upgrade... see netty/netty#10376 for additional details.
…ively and the unsafe configuration must be adjusted.
Follow up of the discussion from netty/netty#10376
The upgrade to JC Tools 3.0 from 4.1.45 to 4.1.46 changes queue behavior when compiled natively on GraalVM.
For Netty 4.1.45 we used the following substitution in the Neo4j Java Driver
to force Netty to not use JC Tools Mscp queues. This is the same substitution that Quarkus uses (see https://github.com/quarkusio/quarkus/blob/0513ad94fd93f4ac377b796521a03dd3a52a156b/extensions/netty/runtime/src/main/java/io/quarkus/netty/runtime/graal/NettySubstitutions.java#L313-L320) and which I proposed to include in Netty itself here (#9989).
This substitution is not enough anymore since Netty 4.1.46.
Netty 4.1.46 updated JC Tools to 3.0.
When I downgrade this to 2.1.1 again, the substitution works as expected.
I have the suspicion that this change here JCTools/JCTools@2ac1a66 triggers a different behavior in native image mode.
This can be mitigated by allowing unsafe, reflective field access to a couple of classes and fields of JC Tools in a
reflection-config.json.{ "name" : "io.netty.util.internal.shaded.org.jctools.queues.MpscArrayQueueProducerIndexField", "fields": [ {"name": "producerIndex", "allowUnsafeAccess": true} ] }, { "name" : "io.netty.util.internal.shaded.org.jctools.queues.MpscArrayQueueProducerLimitField", "fields": [ {"name": "producerLimit", "allowUnsafeAccess": true} ] }, { "name" : "io.netty.util.internal.shaded.org.jctools.queues.MpscArrayQueueConsumerIndexField", "fields": [ {"name": "consumerIndex", "allowUnsafeAccess": true} ] }, { "name" : "io.netty.util.internal.shaded.org.jctools.queues.BaseMpscLinkedArrayQueueProducerFields", "fields": [ {"name": "producerIndex", "allowUnsafeAccess": true} ] }, { "name" : "io.netty.util.internal.shaded.org.jctools.queues.BaseMpscLinkedArrayQueueColdProducerFields", "fields": [ {"name": "producerLimit", "allowUnsafeAccess": true} ] }, { "name" : "io.netty.util.internal.shaded.org.jctools.queues.BaseMpscLinkedArrayQueueConsumerFields", "fields": [ {"name": "consumerIndex", "allowUnsafeAccess": true} ] }If this is not done, any thing bootstrapping Netty will fail with something in this area:
I have created the most simple reproducer I could: https://github.com/michael-simons/native-netty-timeserver-and-client Which is basically the vanilla server/client code from the Time server example of yours.
It can be compiled and run natively on the GraalVM. If you want to see the error, take this commit and update to anything >= 4.1.46 and stuff will fail.
The next commit contains the fixes.
Expected behavior or outcome
Nothing concrete at the moment: We at Neo4j can work around this, but it might have an impact on other projects that use Netty but are aiming for native compilation.
The substation mechanism seems brittle to say at least. Under the light of this experience here, I'd rather think about closing my pr but suggest picking up the suggested reflection config above for a future version of Netty. I personally don't feel to well to refer to shaded things (which we shade again in our driver).
As this was discovered by the kind Quarkus team here quarkusio/quarkus#10200, I'm also paging
@gsmet and @cescoffier as they might run into the same issues with the Netty substitutions in Quarkus itself.
Maybe @nitsanw has an idea why the movement of the the static field initializations are now triggered early?
The text was updated successfully, but these errors were encountered: