Avoid megamorphic call overhead at RandomAccessReader#current

patch by Dmitry Konstantinov; reviewed by Stefan Miklosovic, Francisco Guerrero for CASSANDRA-21399

CHANGES.txt

@@ -1,4 +1,5 @@
 6.0-alpha2
+ * 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)

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();
         }
 

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();