diff --git a/CHANGES.txt b/CHANGES.txt index 6716235022..2b5f146be8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -3,6 +3,7 @@ * Allow nodetool garbagecollect to take a user defined list of SSTables (CASSANDRA-16767) * Add a guardrail for misprepared statements (CASSANDRA-21139) Merged from 6.0: + * Avoid megamorphic call overhead at RandomAccessReader#current (CASSANDRA-21399) * Enable async GC logging for JDK versions which support it to avoid potential hiccups caused by GC log file I/O blocking (CASSANDRA-21372) * Add rowsMutatedPerWriteHistogram metric to track rows mutated per write request (CASSANDRA-21320) * Add TotalRowsRead and TotalRowsMutated counters to TableMetrics for accurate per-table row throughput tracking (CASSANDRA-21321) diff --git a/src/java/org/apache/cassandra/index/sai/disk/v1/vector/RandomAccessReaderAdapter.java b/src/java/org/apache/cassandra/index/sai/disk/v1/vector/RandomAccessReaderAdapter.java index 7448dc0648..64adaf2bfb 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/v1/vector/RandomAccessReaderAdapter.java +++ b/src/java/org/apache/cassandra/index/sai/disk/v1/vector/RandomAccessReaderAdapter.java @@ -46,11 +46,11 @@ public class RandomAccessReaderAdapter extends RandomAccessReader implements io. @Override public void readFully(float[] dest) throws IOException { - BufferHolder bh = bufferHolder; + BufferHolder bh = getBufferHolder(); long position = getPosition(); FloatBuffer floatBuffer; - if (bh.offset() == 0 && position % Float.BYTES == 0) + if (getBufferHolderOffset() == 0 && position % Float.BYTES == 0) { // this is a separate code path because buffer() and asFloatBuffer() both allocate // new and relatively expensive xBuffer objects, so we want to avoid doing that @@ -63,7 +63,7 @@ public class RandomAccessReaderAdapter extends RandomAccessReader implements io. // offset is non-zero, and probably not aligned to Float.BYTES, so // set the position before converting to FloatBuffer. ByteBuffer bb = bh.buffer(); - bb.position(Ints.checkedCast(position - bh.offset())); + bb.position(Ints.checkedCast(position - getBufferHolderOffset())); floatBuffer = bb.asFloatBuffer(); } @@ -94,11 +94,11 @@ public class RandomAccessReaderAdapter extends RandomAccessReader implements io. if (count == 0) return; - BufferHolder bh = bufferHolder; + BufferHolder bh = getBufferHolder(); long position = getPosition(); IntBuffer intBuffer; - if (bh.offset() == 0 && position % Integer.BYTES == 0) + if (getBufferHolderOffset() == 0 && position % Integer.BYTES == 0) { // this is a separate code path because buffer() and asIntBuffer() both allocate // new and relatively expensive xBuffer objects, so we want to avoid doing that @@ -111,7 +111,7 @@ public class RandomAccessReaderAdapter extends RandomAccessReader implements io. // offset is non-zero, and probably not aligned to Integer.BYTES, so // set the position before converting to IntBuffer. ByteBuffer bb = bh.buffer(); - bb.position(Ints.checkedCast(position - bh.offset())); + bb.position(Ints.checkedCast(position - getBufferHolderOffset())); intBuffer = bb.asIntBuffer(); } diff --git a/src/java/org/apache/cassandra/io/util/RandomAccessReader.java b/src/java/org/apache/cassandra/io/util/RandomAccessReader.java index eb5a182402..44b2f0c2fd 100644 --- a/src/java/org/apache/cassandra/io/util/RandomAccessReader.java +++ b/src/java/org/apache/cassandra/io/util/RandomAccessReader.java @@ -22,6 +22,7 @@ import java.nio.ByteOrder; import javax.annotation.concurrent.NotThreadSafe; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.primitives.Ints; @@ -38,7 +39,18 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa private long markedPointer; final Rebufferer rebufferer; - protected BufferHolder bufferHolder = Rebufferer.EMPTY; + + // It is made private to prevent subclasses from modifying the value + // without also updating the corresponding bufferHolderOffset + private BufferHolder bufferHolder = Rebufferer.EMPTY; + + // The value is cached to avoid megamorphic calls of bufferHolder. + // It must be updated in sync with bufferHolder value + private long bufferHolderOffset = 0; + + // The value is cached to avoid megamorphic calls of bufferHolder + // it is immutable, the SSTable early-open mechanism does not mutate existing readers + private final long fileLength; /** * Only created through Builder @@ -49,6 +61,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa { super(Rebufferer.EMPTY.buffer()); this.rebufferer = rebufferer; + this.fileLength = rebufferer.fileLength(); } /** @@ -67,7 +80,9 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa bufferHolder.release(); bufferHolder = rebufferer.rebuffer(position); buffer = bufferHolder.buffer(); - buffer.position(Ints.checkedCast(position - bufferHolder.offset())); + long newOffset = bufferHolder.offset(); + bufferHolderOffset = newOffset; + buffer.position(Ints.checkedCast(position - newOffset)); assert buffer.order() == ByteOrder.BIG_ENDIAN : "Buffer must have BIG ENDIAN byte ordering"; } @@ -76,13 +91,23 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa public long getFilePointer() { if (buffer == null) // closed already - return rebufferer.fileLength(); + return fileLength; return current(); } + protected BufferHolder getBufferHolder() + { + return bufferHolder; + } + + protected long getBufferHolderOffset() + { + return bufferHolderOffset; + } + protected long current() { - return bufferHolder.offset() + buffer.position(); + return bufferHolderOffset + buffer.position(); } public String getPath() @@ -164,6 +189,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa rebufferer.closeReader(); buffer = null; bufferHolder = null; + bufferHolderOffset = 0; //For performance reasons we don't keep a reference to the file //channel so we don't close it @@ -197,7 +223,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa if (buffer == null) throw new IllegalStateException("Attempted to seek in a closed RAR"); - long bufferOffset = bufferHolder.offset(); + long bufferOffset = bufferHolderOffset; if (newPosition >= bufferOffset && newPosition < bufferOffset + buffer.limit()) { buffer.position((int) (newPosition - bufferOffset)); @@ -274,7 +300,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa public long length() { - return rebufferer.fileLength(); + return fileLength; } public long getPosition() @@ -282,6 +308,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa return current(); } + @VisibleForTesting public double getCrcCheckChance() { return rebufferer.getCrcCheckChance();