From 5ff317fe7bd8434dbf3925377477eb95f2f5b7d1 Mon Sep 17 00:00:00 2001 From: huzheng Date: Thu, 30 May 2019 22:17:05 +0800 Subject: [PATCH] HBASE-22504 Optimize the MultiByteBuff#get(ByteBuffer, offset, len) --- .../hadoop/hbase/nio/MultiByteBuff.java | 31 +++++++++++-------- .../hadoop/hbase/util/ByteBufferUtils.java | 25 ++------------- .../hadoop/hbase/nio/TestMultiByteBuff.java | 25 +++++++++++++++ 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java index 186d9ba0675..3ce17090397 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java @@ -580,7 +580,7 @@ public class MultiByteBuff extends ByteBuff { while (length > 0) { int toRead = Math.min(length, this.curItem.remaining()); ByteBufferUtils.copyFromBufferToArray(dst, this.curItem, this.curItem.position(), offset, - toRead); + toRead); this.curItem.position(this.curItem.position() + toRead); length -= toRead; if (length == 0) break; @@ -598,8 +598,7 @@ public class MultiByteBuff extends ByteBuff { sourceOffset = sourceOffset - this.itemBeginPos[itemIndex]; while (length > 0) { int toRead = Math.min((item.limit() - sourceOffset), length); - ByteBufferUtils.copyFromBufferToArray(dst, item, sourceOffset, offset, - toRead); + ByteBufferUtils.copyFromBufferToArray(dst, item, sourceOffset, offset, toRead); length -= toRead; if (length == 0) break; itemIndex++; @@ -1020,24 +1019,30 @@ public class MultiByteBuff extends ByteBuff { } pair.setFirst(ByteBuffer.wrap(dst)); pair.setSecond(0); - return; } /** * Copies the content from an this MBB to a ByteBuffer - * @param out the ByteBuffer to which the copy has to happen - * @param sourceOffset the offset in the MBB from which the elements has - * to be copied + * @param out the ByteBuffer to which the copy has to happen, its position will be advanced. + * @param sourceOffset the offset in the MBB from which the elements has to be copied * @param length the length in the MBB upto which the elements has to be copied */ @Override - public void get(ByteBuffer out, int sourceOffset, - int length) { + public void get(ByteBuffer out, int sourceOffset, int length) { checkRefCount(); - // Not used from real read path actually. So not going with - // optimization - for (int i = 0; i < length; ++i) { - out.put(this.get(sourceOffset + i)); + int itemIndex = getItemIndex(sourceOffset); + ByteBuffer in = this.items[itemIndex]; + sourceOffset = sourceOffset - this.itemBeginPos[itemIndex]; + while (length > 0) { + int toRead = Math.min(in.limit() - sourceOffset, length); + ByteBufferUtils.copyFromBufferToBuffer(in, out, sourceOffset, toRead); + length -= toRead; + if (length == 0) { + break; + } + itemIndex++; + in = this.items[itemIndex]; + sourceOffset = 0; } } 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 abc900ef457..e29ead5a16c 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 @@ -721,8 +721,8 @@ public final class ByteBufferUtils { * @param sourceOffset offset in the source buffer * @param length how many bytes to copy */ - public static void copyFromBufferToBuffer(ByteBuffer in, - ByteBuffer out, int sourceOffset, int length) { + public static void copyFromBufferToBuffer(ByteBuffer in, ByteBuffer out, int sourceOffset, + int length) { if (in.hasArray() && out.hasArray()) { System.arraycopy(in.array(), sourceOffset + in.arrayOffset(), out.array(), out.position() + out.arrayOffset(), length); @@ -737,27 +737,6 @@ public final class ByteBufferUtils { } } - /** - * Find length of common prefix of two parts in the buffer - * @param buffer Where parts are located. - * @param offsetLeft Offset of the first part. - * @param offsetRight Offset of the second part. - * @param limit Maximal length of common prefix. - * @return Length of prefix. - */ - public static int findCommonPrefix(ByteBuffer buffer, int offsetLeft, - int offsetRight, int limit) { - int prefix = 0; - - for (; prefix < limit; ++prefix) { - if (buffer.get(offsetLeft + prefix) != buffer.get(offsetRight + prefix)) { - break; - } - } - - return prefix; - } - /** * Find length of common prefix in two arrays. * @param left Array to be compared. diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java index fcfb77a03f0..74d09405b4a 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java @@ -457,4 +457,29 @@ public class TestMultiByteBuff { assertEquals(i, mbb.getInt()); assertEquals(l, mbb.getLong()); } + + @Test + public void testGetByteBufferWithOffsetAndPos() { + byte[] a = Bytes.toBytes("abcd"); + byte[] b = Bytes.toBytes("efghijkl"); + ByteBuffer aa = ByteBuffer.wrap(a); + ByteBuffer bb = ByteBuffer.wrap(b); + MultiByteBuff mbb = new MultiByteBuff(aa, bb); + ByteBuffer out = ByteBuffer.allocate(12); + mbb.get(out, 0, 1); + assertEquals(out.position(), 1); + assertTrue(Bytes.equals(Bytes.toBytes("a"), 0, 1, out.array(), 0, 1)); + + mbb.get(out, 1, 4); + assertEquals(out.position(), 5); + assertTrue(Bytes.equals(Bytes.toBytes("abcde"), 0, 5, out.array(), 0, 5)); + + mbb.get(out, 10, 1); + assertEquals(out.position(), 6); + assertTrue(Bytes.equals(Bytes.toBytes("abcdek"), 0, 6, out.array(), 0, 6)); + + mbb.get(out, 0, 6); + assertEquals(out.position(), 12); + assertTrue(Bytes.equals(Bytes.toBytes("abcdekabcdef"), 0, 12, out.array(), 0, 12)); + } }