HBASE-27146 Avoid CellUtil.cloneRow in MetaCellComparator (#4571)

Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Reviewed-by: SiCheng-Zheng <643463623@qq.com>
(cherry picked from commit b1691a5318)

hbase-common/src/main/java/org/apache/hadoop/hbase/MetaCellComparator.java

@@ -38,22 +38,46 @@ public class MetaCellComparator extends CellComparatorImpl {
    */
   public static final MetaCellComparator META_COMPARATOR = new MetaCellComparator();
 
-  // TODO: Do we need a ByteBufferKeyValue version of this?
   @Override
   public int compareRows(final Cell left, final Cell right) {
-    return compareRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(),
+    if (left instanceof ByteBufferExtendedCell) {
-      right.getRowArray(), right.getRowOffset(), right.getRowLength());
+      ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
+      if (right instanceof ByteBufferExtendedCell) {
+        ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
+        return compareBBRows(bbLeft.getRowByteBuffer(), bbLeft.getRowPosition(),
+          left.getRowLength(), bbRight.getRowByteBuffer(), bbRight.getRowPosition(),
+          right.getRowLength());
+      } else {
+        return compareBBAndBytesRows(bbLeft.getRowByteBuffer(), bbLeft.getRowPosition(),
+          left.getRowLength(), right.getRowArray(), right.getRowOffset(), right.getRowLength());
+      }
+    } else {
+      if (right instanceof ByteBufferExtendedCell) {
+        ByteBufferExtendedCell bbRight = (ByteBufferExtendedCell) right;
+        return -compareBBAndBytesRows(bbRight.getRowByteBuffer(), bbRight.getRowPosition(),
+          right.getRowLength(), left.getRowArray(), left.getRowOffset(), left.getRowLength());
+      } else {
+        return compareBytesRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(),
+          right.getRowArray(), right.getRowOffset(), right.getRowLength());
+      }
+    }
   }
 
   @Override
   public int compareRows(Cell left, byte[] right, int roffset, int rlength) {
-    return compareRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(), right, roffset,
+    if (left instanceof ByteBufferExtendedCell) {
-      rlength);
+      ByteBufferExtendedCell bbLeft = (ByteBufferExtendedCell) left;
+      return compareBBAndBytesRows(bbLeft.getRowByteBuffer(), bbLeft.getRowPosition(),
+        left.getRowLength(), right, roffset, rlength);
+    } else {
+      return compareBytesRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(), right,
+        roffset, rlength);
+    }
   }
 
   @Override
   public int compareRows(byte[] leftRow, byte[] rightRow) {
-    return compareRows(leftRow, 0, leftRow.length, rightRow, 0, rightRow.length);
+    return compareBytesRows(leftRow, 0, leftRow.length, rightRow, 0, rightRow.length);
   }
 
   @Override
@@ -72,14 +96,31 @@ public class MetaCellComparator extends CellComparatorImpl {
     return ignoreSequenceid ? diff : Longs.compare(b.getSequenceId(), a.getSequenceId());
   }
 
