You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Anoop Sam John (JIRA)" <ji...@apache.org> on 2016/10/25 05:02:58 UTC
[jira] [Comment Edited] (HBASE-16783) Use ByteBufferPool for the
header and message during Rpc response
[ 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)