quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.85k stars 2.7k forks source link

Some Netty features are not enabled by default #39907

Open franz1981 opened 7 months ago

franz1981 commented 7 months ago

Description

Currently there are (after the introduction of Java modules), few Netty features, which are not enabled because the "right" JVM args are not present by default on quarkus applications:

  1. allocating direct buffer(s) without Cleaners: this is used on off-heap pooled arena(s) backed NIO ByteBuffers. It can be enabled adding -Dio.netty.tryReflectionSetAccessible=true --add-opens=java.base/java.nio=ALL-UNNAMED to the JVM args.
  2. allocating "big enough" (> 1024 by default) byte[] without zeroing them, using the "hidden" feature explained at https://shipilev.net/jvm/anatomy-quarks/7-initialization-costs/#_experiment. It can be enabled via -Dio.netty.tryReflectionSetAccessible=true --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED

I'm not proposing to add both, but I think is important to be aware that these features are not available in the current default configuration.

The first problem (related the cleaner option) is pretty important, because:

So, why this second option could be appealing?

  1. the allocated NIO ByteBuffers won't contain any Cleaner saving some memory
  2. the default heuristic for the platform, while going OOM, is not great (see https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/java/nio/Bits.java#L145-L178) and can cause an application to become unresponsive

Implementation ideas

Configured the proposed option by default

franz1981 commented 7 months ago

@zakkak @cescoffier

cescoffier commented 7 months ago

Hum, we cannot really recommend using --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED.

zakkak commented 7 months ago

I agree we should only resolve to opening core modules only if strictly necessary.

franz1981 commented 7 months ago

To me is fine to:

franz1981 commented 2 months ago

The issue related zeroing of heap buffers (i.e. byte[]) can be verified (in native image as well) by enabling debug/trace logging and search for these log messages: https://github.com/netty/netty/blob/afb475dabd5ca9eaa94ccf07f0ecd8431db008ba/common/src/main/java/io/netty/util/internal/PlatformDependent0.java#L507-L517

@zakkak

cescoffier commented 1 week ago

We would need alternatives to enable these options without requiring an add-open.

franz1981 commented 1 week ago

The alternative for the one impacting SSL is already in; by having SSL heap buffer pooling we don't need to care about allocate uninitialized arrays, cause we won't allocate them anymore 😁 For the cleaner one, I have to think 🤔