-  private static int compareRows(byte[] left, int loffset, int llength, byte[] right, int roffset,
+  @FunctionalInterface
-    int rlength) {
+  private interface SearchDelimiter<T> {
-    int leftDelimiter = Bytes.searchDelimiterIndex(left, loffset, llength, HConstants.DELIMITER);
+    int search(T t, int offset, int length, int delimiter);
-    int rightDelimiter = Bytes.searchDelimiterIndex(right, roffset, rlength, HConstants.DELIMITER);
+  }
+
+  @FunctionalInterface
+  private interface SearchDelimiterInReverse<T> {
+    int search(T t, int offset, int length, int delimiter);
+  }
+
+  @FunctionalInterface
+  private interface Compare<L, R> {
+    int compareTo(L left, int loffset, int llength, R right, int roffset, int rlength);
+  }
+
+  private static <L, R> int compareRows(L left, int loffset, int llength, R right, int roffset,
+    int rlength, SearchDelimiter<L> searchLeft, SearchDelimiter<R> searchRight,
+    SearchDelimiterInReverse<L> searchInReverseLeft,
+    SearchDelimiterInReverse<R> searchInReverseRight, Compare<L, R> comparator) {
+    int leftDelimiter = searchLeft.search(left, loffset, llength, HConstants.DELIMITER);
+    int rightDelimiter = searchRight.search(right, roffset, rlength, HConstants.DELIMITER);
     // Compare up to the delimiter
     int lpart = (leftDelimiter < 0 ? llength : leftDelimiter - loffset);
     int rpart = (rightDelimiter < 0 ? rlength : rightDelimiter - roffset);
-    int result = Bytes.compareTo(left, loffset, lpart, right, roffset, rpart);
+    int result = comparator.compareTo(left, loffset, lpart, right, roffset, rpart);
     if (result != 0) {
       return result;
     } else {
@@ -95,14 +136,14 @@ public class MetaCellComparator extends CellComparatorImpl {
     // Move past delimiter
     leftDelimiter++;
     rightDelimiter++;
-    int leftFarDelimiter = Bytes.searchDelimiterIndexInReverse(left, leftDelimiter,
+    int leftFarDelimiter = searchInReverseLeft.search(left, leftDelimiter,
       llength - (leftDelimiter - loffset), HConstants.DELIMITER);
-    int rightFarDelimiter = Bytes.searchDelimiterIndexInReverse(right, rightDelimiter,
+    int rightFarDelimiter = searchInReverseRight.search(right, rightDelimiter,
       rlength - (rightDelimiter - roffset), HConstants.DELIMITER);
     // Now compare middlesection of row.
     lpart = (leftFarDelimiter < 0 ? llength + loffset : leftFarDelimiter) - leftDelimiter;
     rpart = (rightFarDelimiter < 0 ? rlength + roffset : rightFarDelimiter) - rightDelimiter;
-    result = Bytes.compareTo(left, leftDelimiter, lpart, right, rightDelimiter, rpart);
+    result = comparator.compareTo(left, leftDelimiter, lpart, right, rightDelimiter, rpart);
     if (result != 0) {
       return result;
     } else {
@@ -117,28 +158,56 @@ public class MetaCellComparator extends CellComparatorImpl {
     // Compare last part of row, the rowid.
     leftFarDelimiter++;
     rightFarDelimiter++;
-    result = Bytes.compareTo(left, leftFarDelimiter, llength - (leftFarDelimiter - loffset), right,
+    result = comparator.compareTo(left, leftFarDelimiter, llength - (leftFarDelimiter - loffset),
-      rightFarDelimiter, rlength - (rightFarDelimiter - roffset));
+      right, rightFarDelimiter, rlength - (rightFarDelimiter - roffset));
     return result;
   }
 
+  private static int compareBBRows(ByteBuffer left, int loffset, int llength, ByteBuffer right,
+    int roffset, int rlength) {
+    if (left.hasArray()) {
+      return -compareBBAndBytesRows(right, roffset, rlength, left.array(),
+        left.arrayOffset() + loffset, llength);
+    }
+    if (right.hasArray()) {
+      return compareBBAndBytesRows(left, loffset, llength, right.array(),
+        right.arrayOffset() + roffset, rlength);
+    }
+    return compareRows(left, loffset, llength, right, roffset, rlength,
+      ByteBufferUtils::searchDelimiterIndex, ByteBufferUtils::searchDelimiterIndex,
+      ByteBufferUtils::searchDelimiterIndexInReverse,
+      ByteBufferUtils::searchDelimiterIndexInReverse, ByteBufferUtils::compareTo);
+  }
+
+  private static int compareBBAndBytesRows(ByteBuffer left, int loffset, int llength, byte[] right,
+    int roffset, int rlength) {
+    if (left.hasArray()) {
+      return compareBytesRows(left.array(), left.arrayOffset() + loffset, llength, right, roffset,
+        rlength);
+    }
+    return compareRows(left, loffset, llength, right, roffset, rlength,
+      ByteBufferUtils::searchDelimiterIndex, Bytes::searchDelimiterIndex,
+      ByteBufferUtils::searchDelimiterIndexInReverse, Bytes::searchDelimiterIndexInReverse,
+      ByteBufferUtils::compareTo);
+  }
+
+  private static int compareBytesRows(byte[] left, int loffset, int llength, byte[] right,
+    int roffset, int rlength) {
+    return compareRows(left, loffset, llength, right, roffset, rlength, Bytes::searchDelimiterIndex,
+      Bytes::searchDelimiterIndex, Bytes::searchDelimiterIndexInReverse,
+      Bytes::searchDelimiterIndexInReverse, Bytes::compareTo);
+  }
+
   @Override
   public int compareRows(ByteBuffer row, Cell cell) {
-    byte[] array;
+    if (cell instanceof ByteBufferExtendedCell) {
-    int offset;
+      ByteBufferExtendedCell bbCell = (ByteBufferExtendedCell) cell;
-    int len = row.remaining();
+      return compareBBRows(row, row.position(), row.remaining(), bbCell.getRowByteBuffer(),
-    if (row.hasArray()) {
+        bbCell.getRowPosition(), cell.getRowLength());
-      array = row.array();
-      offset = row.position() + row.arrayOffset();
     } else {
-      // We copy the row array if offheap just so we can do a compare. We do this elsewhere too
+      return compareBBAndBytesRows(row, row.position(), row.remaining(), cell.getRowArray(),
-      // in BBUtils when Cell is backed by an offheap ByteBuffer. Needs fixing so no copy. TODO.
+        cell.getRowOffset(), cell.getRowLength());
-      array = new byte[len];
-      offset = 0;
-      ByteBufferUtils.copyFromBufferToArray(array, row, row.position(), 0, len);
     }
-    // Reverse result since we swap the order of the params we pass below.
-    return -compareRows(cell, array, offset, len);
   }
 
   @Override

hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java

@@ -1216,4 +1216,32 @@ public final class ByteBufferUtils {
   public static String toStringBinary(final ByteBuffer b) {
     return toStringBinary(b, 0, b.capacity());
   }
+
+  /**
+   * Find index of passed delimiter.
+   * @return Index of delimiter having started from start of <code>b</code> moving rightward.
+   */
+  public static int searchDelimiterIndex(ByteBuffer b, int offset, final int length,
+    final int delimiter) {
+    for (int i = offset, n = offset + length; i < n; i++) {
+      if (b.get(i) == delimiter) {
+        return i;
+      }
+    }
+    return -1;
+  }
+
+  /**
+   * Find index of passed delimiter walking from end of buffer backwards.
+   * @return Index of delimiter
+   */
+  public static int searchDelimiterIndexInReverse(ByteBuffer b, int offset, int length,
+    int delimiter) {
+    for (int i = offset + length - 1; i >= offset; i--) {
+      if (b.get(i) == delimiter) {
+        return i;
+      }
+    }
+    return -1;
+  }
 }

hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java

@@ -2576,7 +2576,8 @@ public class Bytes implements Comparable<Bytes> {
   }
 
   /**
-   * nn * @return Index of delimiter having started from start of <code>b</code> moving rightward.
+   * Find index of passed delimiter.
+   * @return Index of delimiter having started from start of <code>b</code> moving rightward.
    */
   public static int searchDelimiterIndex(final byte[] b, int offset, final int length,
     final int delimiter) {
@@ -2594,8 +2595,8 @@ public class Bytes implements Comparable<Bytes> {
   }
 
   /**
-   * Find index of passed delimiter walking from end of buffer backwards. nn * @return Index of
+   * Find index of passed delimiter walking from end of buffer backwards.
-   * delimiter
+   * @return Index of delimiter
    */
   public static int searchDelimiterIndexInReverse(final byte[] b, final int offset,
     final int length, final int delimiter) {