From b1691a53185efcafc1d2a15767a88825f4d7b625 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Fri, 24 Jun 2022 22:29:17 +0800 Subject: [PATCH] HBASE-27146 Avoid CellUtil.cloneRow in MetaCellComparator (#4571) Signed-off-by: Bryan Beaudreault Reviewed-by: SiCheng-Zheng <643463623@qq.com> --- .../hadoop/hbase/MetaCellComparator.java | 127 ++++++++++++++---- .../hadoop/hbase/util/ByteBufferUtils.java | 28 ++++ .../org/apache/hadoop/hbase/util/Bytes.java | 7 +- 3 files changed, 130 insertions(+), 32 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/MetaCellComparator.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/MetaCellComparator.java index 0ab6fce69ef..43d9e3ee9d7 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/MetaCellComparator.java +++ b/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(), - right.getRowArray(), right.getRowOffset(), right.getRowLength()); + if (left instanceof ByteBufferExtendedCell) { + 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, - rlength); + if (left instanceof ByteBufferExtendedCell) { + 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, - int rlength) { - int leftDelimiter = Bytes.searchDelimiterIndex(left, loffset, llength, HConstants.DELIMITER); - int rightDelimiter = Bytes.searchDelimiterIndex(right, roffset, rlength, HConstants.DELIMITER); + @FunctionalInterface + private interface SearchDelimiter { + int search(T t, int offset, int length, int delimiter); + } + + @FunctionalInterface + private interface SearchDelimiterInReverse { + int search(T t, int offset, int length, int delimiter); + } + + @FunctionalInterface + private interface Compare { + int compareTo(L left, int loffset, int llength, R right, int roffset, int rlength); + } + + private static int compareRows(L left, int loffset, int llength, R right, int roffset, + int rlength, SearchDelimiter searchLeft, SearchDelimiter searchRight, + SearchDelimiterInReverse searchInReverseLeft, + SearchDelimiterInReverse searchInReverseRight, Compare 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, - rightFarDelimiter, rlength - (rightFarDelimiter - roffset)); + result = comparator.compareTo(left, leftFarDelimiter, llength - (leftFarDelimiter - loffset), + 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; - int offset; - int len = row.remaining(); - if (row.hasArray()) { - array = row.array(); - offset = row.position() + row.arrayOffset(); + if (cell instanceof ByteBufferExtendedCell) { + ByteBufferExtendedCell bbCell = (ByteBufferExtendedCell) cell; + return compareBBRows(row, row.position(), row.remaining(), bbCell.getRowByteBuffer(), + bbCell.getRowPosition(), cell.getRowLength()); } else { - // We copy the row array if offheap just so we can do a compare. We do this elsewhere too - // in BBUtils when Cell is backed by an offheap ByteBuffer. Needs fixing so no copy. TODO. - array = new byte[len]; - offset = 0; - ByteBufferUtils.copyFromBufferToArray(array, row, row.position(), 0, len); + return compareBBAndBytesRows(row, row.position(), row.remaining(), cell.getRowArray(), + cell.getRowOffset(), cell.getRowLength()); } - // Reverse result since we swap the order of the params we pass below. - return -compareRows(cell, array, offset, len); } @Override diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java index 5fc28bd9853..32c6779bc04 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java @@ -1191,4 +1191,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 b 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; + } } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java index c68eb428b84..2d5e7f4266d 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java @@ -2387,7 +2387,8 @@ public class Bytes implements Comparable { } /** - * nn * @return Index of delimiter having started from start of b moving rightward. + * Find index of passed delimiter. + * @return Index of delimiter having started from start of b moving rightward. */ public static int searchDelimiterIndex(final byte[] b, int offset, final int length, final int delimiter) { @@ -2405,8 +2406,8 @@ public class Bytes implements Comparable { } /** - * Find index of passed delimiter walking from end of buffer backwards. nn * @return Index of - * delimiter + * Find index of passed delimiter walking from end of buffer backwards. + * @return Index of delimiter */ public static int searchDelimiterIndexInReverse(final byte[] b, final int offset, final int length, final int delimiter) {