hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anoop Sam John (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HBASE-16783) Use ByteBufferPool for the header and message during Rpc response
Date Tue, 25 Oct 2016 05:02:58 GMT

    [ https://issues.apache.org/jira/browse/HBASE-16783?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15604202#comment-15604202
] 

Anoop Sam John edited comment on HBASE-16783 at 10/25/16 5:02 AM:
------------------------------------------------------------------

+1.
Some minor comments. Can fix on commit
bq.ByteBuffer possiblePBBuf = (cellBlockSize > 0) ? cellBlock.get(cellBlock.size() - 1)
: null;
Ya some comments above this line what we are trying to do/optimize here. This is irrsepective
of the BBPool. We made the cellblock any way and of there is space left in the buffer of that,
make use of that rather than creating a temp one.

bq.possiblePBBuf.remaining() + totalPBSize <= possiblePBBuf.capacity()
Better to do possiblePBBuf.limit() + totalPBSize <= possiblePBBuf.capacity(). No issue
for above also as the pos will be always zero. But this will be better in generic way. Even
down we do the similar only
 bq. pbBuf.limit(totalPBSize + limit);

{code}
if (possiblePBBuf != null) {
542	        // Only if the last buffer has enough space for header use it. Else allocate
543	        // a new buffer. Assume they are all flipped
544	        if (possiblePBBuf.remaining() + totalPBSize <= possiblePBBuf.capacity()) {
{code}
Can we have both checks in single if? Wont be looking so complex also. Then u can avoid this
duplicated else blocks
{code}
} else {
560	          return createHeaderAndMessageBytes(result, header, totalSize, totalPBSize);
561	        }
562	      } else {
563	        return createHeaderAndMessageBytes(result, header, totalSize, totalPBSize);
564	      }
{code}


was (Author: anoop.hbase):
+1.
Some minor comments. Can fix on commit
bq.ByteBuffer possiblePBBuf = (cellBlockSize > 0) ? cellBlock.get(cellBlock.size() - 1)
: null;
Ya some comments above this line what we are trying to do/optimize here. This is irrsepective
of the BBPool. We made the cellblock any way and of there is space left in the buffer of that,
make use of that rather than creating a temp one.

bq.possiblePBBuf.remaining() + totalPBSize <= possiblePBBuf.capacity()
Better to do possiblePBBuf.limit() + totalPBSize <= possiblePBBuf.capacity(). No issue
for above also as the pos will be always zero. But this will be better in generic way. Even
down we do the similar only
 bq. pbBuf.limit(totalPBSize + limit);

{quote}
if (possiblePBBuf != null) {
542	        // Only if the last buffer has enough space for header use it. Else allocate
543	        // a new buffer. Assume they are all flipped
544	        if (possiblePBBuf.remaining() + totalPBSize <= possiblePBBuf.capacity()) {
{quote}
Can we have both checks in single if? Wont be looking so complex also. Then u can avoid this
duplicated else blocks
{quote}
} else {
560	          return createHeaderAndMessageBytes(result, header, totalSize, totalPBSize);
561	        }
562	      } else {
563	        return createHeaderAndMessageBytes(result, header, totalSize, totalPBSize);
564	      }
{quote}

> Use ByteBufferPool for the header and message during Rpc response
> -----------------------------------------------------------------
>
>                 Key: HBASE-16783
>                 URL: https://issues.apache.org/jira/browse/HBASE-16783
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: ramkrishna.s.vasudevan
>            Assignee: ramkrishna.s.vasudevan
>            Priority: Minor
>         Attachments: HBASE-16783.patch, HBASE-16783_1.patch, HBASE-16783_2.patch, HBASE-16783_3.patch,
HBASE-16783_4.patch, HBASE-16783_5.patch, HBASE-16783_6.patch
>
>
> With ByteBufferPool in place we could avoid the byte[] creation in RpcServer#createHeaderAndMessageBytes
and try using the Buffer from the pool rather than creating byte[] every time.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message