Refactor SslHandler internals to always use heap buffers for JDK SSLE… #9696
Conversation
…ngine implementation Motivation: We should aim to always use heap buffers when using the JDK SSLEngine for now as it wants to operate on byte[] and so will do internal memory copies if a non heap buffer is used. Beside this it will always return BUFFER_OVERFLOW when a smaller buffer then 16kb is used when calling wrap(...) (even if a very small amount of bytes should be encrypted). This can lead to excercive direct memory usage and pressure for no good reason. Modifications: Refactor internals of SslHandler to ensure we use heap buffers for the JDK SSLEngine impelementation Result: Less direct memory usage when JDK SSLEngine implementation is used
|
/cc @gmcatsf @tbrooks8 WDYT ? |
| return handler.engine.getSession().getPacketBufferSize(); | ||
| ByteBuf allocateWrapBuffer(SslHandler handler, ByteBufAllocator allocator, | ||
| int pendingBytes, int numComponents) { | ||
| // As for the JDK SSLEngine we always need to allocate buffers of the size of 16kb |
There was a problem hiding this comment.
NIT: It's technically over 16KB by default. Something around 16609. And can be configured to something different based on the context. Maybe say:
// As for the JDK SSLEngine we always need to allocate buffers of the size required by the SSLEngine (normally
// ~16KB). This is required even if the amount of data to encrypt is very small. Use heap buffers to reduce
There was a problem hiding this comment.
good idea... @tbrooks8 I wonder if you could also test / verify / benchmark this change as it seems you care about this :)
|
@normanmaurer, my understanding is that bytes in heap buffer will be copied into native memory later in the pipeline and could you comment if this change would not increase pressure down the pipeline? |
|
@gmcatsf at the end you will native memory and there is no way around this requirement. That said with this change the needed native memory should be minimised as it is very unlikely that you will always need 16kb buffers. |
What are you interested in? Like a JMH comparing usage of the SslHandler with direct buffers vs. heap buffers? I assume there are already unit tests around the SslHandler? |
|
@tbrooks8 I was just interested if you have some sort of benchmark that involves |
|
@normanmaurer My prior PR was motivated by the fact that I saw direct buffers being allocated (assertions triggering) in an environment without direct buffer pooling enabled. Not like a specific microbenchmark. I can download a netty snapshot against my build and check that the test assertions are not triggering anymore? |
|
@tbrooks8 I guess that is a good start :) Should I generate one for you or can you do it by yourself ? |
|
I can do it. I'll pull your branch and comment with results later today. |
|
@tbrooks8 <3 ... thanks a lot! |
|
I built the jars today and ran our Netty module test suite and everything looked good to me. We do have some full Elasticsearch benchmarks that use TLS. But unfortunately it's not the type of thing that I can easily run against a snapshot. I can monitor after the next release and reach out if I see any regressions though. |
#9696) Motivation: We should aim to always use heap buffers when using the JDK SSLEngine for now as it wants to operate on byte[] and so will do internal memory copies if a non heap buffer is used. Beside this it will always return BUFFER_OVERFLOW when a smaller buffer then 16kb is used when calling wrap(...) (even if a very small amount of bytes should be encrypted). This can lead to excercive direct memory usage and pressure for no good reason. Modifications: Refactor internals of SslHandler to ensure we use heap buffers for the JDK SSLEngine impelementation Result: Less direct memory usage when JDK SSLEngine implementation is used
…ngine implementation
Motivation:
We should aim to always use heap buffers when using the JDK SSLEngine for now as it wants to operate on byte[] and so will do internal memory copies if a non heap buffer is used. Beside this it will always return BUFFER_OVERFLOW when a smaller buffer then 16kb is used when calling wrap(...) (even if a very small amount of bytes should be encrypted). This can lead to excercive direct memory usage and pressure for no good reason.
Modifications:
Refactor internals of SslHandler to ensure we use heap buffers for the JDK SSLEngine impelementation
Result:
Less direct memory usage when JDK SSLEngine implementation is used