HBase
  1. HBase
  2. HBASE-4691

Remove more unnecessary byte[] copies from KeyValues

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.94.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Just looking through the code I found some more spots where we unnecessarily copy byte[] rather than just passing offset and length around.

      1. 4691.txt
        5 kB
        Lars Hofhansl

        Activity

        Hide
        Lars Hofhansl added a comment -

        This fixes a few. Nothing too serious.
        ReplicationSink used to copy every single KeyValue again.

        Show
        Lars Hofhansl added a comment - This fixes a few. Nothing too serious. ReplicationSink used to copy every single KeyValue again.
        Hide
        Ted Yu added a comment -

        Patch looks fine to me.

        Show
        Ted Yu added a comment - Patch looks fine to me.
        Hide
        stack added a comment -

        +1

        There were a few howlers in there. E.g:

        -    if (kv == null || kv.getValue().length != Bytes.SIZEOF_LONG)
        +    if (kv == null || kv.getValueLength() != Bytes.SIZEOF_LONG)
        

        Create a byte array just to get the length.

        And this one:

                   byte[] lastKey = kvs.get(0).getRow();
        -          Put put = new Put(kvs.get(0).getRow(),
        -              kvs.get(0).getTimestamp());
        

        Good stuff Lars.

        Show
        stack added a comment - +1 There were a few howlers in there. E.g: - if (kv == null || kv.getValue().length != Bytes.SIZEOF_LONG) + if (kv == null || kv.getValueLength() != Bytes.SIZEOF_LONG) Create a byte array just to get the length. And this one: byte [] lastKey = kvs.get(0).getRow(); - Put put = new Put(kvs.get(0).getRow(), - kvs.get(0).getTimestamp()); Good stuff Lars.
        Hide
        Lars Hofhansl added a comment -

        All tests pass... Will commit to trunk later today.

        Show
        Lars Hofhansl added a comment - All tests pass... Will commit to trunk later today.
        Hide
        Lars Hofhansl added a comment -

        Committed to trunk

        Show
        Lars Hofhansl added a comment - Committed to trunk
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2381 (See https://builds.apache.org/job/HBase-TRUNK/2381/)
        HBASE-4691 Remove more unnecessary byte[] copies from KeyValues

        larsh :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/coprocessor/LongColumnInterpreter.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/ColumnAggregationEndpoint.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2381 (See https://builds.apache.org/job/HBase-TRUNK/2381/ ) HBASE-4691 Remove more unnecessary byte[] copies from KeyValues larsh : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/coprocessor/LongColumnInterpreter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/coprocessor/ColumnAggregationEndpoint.java

          People

          • Assignee:
            Lars Hofhansl
            Reporter:
            Lars Hofhansl
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development