mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Emmanuel Lecharny (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DIRMINA-1061) When AbstractPollingIoProcessor read nothing, free the temporary buffer should be better
Date Fri, 16 Dec 2016 09:53:58 GMT

    [ https://issues.apache.org/jira/browse/DIRMINA-1061?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15753972#comment-15753972

Emmanuel Lecharny commented on DIRMINA-1061:

Let me explain a bit more in detail what's {{IoBuffer.free()}} does - or does not -.

{{IoBuffer}} is a wrapper on top of the standard java {{ByteBuffer}} class. There are many
reasons we want to wrap this class :
- there are 2 flavors of {{ByteBuffer}} : direct and non-direct. Direct buffers are allocated
out of the Java memory space, and they are costly to allocate, and costly to free. OTOH, it's
the prefered flavor wen it comes to 'talk' to the OS
- {{ByteBuffer}} instances are not resizable. This is really problematic for us, as we need
to grow it when we read the data associated to a message
- we may want to pool buffers, in order to reuse them (one typical use is to use a pool of
buffers stored in the Thread Local Storage : this pool will not be synchronized, because it
does not need to be, and allocation is done only once. OTOH, it eats more memory...)

For these reasons, we added the {{free}} method to give an opportunity to the implementation
to do something useful when freeing the buffer. Like, puting back the buffer into the pool...

Regarding the piece of code you point out, once could ask if it's necessary to allocate a
buffer when we have potentially nothing to read. we *could* preallocate a buffer that is reused
again and again for the sole purpose of reading the data from the {{Channel}}. The problem
with this approach is that once we have read these data, we will have to copy the buffer into
another buffer that is going to be used by the application. The problem is that if the buffer
is a Direct buffer, this will require a copy, when we wanted to avoid that copy ({{ByteChannel.read()}}
will use the provided buffer if it's a direct one, or create a direct buffer internally and
copy its content into the provided not direct buffer). In some cases, that will induce a performance

Everything is a tradeoff ;-)

Anyway, thanks for your comment ! Feel free to add any comment !

> When AbstractPollingIoProcessor read nothing, free the temporary buffer should be better
> ----------------------------------------------------------------------------------------
>                 Key: DIRMINA-1061
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-1061
>             Project: MINA
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 2.0.16
>            Reporter: Mark
>            Priority: Minor
> {code:title=org.apache.mina.core.polling.AbstractPollingIoProcessor.java|borderStyle=solid}
> private void read(S session) {
>         IoSessionConfig config = session.getConfig();
>         int bufferSize = config.getReadBufferSize();
>         IoBuffer buf = IoBuffer.allocate(bufferSize);
> //...
> //...
>             if (readBytes > 0) {
>                 IoFilterChain filterChain = session.getFilterChain();
>                 filterChain.fireMessageReceived(buf);
>                 buf = null;
>                 if (hasFragmentation) {
>                     if (readBytes << 1 < config.getReadBufferSize()) {
>                         session.decreaseReadBufferSize();
>                     } else if (readBytes == config.getReadBufferSize()) {
>                         session.increaseReadBufferSize();
>                     }
>                 }
>             }
> {code}
> it seems that this method will be called when session closing.
> it'll better to call buf.free() if readBytes==0, to help allocator recycle this buffer,

This message was sent by Atlassian JIRA

View raw message