diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/CopyKeyDataBlockEncoder.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/CopyKeyDataBlockEncoder.java index 9310f32e859..de2da5a3767 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/CopyKeyDataBlockEncoder.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/CopyKeyDataBlockEncoder.java @@ -68,7 +68,7 @@ public class CopyKeyDataBlockEncoder extends BufferedDataBlockEncoder { @Override public Cell getFirstKeyCellInBlock(ByteBuff block) { - int keyLength = block.getIntStrictlyForward(Bytes.SIZEOF_INT); + int keyLength = block.getIntAfterPosition(Bytes.SIZEOF_INT); int pos = 3 * Bytes.SIZEOF_INT; ByteBuffer key = block.asSubByteBuffer(pos + keyLength).duplicate(); return createFirstKeyCell(key, keyLength); diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/ByteBuff.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/ByteBuff.java index 4398f5d6147..9a6041f3614 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/ByteBuff.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/ByteBuff.java @@ -175,13 +175,13 @@ public abstract class ByteBuff { public abstract byte get(int index); /** - * Fetches the byte at the given index. Does not change position of the underlying ByteBuffers. - * The difference for this API from {@link #get(int)} the index specified should be after - * the current position. If not throws IndexOutOfBoundsException - * @param index + * Fetches the byte at the given offset from current position. Does not change position + * of the underlying ByteBuffers. + * + * @param offset * @return the byte value at the given index. */ - public abstract byte getByteStrictlyForward(int index); + public abstract byte getByteAfterPosition(int offset); /** * Writes a byte to this ByteBuff at the current position and increments the position @@ -266,13 +266,13 @@ public abstract class ByteBuff { public abstract short getShort(int index); /** - * Fetches the short at the given index. Does not change position of the underlying ByteBuffers. - * The difference for this API from {@link #getShort(int)} the index specified should be - * after the current position. If not throws IndexOutOfBoundsException - * @param index + * Fetches the short value at the given offset from current position. Does not change position + * of the underlying ByteBuffers. + * + * @param offset * @return the short value at the given index. */ - public abstract short getShortStrictlyForward(int index); + public abstract short getShortAfterPosition(int offset); /** * Returns the int value at the current position. Also advances the position by the size of int @@ -301,15 +301,14 @@ public abstract class ByteBuff { public abstract int getInt(int index); /** - * Fetches the int at the given index. Does not change position of the underlying ByteBuffers. - * The difference for this API from {@link #getInt(int)} the index specified should be after - * the current position. If not throws IndexOutOfBoundsException - * @param index + * Fetches the int value at the given offset from current position. Does not change position + * of the underlying ByteBuffers. + * + * @param offset * @return the int value at the given index. */ - // TODO: any better name here?? getIntFromSubsequentPosition? or getIntAfterCurrentPosition? - // TODO : Make this relative wrt current position? Follow on JIRA - public abstract int getIntStrictlyForward(int index); + public abstract int getIntAfterPosition(int offset); + /** * Returns the long value at the current position. Also advances the position by the size of long * @@ -338,13 +337,13 @@ public abstract class ByteBuff { public abstract long getLong(int index); /** - * Fetches the long at the given index. Does not change position of the underlying ByteBuffers. - * The difference for this API from {@link #getLong(int)} the index specified should be after - * the current position. If not throws IndexOutOfBoundsException - * @param index + * Fetches the long value at the given offset from current position. Does not change position + * of the underlying ByteBuffers. + * + * @param offset * @return the long value at the given index. */ - public abstract long getLongStrictlyForward(int index); + public abstract long getLongAfterPosition(int offset); /** * Copy the content from this ByteBuff to a byte[] based on the given offset and 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 984ade5e740..ad339e3e7c9 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 @@ -132,12 +132,9 @@ public class MultiByteBuff extends ByteBuff { } @Override - public byte getByteStrictlyForward(int index) { + public byte getByteAfterPosition(int offset) { // Mostly the index specified will land within this current item. Short circuit for that - if(index < (this.itemBeginPos[this.curItemIndex] + this.curItem.position())) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } + int index = offset + this.position(); int itemIndex = getItemIndexFromCurItemIndex(index); return ByteBufferUtils.toByte(this.items[itemIndex], index - this.itemBeginPos[itemIndex]); } @@ -189,12 +186,9 @@ public class MultiByteBuff extends ByteBuff { } @Override - public int getIntStrictlyForward(int index) { + public int getIntAfterPosition(int offset) { // Mostly the index specified will land within this current item. Short circuit for that - if(index < (this.itemBeginPos[this.curItemIndex] + this.curItem.position())) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } + int index = offset + this.position(); int itemIndex; if (this.itemBeginPos[this.curItemIndex + 1] > index) { itemIndex = this.curItemIndex; @@ -237,12 +231,9 @@ public class MultiByteBuff extends ByteBuff { } @Override - public short getShortStrictlyForward(int index) { + public short getShortAfterPosition(int offset) { // Mostly the index specified will land within this current item. Short circuit for that - if(index < (this.itemBeginPos[this.curItemIndex] + this.curItem.position())) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } + int index = offset + this.position(); int itemIndex; if (this.itemBeginPos[this.curItemIndex + 1] > index) { itemIndex = this.curItemIndex; @@ -366,12 +357,9 @@ public class MultiByteBuff extends ByteBuff { } @Override - public long getLongStrictlyForward(int index) { + public long getLongAfterPosition(int offset) { // Mostly the index specified will land within this current item. Short circuit for that - if(index < (this.itemBeginPos[this.curItemIndex] + this.curItem.position())) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } + int index = offset + this.position(); int itemIndex; if (this.itemBeginPos[this.curItemIndex + 1] > index) { itemIndex = this.curItemIndex; diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/SingleByteBuff.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/SingleByteBuff.java index 62601f318d9..bfe5ba8cafb 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/SingleByteBuff.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/SingleByteBuff.java @@ -138,12 +138,8 @@ public class SingleByteBuff extends ByteBuff { } @Override - public byte getByteStrictlyForward(int index) { - if (index < this.buf.position()) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } - return ByteBufferUtils.toByte(this.buf, index); + public byte getByteAfterPosition(int offset) { + return ByteBufferUtils.toByte(this.buf, this.buf.position() + offset); } @Override @@ -222,12 +218,8 @@ public class SingleByteBuff extends ByteBuff { } @Override - public short getShortStrictlyForward(int index) { - if (index < this.buf.position()) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } - return ByteBufferUtils.toShort(this.buf, index); + public short getShortAfterPosition(int offset) { + return ByteBufferUtils.toShort(this.buf, this.buf.position() + offset); } @Override @@ -247,12 +239,8 @@ public class SingleByteBuff extends ByteBuff { } @Override - public int getIntStrictlyForward(int index) { - if (index < this.buf.position()) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } - return ByteBufferUtils.toInt(this.buf, index); + public int getIntAfterPosition(int offset) { + return ByteBufferUtils.toInt(this.buf, this.buf.position() + offset); } @Override @@ -272,12 +260,8 @@ public class SingleByteBuff extends ByteBuff { } @Override - public long getLongStrictlyForward(int index) { - if (index < this.buf.position()) { - throw new IndexOutOfBoundsException("The index " + index - + " should not be less than current position " + this.position()); - } - return ByteBufferUtils.toLong(this.buf, index); + public long getLongAfterPosition(int offset) { + return ByteBufferUtils.toLong(this.buf, this.buf.position() + offset); } @Override 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 588e946a1a4..6bd4c263dfb 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 @@ -96,13 +96,6 @@ public class TestMultiByteBuff { assertEquals(b1, mbb.get()); mbb.put(b); assertEquals(l2, mbb.getLong(22)); - try { - // This should fail because we have already move to a position - // greater than 22 - mbb.getLongStrictlyForward(22); - fail(); - } catch (IndexOutOfBoundsException e) { - } } @Test @@ -313,12 +306,12 @@ public class TestMultiByteBuff { mbb1.position(7); mbb1.put((byte) 2); mbb1.putInt(3); - mbb1.position(0); - mbb1.getIntStrictlyForward(4); + mbb1.rewind(); + mbb1.getIntAfterPosition(4); byte res = mbb1.get(7); assertEquals((byte) 2, res); mbb1.position(7); - int intRes = mbb1.getIntStrictlyForward(8); + int intRes = mbb1.getIntAfterPosition(1); assertEquals(3, intRes); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java index 85190d6d2d5..30cf7ab064a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlockIndex.java @@ -398,9 +398,9 @@ public class HFileBlockIndex { BlockType.LEAF_INDEX, null); ByteBuff b = midLeafBlock.getBufferWithoutHeader(); - int numDataBlocks = b.getInt(); - int keyRelOffset = b.getIntStrictlyForward(Bytes.SIZEOF_INT * (midKeyEntry + 1)); - int keyLen = b.getIntStrictlyForward(Bytes.SIZEOF_INT * (midKeyEntry + 2)) - + int numDataBlocks = b.getIntAfterPosition(0); + int keyRelOffset = b.getIntAfterPosition(Bytes.SIZEOF_INT * (midKeyEntry + 1)); + int keyLen = b.getIntAfterPosition(Bytes.SIZEOF_INT * (midKeyEntry + 2)) - keyRelOffset; int keyOffset = Bytes.SIZEOF_INT * (numDataBlocks + 2) + keyRelOffset + SECONDARY_INDEX_ENTRY_OVERHEAD; @@ -701,7 +701,7 @@ public class HFileBlockIndex { static int binarySearchNonRootIndex(Cell key, ByteBuff nonRootIndex, CellComparator comparator) { - int numEntries = nonRootIndex.getIntStrictlyForward(0); + int numEntries = nonRootIndex.getIntAfterPosition(0); int low = 0; int high = numEntries - 1; int mid = 0; @@ -719,7 +719,7 @@ public class HFileBlockIndex { mid = (low + high) >>> 1; // Midkey's offset relative to the end of secondary index - int midKeyRelOffset = nonRootIndex.getIntStrictlyForward(Bytes.SIZEOF_INT * (mid + 1)); + int midKeyRelOffset = nonRootIndex.getIntAfterPosition(Bytes.SIZEOF_INT * (mid + 1)); // The offset of the middle key in the blockIndex buffer int midKeyOffset = entriesOffset // Skip secondary index @@ -729,7 +729,7 @@ public class HFileBlockIndex { // We subtract the two consecutive secondary index elements, which // gives us the size of the whole (offset, onDiskSize, key) tuple. We // then need to subtract the overhead of offset and onDiskSize. - int midLength = nonRootIndex.getIntStrictlyForward(Bytes.SIZEOF_INT * (mid + 2)) - + int midLength = nonRootIndex.getIntAfterPosition(Bytes.SIZEOF_INT * (mid + 2)) - midKeyRelOffset - SECONDARY_INDEX_ENTRY_OVERHEAD; // we have to compare in this order, because the comparator order @@ -794,7 +794,7 @@ public class HFileBlockIndex { int entryIndex = binarySearchNonRootIndex(key, nonRootBlock, comparator); if (entryIndex != -1) { - int numEntries = nonRootBlock.getIntStrictlyForward(0); + int numEntries = nonRootBlock.getIntAfterPosition(0); // The end of secondary index and the beginning of entries themselves. int entriesOffset = Bytes.SIZEOF_INT * (numEntries + 2); @@ -802,7 +802,7 @@ public class HFileBlockIndex { // The offset of the entry we are interested in relative to the end of // the secondary index. int entryRelOffset = nonRootBlock - .getIntStrictlyForward(Bytes.SIZEOF_INT * (1 + entryIndex)); + .getIntAfterPosition(Bytes.SIZEOF_INT * (1 + entryIndex)); nonRootBlock.position(entriesOffset + entryRelOffset); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java index b285f0145f4..41893202fa2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java @@ -517,16 +517,16 @@ public class HFileReaderImpl implements HFile.Reader, Configurable { // Trying to imitate what was done - need to profile if this is better or // earlier way is better by doing mark and reset? // But ensure that you read long instead of two ints - long ll = blockBuffer.getLongStrictlyForward(blockBuffer.position()); + long ll = blockBuffer.getLongAfterPosition(0); // Read top half as an int of key length and bottom int as value length this.currKeyLen = (int)(ll >> Integer.SIZE); this.currValueLen = (int)(Bytes.MASK_FOR_LOWER_INT_IN_LONG ^ ll); checkKeyValueLen(); // Move position past the key and value lengths and then beyond the key and value - int p = blockBuffer.position() + (Bytes.SIZEOF_LONG + currKeyLen + currValueLen); + int p = (Bytes.SIZEOF_LONG + currKeyLen + currValueLen); if (reader.getFileContext().isIncludesTags()) { // Tags length is a short. - this.currTagsLen = blockBuffer.getShortStrictlyForward(p); + this.currTagsLen = blockBuffer.getShortAfterPosition(p); checkTagsLen(); p += (Bytes.SIZEOF_SHORT + currTagsLen); } @@ -543,9 +543,9 @@ public class HFileReaderImpl implements HFile.Reader, Configurable { /** * Read mvcc. Does checks to see if we even need to read the mvcc at all. - * @param position + * @param offsetFromPos */ - protected void readMvccVersion(final int position) { + protected void readMvccVersion(final int offsetFromPos) { // See if we even need to decode mvcc. if (!this.reader.shouldIncludeMemstoreTS()) return; if (!this.reader.isDecodeMemstoreTS()) { @@ -553,25 +553,26 @@ public class HFileReaderImpl implements HFile.Reader, Configurable { currMemstoreTSLen = 1; return; } - _readMvccVersion(position); + _readMvccVersion(offsetFromPos); } /** * Actually do the mvcc read. Does no checks. - * @param position + * @param offsetFromPos */ - private void _readMvccVersion(final int position) { + private void _readMvccVersion(int offsetFromPos) { // This is Bytes#bytesToVint inlined so can save a few instructions in this hot method; i.e. // previous if one-byte vint, we'd redo the vint call to find int size. // Also the method is kept small so can be inlined. - byte firstByte = blockBuffer.getByteStrictlyForward(position); + byte firstByte = blockBuffer.getByteAfterPosition(offsetFromPos); int len = WritableUtils.decodeVIntSize(firstByte); if (len == 1) { this.currMemstoreTS = firstByte; } else { long i = 0; + offsetFromPos++; for (int idx = 0; idx < len - 1; idx++) { - byte b = blockBuffer.get(position + 1 + idx); + byte b = blockBuffer.getByteAfterPosition(offsetFromPos + idx); i = i << 8; i = i | (b & 0xFF); } @@ -580,11 +581,6 @@ public class HFileReaderImpl implements HFile.Reader, Configurable { this.currMemstoreTSLen = len; } - protected void readMvccVersion() { - // TODO CLEANUP!!! - readMvccVersion(blockBuffer.arrayOffset() + blockBuffer.position()); - } - /** * Within a loaded block, seek looking for the last key that is smaller than * (or equal to?) the key we are interested in. @@ -603,11 +599,11 @@ public class HFileReaderImpl implements HFile.Reader, Configurable { protected int blockSeek(Cell key, boolean seekBefore) { int klen, vlen, tlen = 0; int lastKeyValueSize = -1; - int pos = -1; + int offsetFromPos; do { - pos = blockBuffer.position(); + offsetFromPos = 0; // Better to ensure that we use the BB Utils here - long ll = blockBuffer.getLongStrictlyForward(pos); + long ll = blockBuffer.getLongAfterPosition(offsetFromPos); klen = (int)(ll >> Integer.SIZE); vlen = (int)(Bytes.MASK_FOR_LOWER_INT_IN_LONG ^ ll); if (klen < 0 || vlen < 0 || klen > blockBuffer.limit() @@ -617,28 +613,28 @@ public class HFileReaderImpl implements HFile.Reader, Configurable { + block.getOffset() + ", block length: " + blockBuffer.limit() + ", position: " + blockBuffer.position() + " (without header)."); } - pos += Bytes.SIZEOF_LONG; - blockBuffer.asSubByteBuffer(pos, klen, pair); + offsetFromPos += Bytes.SIZEOF_LONG; + blockBuffer.asSubByteBuffer(blockBuffer.position() + offsetFromPos, klen, pair); // TODO :change here after Bufferbackedcells come keyOnlyKv.setKey(pair.getFirst().array(), pair.getFirst().arrayOffset() + pair.getSecond(), klen); int comp = reader.getComparator().compareKeyIgnoresMvcc(key, keyOnlyKv); - pos += klen + vlen; + offsetFromPos += klen + vlen; if (this.reader.getFileContext().isIncludesTags()) { // Read short as unsigned, high byte first - tlen = ((blockBuffer.getByteStrictlyForward(pos) & 0xff) << 8) - ^ (blockBuffer.getByteStrictlyForward(pos + 1) & 0xff); + tlen = ((blockBuffer.getByteAfterPosition(offsetFromPos) & 0xff) << 8) + ^ (blockBuffer.getByteAfterPosition(offsetFromPos + 1) & 0xff); if (tlen < 0 || tlen > blockBuffer.limit()) { throw new IllegalStateException("Invalid tlen " + tlen + ". Block offset: " + block.getOffset() + ", block length: " + blockBuffer.limit() + ", position: " + blockBuffer.position() + " (without header)."); } // add the two bytes read for the tags. - pos += tlen + (Bytes.SIZEOF_SHORT); + offsetFromPos += tlen + (Bytes.SIZEOF_SHORT); } if (this.reader.shouldIncludeMemstoreTS()) { // Directly read the mvcc based on current position - readMvccVersion(pos); + readMvccVersion(offsetFromPos); } if (comp == 0) { if (seekBefore) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java index bc82aee85df..5d9ac9d03df 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestHFileBlockIndex.java @@ -608,14 +608,14 @@ public class TestHFileBlockIndex { if (block.getBlockType() != BlockType.LEAF_INDEX) return; ByteBuff b = block.getBufferReadOnly(); - int n = b.getInt(); + int n = b.getIntAfterPosition(0); // One int for the number of items, and n + 1 for the secondary index. int entriesOffset = Bytes.SIZEOF_INT * (n + 2); // Get all the keys from the leaf index block. S for (int i = 0; i < n; ++i) { - int keyRelOffset = b.getIntStrictlyForward(Bytes.SIZEOF_INT * (i + 1)); - int nextKeyRelOffset = b.getIntStrictlyForward(Bytes.SIZEOF_INT * (i + 2)); + int keyRelOffset = b.getIntAfterPosition(Bytes.SIZEOF_INT * (i + 1)); + int nextKeyRelOffset = b.getIntAfterPosition(Bytes.SIZEOF_INT * (i + 2)); int keyLen = nextKeyRelOffset - keyRelOffset; int keyOffset = b.arrayOffset() + entriesOffset + keyRelOffset + HFileBlockIndex.SECONDARY_INDEX_ENTRY_OVERHEAD;