[jira] [Comment Edited] (HBASE-13387) Add ByteBufferedCell an extension to Cell

["Anoop Sam John (JIRA)" <ji...@apache.org>]

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

Anoop Sam John edited comment on HBASE-13387 at 7/3/15 5:42 PM:
----------------------------------------------------------------

Worked on a patch for this. Mainly in CellComparator and CellUtil matching APIs, added the ByteBufferedCell instance check. When the cell is instance of ByteBufferedCell , we will use getXXXByteBuffer() API  rathen than getXXXArray().   (Pls note that HBASE-12345 added Unsafe based compare in ByteBufferUtil to compare BBs).
ByteBufferedCell is created as an interface extending Cell.
Doing perf test with PE (range scan with 10K range and all cells filtered out at server) I was seeing a 7% perf down. ( This is with this patch alone which add some extra burden of below kind of type check in our compare methods)
{code}
 public int compareRows(final Cell left, final Cell right) {
    if (left instanceof ByteBufferedCell && right instanceof ByteBufferedCell) {
      return ByteBufferUtils.compareTo(((ByteBufferedCell) left).getRowByteBuffer(),
          ((ByteBufferedCell) left).getRowPositionInByteBuffer(), left.getRowLength(),
          ((ByteBufferedCell) right).getRowByteBuffer(),
          ((ByteBufferedCell) right).getRowPositionInByteBuffer(), right.getRowLength());
    }
    if (left instanceof ByteBufferedCell) {
      return ByteBufferUtils.compareTo(((ByteBufferedCell) left).getRowByteBuffer(),
          ((ByteBufferedCell) left).getRowPositionInByteBuffer(), left.getRowLength(),
          right.getRowArray(), right.getRowOffset(), right.getRowLength());
    }
    if (right instanceof ByteBufferedCell) {
      return -(ByteBufferUtils.compareTo(((ByteBufferedCell) right).getRowByteBuffer(),
          ((ByteBufferedCell) right).getRowPositionInByteBuffer(), right.getRowLength(),
          left.getRowArray(), left.getRowOffset(), left.getRowLength()));
    }
    return Bytes.compareTo(left.getRowArray(), left.getRowOffset(), left.getRowLength(),
        right.getRowArray(), right.getRowOffset(), right.getRowLength());

   }
{code}
Basically the code is like this and we are still not making cell of type ByteBufferedCell.

Then tested by changing ByteBufferedCell into an abstract class instead of Interface.  
The diff is quite visible.  There is no perf degrade with this.

Calling non overriding method via interface type seems less performing. Should be related to java run time optimization and inlining.


was (Author: anoop.hbase):
Worked on a patch for this. Mainly in CellComparator and CellUtil matching APIs, added the ByteBufferedCell instance check. When the cell is instance of ByteBufferedCell , we will use getXXXByteBuffer() API  rathen than getXXXArray().   (Pls note that HBASE-12345 added Unsafe based compare in ByteBufferUtil to compare BBs).
ByteBufferedCell is created as an interface extending Cell.
Doing perf test with PE (range scan with 10K range and all cells filtered out at server) I was seeing a 7% perf down.
{code}
 public int compareRows(final Cell left, final Cell right) {
    if (left instanceof ByteBufferedCell && right instanceof ByteBufferedCell) {
      return ByteBufferUtils.compareTo(((ByteBufferedCell) left).getRowByteBuffer(),
          ((ByteBufferedCell) left).getRowPositionInByteBuffer(), left.getRowLength(),
          ((ByteBufferedCell) right).getRowByteBuffer(),
          ((ByteBufferedCell) right).getRowPositionInByteBuffer(), right.getRowLength());
    }
    if (left instanceof ByteBufferedCell) {
      return ByteBufferUtils.compareTo(((ByteBufferedCell) left).getRowByteBuffer(),
          ((ByteBufferedCell) left).getRowPositionInByteBuffer(), left.getRowLength(),
          right.getRowArray(), right.getRowOffset(), right.getRowLength());
    }
    if (right instanceof ByteBufferedCell) {
      return -(ByteBufferUtils.compareTo(((ByteBufferedCell) right).getRowByteBuffer(),
          ((ByteBufferedCell) right).getRowPositionInByteBuffer(), right.getRowLength(),
          left.getRowArray(), left.getRowOffset(), left.getRowLength()));
    }
    return Bytes.compareTo(left.getRowArray(), left.getRowOffset(), left.getRowLength(),
        right.getRowArray(), right.getRowOffset(), right.getRowLength());

   }
{code}
Basically the code is like this and we are still not making cell of type ByteBufferedCell.

Then tested by changing ByteBufferedCell into an abstract class instead of Interface.  
The diff is quite visible.  There is no perf degrade with this.

Calling non overriding method via interface type seems less performing. Should be related to java run time optimization and inlining.

> Add ByteBufferedCell an extension to Cell
> -----------------------------------------
>
>                 Key: HBASE-13387
>                 URL: https://issues.apache.org/jira/browse/HBASE-13387
>             Project: HBase
>          Issue Type: Sub-task
>          Components: regionserver, Scanners
>            Reporter: Anoop Sam John
>            Assignee: Anoop Sam John
>         Attachments: ByteBufferedCell.docx, WIP_HBASE-13387_V2.patch, WIP_ServerCell.patch
>
>
> This came in btw the discussion abt the parent Jira and recently Stack added as a comment on the E2E patch on the parent Jira.
> The idea is to add a new Interface 'ByteBufferedCell'  in which we can add new buffer based getter APIs and getters for position in components in BB.  We will keep this interface @InterfaceAudience.Private.   When the Cell is backed by a DBB, we can create an Object implementing this new interface.
> The Comparators has to be aware abt this new Cell extension and has to use the BB based APIs rather than getXXXArray().  Also give util APIs in CellUtil to abstract the checks for new Cell type.  (Like matchingXXX APIs, getValueAs<type> APIs etc)



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