You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lécharny <el...@gmail.com> on 2022/01/14 08:49:43 UTC
MINA read buffer default size: is it too small ?
Hi,
I'm reviewing some parts of MINA (more specifically the SSL part), and I
found that we default the read buffer to 2048 bytes, which seems a bit
too small to me.
Typically, when handling TLS messages, we may have up to 16384 PDUs
(max). If the buffer is too small, we havve to do some extra read up to
the point we have received all the needed data.
At this point, I wonder if it wouldn't be a better idea to size the
buffer to at least the max PDU size of a TLS message (ie 16384 bytes).
The drawback is that it may be more demanding on the GC, as we will
allocate bigger buffers for each incoming message.
Incenditally I wonder if it wouldn't be a good idea to use a Thread
Local Storage to keep the allocated buffer, and avoiding the allocation
of such a buffer for every call (assuming we copy it if there is an
executor in the chain).
Thoughts ?
--
Emmanuel Lécharny
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: MINA read buffer default size: is it too small ?
Posted by Emmanuel Lécharny <el...@gmail.com>.
On 14/01/2022 14:46, Jonathan Valliere wrote:
> https://github.com/apache/mina/blob/c0ee516726c21115b196834ee984be786cea5818/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L277
>
> in 2.2.X the decode buffer is automatically calculated based on the input
> size so the entire input buffer can be decoded.
Same thing in 2.1.X:
https://github.com/apache/mina/blob/e49579069e49d733d8f75bc67b2e16311b619ffe/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java#L762
private SSLEngineResult unwrap() throws SSLException {
// We first have to create the application buffer if it does
not exist
if (appBuffer == null) {
appBuffer = IoBuffer.allocate(inNetBuffer.remaining());
These buffers cannot be
> cached anywhere like Thread-Local because the decoded buffer must be
> cleanly passed onto the next filter which may or may not go asynchronous.
Yep, but I think it should be the responsability of the executor to deal
with a buffer copy, otherwise we should pass the buffer up the stream
without any copy. Thus a TLS buffer could be safely used.
Btw, we also copy the incoming buffer *before* passing it to the SslEngine:
https://github.com/apache/mina/blob/e49579069e49d733d8f75bc67b2e16311b619ffe/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java#L348
SslHandler.messageReceived(NextFilter nextFilter, ByteBuffer buf)
throws SSLException {
...
// append buf to inNetBuffer
if (inNetBuffer == null) {
inNetBuffer =
IoBuffer.allocate(buf.remaining()).setAutoExpand(true);
}
inNetBuffer.put(buf);
...
unless we have more data in the buffer to process:
https://github.com/apache/mina/blob/e49579069e49d733d8f75bc67b2e16311b619ffe/mina-core/src/main/java/org/apache/mina/filter/ssl/SslHandler.java#L365
// prepare to be written again
if (inNetBuffer.hasRemaining()) {
inNetBuffer.compact();
} else {
inNetBuffer.free();
inNetBuffer = null;
}
That is a waste, IMO
>
> On Fri, Jan 14, 2022 at 7:38 AM Jonathan Valliere <jo...@apache.org>
> wrote:
>
>> Please point to the line of code in GitHub. GitHub has a great tool for
>> linking directly to lines.
>>
>> On Fri, Jan 14, 2022 at 3:49 AM Emmanuel Lécharny <el...@gmail.com>
>> wrote:
>>
>>> Hi,
>>>
>>> I'm reviewing some parts of MINA (more specifically the SSL part), and I
>>> found that we default the read buffer to 2048 bytes, which seems a bit
>>> too small to me.
>>>
>>> Typically, when handling TLS messages, we may have up to 16384 PDUs
>>> (max). If the buffer is too small, we havve to do some extra read up to
>>> the point we have received all the needed data.
>>>
>>> At this point, I wonder if it wouldn't be a better idea to size the
>>> buffer to at least the max PDU size of a TLS message (ie 16384 bytes).
>>>
>>> The drawback is that it may be more demanding on the GC, as we will
>>> allocate bigger buffers for each incoming message.
>>>
>>>
>>> Incenditally I wonder if it wouldn't be a good idea to use a Thread
>>> Local Storage to keep the allocated buffer, and avoiding the allocation
>>> of such a buffer for every call (assuming we copy it if there is an
>>> executor in the chain).
>>>
>>>
>>> Thoughts ?
>>>
>>> --
>>> Emmanuel Lécharny
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
>>> For additional commands, e-mail: dev-help@mina.apache.org
>>>
>>>
>
--
*Emmanuel Lécharny - CTO* 205 Promenade des Anglais – 06200 NICE
T. +33 (0)4 89 97 36 50
P. +33 (0)6 08 33 32 61
emmanuel.lecharny@busit.com https://www.busit.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: MINA read buffer default size: is it too small ?
Posted by Emmanuel Lécharny <el...@gmail.com>.
On 17/01/2022 15:24, Jonathan Valliere wrote:
> Emmanuel,
>
> The socket receive side buffer is stored for as long as necessary. You are
> referring to the clear-text application buffer here
> https://github.com/apache/mina/blob/660ab2375b4b47b5ebe86226c92f3138be4c96e8/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L277
>
> It would be possible to cache this in a Thread-Local-style however, not all
> SSL sessions have the same parameters so you risk necessary expansions;
I was thinking that we could use a big enough buffer to fit all the
needs (like 32kb), assuming the compression method is always NULL, but
it may not be the case, so yeah, it might not be such a good idea.
> additionally you would have to permanently remove that buffer from
> Thread-Local once anything is written into it for safety of the
> user-application. Have to be very careful not to maintain any pointers
> once that or a section of that buffer has been passed onto the next
> downstream filter.
Indeed. Zeroing the buffer after each usage probably makes the gain of
having a unique buffer pretty null.
All in all, allocating 16Kb of data is quite fast in modern JVM, and
even if it has to be zeroed, it won't remain in memory for a long time.
>
> Doesn't IoBuffer have a built-in caching mechanism?
Not by default. We use a SimpleBufforAllocator. Although there is a
CachedBufferAllocator, it's not use anywhere.
free() is called when
> its no longer needed which should make it fully compatible
> https://github.com/apache/mina/blob/660ab2375b4b47b5ebe86226c92f3138be4c96e8/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L201
>
> There is some room for improvements on decoding as many messages as
> possible on the receive side before looping. I will think about that.
Ok, thanks !
--
Emmanuel Lécharny
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: MINA read buffer default size: is it too small ?
Posted by Jonathan Valliere <jo...@apache.org>.
Emmanuel,
The socket receive side buffer is stored for as long as necessary. You are
referring to the clear-text application buffer here
https://github.com/apache/mina/blob/660ab2375b4b47b5ebe86226c92f3138be4c96e8/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L277
It would be possible to cache this in a Thread-Local-style however, not all
SSL sessions have the same parameters so you risk necessary expansions;
additionally you would have to permanently remove that buffer from
Thread-Local once anything is written into it for safety of the
user-application. Have to be very careful not to maintain any pointers
once that or a section of that buffer has been passed onto the next
downstream filter.
Doesn't IoBuffer have a built-in caching mechanism? free() is called when
its no longer needed which should make it fully compatible
https://github.com/apache/mina/blob/660ab2375b4b47b5ebe86226c92f3138be4c96e8/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandlerG0.java#L201
There is some room for improvements on decoding as many messages as
possible on the receive side before looping. I will think about that.
On Mon, Jan 17, 2022 at 1:30 AM Emmanuel Lécharny <el...@gmail.com>
wrote:
>
>
> On 14/01/2022 14:46, Jonathan Valliere wrote:
> >
> https://github.com/apache/mina/blob/c0ee516726c21115b196834ee984be786cea5818/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L277
> >
> > in 2.2.X the decode buffer is automatically calculated based on the input
> > size so the entire input buffer can be decoded. These buffers cannot be
> > cached anywhere like Thread-Local because the decoded buffer must be
> > cleanly passed onto the next filter which may or may not go asynchronous.
>
>
> Hi Jonathan,
>
> when we receive data, we first allocate a 2048 buffer (default size, can
> be changed with config), read the data and propagate it to the chain.
>
> The Sslhandler first step is to allocate a big (16Kb) buffer before
> calling SslEngine.unwrap().
>
> This is where I think it might make sense to use a TLS stored buffer:
> - we reuse it in SslHandler for the app buffer
> - if unwrap does not have any byte produced (HS), then we can simply
> reset the buffer
> - otherwise we can now allocate the proper buffer (which is likely to be
> way smaller) to fit the clear text data into it, instead of passing the
> big allocated buffer to the next filter.
>
> The gain would be:
> - avoiding allocating a buffer during the HS when unwrap does not
> produce anything
> - creating smaller objects in the standard processing (ie data processing)
>
> Emmanuel
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
> For additional commands, e-mail: dev-help@mina.apache.org
>
>
Re: MINA read buffer default size: is it too small ?
Posted by Emmanuel Lécharny <el...@gmail.com>.
On 14/01/2022 14:46, Jonathan Valliere wrote:
> https://github.com/apache/mina/blob/c0ee516726c21115b196834ee984be786cea5818/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L277
>
> in 2.2.X the decode buffer is automatically calculated based on the input
> size so the entire input buffer can be decoded. These buffers cannot be
> cached anywhere like Thread-Local because the decoded buffer must be
> cleanly passed onto the next filter which may or may not go asynchronous.
Hi Jonathan,
when we receive data, we first allocate a 2048 buffer (default size, can
be changed with config), read the data and propagate it to the chain.
The Sslhandler first step is to allocate a big (16Kb) buffer before
calling SslEngine.unwrap().
This is where I think it might make sense to use a TLS stored buffer:
- we reuse it in SslHandler for the app buffer
- if unwrap does not have any byte produced (HS), then we can simply
reset the buffer
- otherwise we can now allocate the proper buffer (which is likely to be
way smaller) to fit the clear text data into it, instead of passing the
big allocated buffer to the next filter.
The gain would be:
- avoiding allocating a buffer during the HS when unwrap does not
produce anything
- creating smaller objects in the standard processing (ie data processing)
Emmanuel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: MINA read buffer default size: is it too small ?
Posted by Jonathan Valliere <jo...@apache.org>.
https://github.com/apache/mina/blob/c0ee516726c21115b196834ee984be786cea5818/mina-core/src/main/java/org/apache/mina/filter/ssl/SSLHandler.java#L277
in 2.2.X the decode buffer is automatically calculated based on the input
size so the entire input buffer can be decoded. These buffers cannot be
cached anywhere like Thread-Local because the decoded buffer must be
cleanly passed onto the next filter which may or may not go asynchronous.
On Fri, Jan 14, 2022 at 7:38 AM Jonathan Valliere <jo...@apache.org>
wrote:
> Please point to the line of code in GitHub. GitHub has a great tool for
> linking directly to lines.
>
> On Fri, Jan 14, 2022 at 3:49 AM Emmanuel Lécharny <el...@gmail.com>
> wrote:
>
>> Hi,
>>
>> I'm reviewing some parts of MINA (more specifically the SSL part), and I
>> found that we default the read buffer to 2048 bytes, which seems a bit
>> too small to me.
>>
>> Typically, when handling TLS messages, we may have up to 16384 PDUs
>> (max). If the buffer is too small, we havve to do some extra read up to
>> the point we have received all the needed data.
>>
>> At this point, I wonder if it wouldn't be a better idea to size the
>> buffer to at least the max PDU size of a TLS message (ie 16384 bytes).
>>
>> The drawback is that it may be more demanding on the GC, as we will
>> allocate bigger buffers for each incoming message.
>>
>>
>> Incenditally I wonder if it wouldn't be a good idea to use a Thread
>> Local Storage to keep the allocated buffer, and avoiding the allocation
>> of such a buffer for every call (assuming we copy it if there is an
>> executor in the chain).
>>
>>
>> Thoughts ?
>>
>> --
>> Emmanuel Lécharny
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
>> For additional commands, e-mail: dev-help@mina.apache.org
>>
>>
Re: MINA read buffer default size: is it too small ?
Posted by Jonathan Valliere <jo...@apache.org>.
Please point to the line of code in GitHub. GitHub has a great tool for
linking directly to lines.
On Fri, Jan 14, 2022 at 3:49 AM Emmanuel Lécharny <el...@gmail.com>
wrote:
> Hi,
>
> I'm reviewing some parts of MINA (more specifically the SSL part), and I
> found that we default the read buffer to 2048 bytes, which seems a bit
> too small to me.
>
> Typically, when handling TLS messages, we may have up to 16384 PDUs
> (max). If the buffer is too small, we havve to do some extra read up to
> the point we have received all the needed data.
>
> At this point, I wonder if it wouldn't be a better idea to size the
> buffer to at least the max PDU size of a TLS message (ie 16384 bytes).
>
> The drawback is that it may be more demanding on the GC, as we will
> allocate bigger buffers for each incoming message.
>
>
> Incenditally I wonder if it wouldn't be a good idea to use a Thread
> Local Storage to keep the allocated buffer, and avoiding the allocation
> of such a buffer for every call (assuming we copy it if there is an
> executor in the chain).
>
>
> Thoughts ?
>
> --
> Emmanuel Lécharny
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
> For additional commands, e-mail: dev-help@mina.apache.org
>
>