Expose OpenSSL engine settings

[franz1981]

Description

Currently, due to #39907 the default JDK provider is (further) under performing.
In particular:

• vertx, for good reasons, doesn't allow to pool byte[] buffers
• JDK SSL engine requires to allocate byte[] Netty buffers
• these buffers cost CPU usage due to Some Netty features are not enabled by default #39907 which is unable to skip zeroing them

And is pretty relevant in some profiling data on SSLEngine.
To completely solve this, quarkus could expose setting, at least for JVM mode, the choice of the SSL engine which piggyback to https://vertx.io/docs/vertx-core/java/#_ssl_engine

The OpenSSL engine doesn't require to use heap Netty buffers, saving the needy to add JVM options to skip zeroing them.

This would make many JVM Quarkus users happier

Implementation ideas

No response

[franz1981]

@geoand and @vietj

[franz1981]

This is a call stack which shows that memset is happening while it shouldn't - and the allocation of heap buffers in the hot path.

[geoand]

👌

[franz1981]

This is from an old Norman M presentation but I can say that is still very relevant - I have tested OpenJDK 21 before opening this issue

[franz1981]

I have verified that

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

Is doing its job and helping, but sadly given that the benchmark I have used is limited by the database, it was "just" saving CPU cycles due to saving the zeroing, which is still a free lunch

[geoand]

The OpenSSL engine would be an issue in native mode.

[franz1981]

Maybe? I remember some of the Netty stuff has been moved into native image - for micronaut. Maybe worth checking, and not only...what if we could enable it just on JVM mode? Or at least give the users the option to set it?
For native transport we do already - and native cannot use it afaik

[geoand]

Maybe worth checking

Definitely

what if we could enable it just on JVM mode

This is something we generally shy away from doing as it compromised the native image experience.

Let's include @cescoffier into the conversation

[cescoffier]

I have always stayed away from the OpenSSL engine:

• hard to integrate properly in containers (requires openssl, and the right version, at the right place)
• native is complicated
• there are some native calls that are likely going to pin virtual thread once we are there

[franz1981]

I can understand why we don't want to make OpenSSL enabled by default or through some discovery, but similarly to preferNativeTransport exposing it to users which knows what to do seems relevant, especially given the big difference in performance it could bring.

Conversely, to improve what we got we still have the option of adding -Dio.netty.tryReflectionSetAccessible=true --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED to the JVM args to at least reduce (a bit) the cost of allocating big byte[] in the hot path of JDK SSL.

[franz1981]

Just for reference, before -Dio.netty.tryReflectionSetAccessible=true --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED:

while, after applying it

The violet part is io/netty/util/internal/PlatformDependent.allocateUninitializedArray which cost is much reduced and the allocations which appear there are likely the ones before the default threshold of 1024 bytes.

@zakkak as well

[franz1981]

Another option could be to enable pooling of byte[], but TBH I'm not feeling very happy about it and I beleive @vietj got some good reason why is not enabled by default on vertx, am I right?

[geoand]

I think we should make it easy for users to opt-in to OpenSSL because as @franz1981 has demonstrated, the performance improvement is non trivial

[cescoffier]

We could add support for OpenSSL. There would be a few action items and constraints:

• it will be JVM only (so there is a difference of behavior between native and JVM, like native transport (not supported in native))
• Containerization should be covered thoroughly, with the correct version at the right place. Compare the image size...
• The flag/options should be marked as "advanced" (and a section in the doc should explain the pro/cons and describe the configuration)

I'm also concerned with the --add-opens popping up there and there...

[geoand]

I'm also concerned with the --add-opens popping up there and there...

AFAIU, this is not needed for OpenSSL

[franz1981]

@cescoffier

+100

AFAIU, this is not needed for OpenSSL

Exactly: OpenSSL just use off-heap pooled buffers, which are fine as they are.

The JVM arg instead is key with JDK SSL to have decent performance - and still the allocations of byte[] won't go away for JDK SSL - which will likely impact heap sizing/RSS regardless.

[franz1981]

So, about #41880 (comment): adding -Dio.netty.tryReflectionSetAccessible=true --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED to our JVM args. wdyt? we already have something similar I think.

[geoand]

So, about #41880 (comment): adding -Dio.netty.tryReflectionSetAccessible=true --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED to our JVM args. wdyt? we already have something similar I think.

Adding --add-opens cannot be done by us

[franz1981]

I've proceed into understanding why such big allocations for JDK SSL and found that https://github.com/netty/netty/blob/5085cef149134951c94a02a743ed70025c8cdad4/handler/src/main/java/io/netty/handler/ssl/SslHandler.java#L334

handler.engine.getSession().getPacketBufferSize() is 16709 bytes (!!) for SSLEngine - which means that regardless the traffic or whatever, we always allocate such a big buffer(s).

@geoand I would like to reconsider what found in vertx at eclipse-vertx/vert.x#5168 (comment) and ask @vietj if we can enable heap buffer pooling if SSL is configured. Allocating such a big heap buffers in the hot path is really not what we want...

See netty/netty@39cc7a6#diff-963dd9b8e77e913395273812889e9b6f2f449fe55e2b748b95311b219d1c58faR302 for more info

netty/netty#9685 (comment) too (although it was opened when off heap buffers were used)

[franz1981]

I'v sent an additional commit to eclipse-vertx/vert.x#5262 to address this
.
This is going to impact RSS, clearly, because the heap arena pools are not free and will stay still, regardless, per each event loop which is going to use it, including the thread-local ones, but the runtime performance will improve much more than just what shown #41880 (comment)

[franz1981]

I fear that netty/netty#9696 is no longer valid, so i've opened netty/netty#14208 to investigate on it

[franz1981]

Sadly it seems we have no other choice than including other SSL engines somehow or at least proceed with eclipse-vertx/vert.x#5262; netty/netty#14208 shows that the JDK SSL engine still relies on byte[] and cannot use off-heap buffers proficiently.

[cescoffier]

Just a head's up - we did not make progress on this so far.

[franz1981]

I have sent 2 pr to vertex to address the initial motivation to expose the engine settings - although being able to do it is still a good idea for xp users to me