diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java index b8e4640..6216193 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java @@ -790,16 +790,9 @@ public class HFileReaderV2 extends AbstractHFileReader { } /** - * Go to the next key/value in the block section. Loads the next block if - * necessary. If successful, {@link #getKey()} and {@link #getValue()} can - * be called. - * - * @return true if successfully navigated to the next key/value + * Set the position on current backing blockBuffer. */ - @Override - public boolean next() throws IOException { - assertSeeked(); - + private void positionThisBlockBuffer() { try { blockBuffer.position(getNextCellStartPosition()); } catch (IllegalArgumentException e) { @@ -810,32 +803,60 @@ public class HFileReaderV2 extends AbstractHFileReader { + "; currBlock currBlockOffset = " + block.getOffset()); throw e; } + } - if (blockBuffer.remaining() <= 0) { - long lastDataBlockOffset = - reader.getTrailer().getLastDataBlockOffset(); - - if (block.getOffset() >= lastDataBlockOffset) { - setNonSeekedState(); - return false; - } - - // read the next block - HFileBlock nextBlock = readNextDataBlock(); - if (nextBlock == null) { - setNonSeekedState(); - return false; - } + /** + * Set our selves up for the next 'next' invocation, set up next block. + * @return True is more to read else false if at the end. + * @throws IOException + */ + private boolean positionForNextBlock() throws IOException { + // Methods are small so they get inlined because they are 'hot'. + long lastDataBlockOffset = reader.getTrailer().getLastDataBlockOffset(); + if (block.getOffset() >= lastDataBlockOffset) { + setNonSeekedState(); + return false; + } + return isNextBlock(); + } - updateCurrBlock(nextBlock); - return true; + private boolean isNextBlock() throws IOException { + // Methods are small so they get inlined because they are 'hot'. + HFileBlock nextBlock = readNextDataBlock(); + if (nextBlock == null) { + setNonSeekedState(); + return false; } + updateCurrBlock(nextBlock); + return true; + } + private final boolean _next() throws IOException { + // Small method so can be inlined. Its a hot one. + if (blockBuffer.remaining() <= 0) { + return positionForNextBlock(); + } // We are still in the same block. readKeyValueLen(); return true; } + /** + * Go to the next key/value in the block section. Loads the next block if + * necessary. If successful, {@link #getKey()} and {@link #getValue()} can + * be called. + * + * @return true if successfully navigated to the next key/value + */ + @Override + public boolean next() throws IOException { + // This is a hot method so extreme measures taken to ensure it is small and inlineable. + // Checked by setting: -XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining -XX:+PrintCompilation + assertSeeked(); + positionThisBlockBuffer(); + return _next(); + } + protected int getNextCellStartPosition() { return blockBuffer.position() + KEY_VALUE_LEN_SIZE + currKeyLen + currValueLen + currMemstoreTSLen; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV3.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV3.java index b28d8c1..d67540d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV3.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV3.java @@ -203,30 +203,49 @@ public class HFileReaderV3 extends HFileReaderV2 { return nextKvPos; } - protected void readKeyValueLen() { - blockBuffer.mark(); - currKeyLen = blockBuffer.getInt(); - currValueLen = blockBuffer.getInt(); - if (currKeyLen < 0 || currValueLen < 0 || currKeyLen > blockBuffer.limit() - || currValueLen > blockBuffer.limit()) { - throw new IllegalStateException("Invalid currKeyLen " + currKeyLen + " or currValueLen " - + currValueLen + ". Block offset: " - + block.getOffset() + ", block length: " + blockBuffer.limit() + ", position: " - + blockBuffer.position() + " (without header)."); + private final void setAndCheckKeyValueLen() { + this.currKeyLen = blockBuffer.getInt(); + this.currValueLen = blockBuffer.getInt(); + if (checkLen(this.currKeyLen) || checkLen(this.currValueLen)) { + throw new IllegalStateException("Invalid currKeyLen " + this.currKeyLen + + " or currValueLen " + this.currValueLen + ". Block offset: " + block.getOffset() + + ", block length: " + this.blockBuffer.limit() + ", position: " + + this.blockBuffer.position() + " (without header)."); } - ByteBufferUtils.skip(blockBuffer, currKeyLen + currValueLen); - if (reader.hfileContext.isIncludesTags()) { - // Read short as unsigned, high byte first - currTagsLen = ((blockBuffer.get() & 0xff) << 8) ^ (blockBuffer.get() & 0xff); - if (currTagsLen < 0 || currTagsLen > blockBuffer.limit()) { - throw new IllegalStateException("Invalid currTagsLen " + currTagsLen + ". Block offset: " - + block.getOffset() + ", block length: " + blockBuffer.limit() + ", position: " - + blockBuffer.position() + " (without header)."); - } - ByteBufferUtils.skip(blockBuffer, currTagsLen); + } + + /** + * @param v + * @return True if v < 0 or v > current block buffer limit. + */ + private final boolean checkLen(final long v) { + return v < 0 || v > this.blockBuffer.limit(); + } + + private final void setAndCheckTagsLen() { + if (!reader.hfileContext.isIncludesTags()) return; + // Read short as unsigned, high byte first + this.currTagsLen = ((this.blockBuffer.get() & 0xff) << 8) ^ (this.blockBuffer.get() & 0xff); + if (this.currTagsLen < 0 || this.currTagsLen > this.blockBuffer.limit()) { + throw new IllegalStateException("Invalid currTagsLen " + this.currTagsLen + + ". Block offset: " + block.getOffset() + ", block length: " + this.blockBuffer.limit() + + ", position: " + this.blockBuffer.position() + " (without header)."); + } + ByteBufferUtils.skip(this.blockBuffer, this.currTagsLen); + } + + protected final void readKeyValueLen() { + // This is a hot method according to -XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining. + // We do stuff in here to make the method short so can be inlined. + this.blockBuffer.mark(); + try { + setAndCheckKeyValueLen(); + ByteBufferUtils.skip(this.blockBuffer, this.currKeyLen + this.currValueLen); + setAndCheckTagsLen(); + readMvccVersion(); + } finally { + this.blockBuffer.reset(); } - readMvccVersion(); - blockBuffer.reset(); } /**