Skip to content

Commit 6eb2d5d

Browse files
committed
Merge branch 'cassandra-6.0' into trunk
* cassandra-6.0: Avoid megamorphic call overhead at RandomAccessReader#current
2 parents e6064f1 + c0999f0 commit 6eb2d5d

3 files changed

Lines changed: 40 additions & 12 deletions

File tree

CHANGES.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Allow nodetool garbagecollect to take a user defined list of SSTables (CASSANDRA-16767)
44
* Add a guardrail for misprepared statements (CASSANDRA-21139)
55
Merged from 6.0:
6+
* Avoid megamorphic call overhead at RandomAccessReader#current (CASSANDRA-21399)
67
* Enable async GC logging for JDK versions which support it to avoid potential hiccups caused by GC log file I/O blocking (CASSANDRA-21372)
78
* Add rowsMutatedPerWriteHistogram metric to track rows mutated per write request (CASSANDRA-21320)
89
* 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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ static ReaderSupplier createSupplier(FileHandle fileHandle)
4646
@Override
4747
public void readFully(float[] dest) throws IOException
4848
{
49-
BufferHolder bh = bufferHolder;
49+
BufferHolder bh = getBufferHolder();
5050
long position = getPosition();
5151

5252
FloatBuffer floatBuffer;
53-
if (bh.offset() == 0 && position % Float.BYTES == 0)
53+
if (getBufferHolderOffset() == 0 && position % Float.BYTES == 0)
5454
{
5555
// this is a separate code path because buffer() and asFloatBuffer() both allocate
5656
// new and relatively expensive xBuffer objects, so we want to avoid doing that
@@ -63,7 +63,7 @@ public void readFully(float[] dest) throws IOException
6363
// offset is non-zero, and probably not aligned to Float.BYTES, so
6464
// set the position before converting to FloatBuffer.
6565
ByteBuffer bb = bh.buffer();
66-
bb.position(Ints.checkedCast(position - bh.offset()));
66+
bb.position(Ints.checkedCast(position - getBufferHolderOffset()));
6767
floatBuffer = bb.asFloatBuffer();
6868
}
6969

@@ -94,11 +94,11 @@ public void read(int[] dest, int offset, int count) throws IOException
9494
if (count == 0)
9595
return;
9696

97-
BufferHolder bh = bufferHolder;
97+
BufferHolder bh = getBufferHolder();
9898
long position = getPosition();
9999

100100
IntBuffer intBuffer;
101-
if (bh.offset() == 0 && position % Integer.BYTES == 0)
101+
if (getBufferHolderOffset() == 0 && position % Integer.BYTES == 0)
102102
{
103103
// this is a separate code path because buffer() and asIntBuffer() both allocate
104104
// new and relatively expensive xBuffer objects, so we want to avoid doing that
@@ -111,7 +111,7 @@ public void read(int[] dest, int offset, int count) throws IOException
111111
// offset is non-zero, and probably not aligned to Integer.BYTES, so
112112
// set the position before converting to IntBuffer.
113113
ByteBuffer bb = bh.buffer();
114-
bb.position(Ints.checkedCast(position - bh.offset()));
114+
bb.position(Ints.checkedCast(position - getBufferHolderOffset()));
115115
intBuffer = bb.asIntBuffer();
116116
}
117117

src/java/org/apache/cassandra/io/util/RandomAccessReader.java

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import javax.annotation.concurrent.NotThreadSafe;
2424

25+
import com.google.common.annotations.VisibleForTesting;
2526
import com.google.common.base.Preconditions;
2627
import com.google.common.primitives.Ints;
2728

@@ -38,7 +39,18 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
3839
private long markedPointer;
3940

4041
final Rebufferer rebufferer;
41-
protected BufferHolder bufferHolder = Rebufferer.EMPTY;
42+
43+
// It is made private to prevent subclasses from modifying the value
44+
// without also updating the corresponding bufferHolderOffset
45+
private BufferHolder bufferHolder = Rebufferer.EMPTY;
46+
47+
// The value is cached to avoid megamorphic calls of bufferHolder.
48+
// It must be updated in sync with bufferHolder value
49+
private long bufferHolderOffset = 0;
50+
51+
// The value is cached to avoid megamorphic calls of bufferHolder
52+
// it is immutable, the SSTable early-open mechanism does not mutate existing readers
53+
private final long fileLength;
4254

4355
/**
4456
* Only created through Builder
@@ -49,6 +61,7 @@ protected RandomAccessReader(Rebufferer rebufferer)
4961
{
5062
super(Rebufferer.EMPTY.buffer());
5163
this.rebufferer = rebufferer;
64+
this.fileLength = rebufferer.fileLength();
5265
}
5366

5467
/**
@@ -67,7 +80,9 @@ private void reBufferAt(long position)
6780
bufferHolder.release();
6881
bufferHolder = rebufferer.rebuffer(position);
6982
buffer = bufferHolder.buffer();
70-
buffer.position(Ints.checkedCast(position - bufferHolder.offset()));
83+
long newOffset = bufferHolder.offset();
84+
bufferHolderOffset = newOffset;
85+
buffer.position(Ints.checkedCast(position - newOffset));
7186

7287
assert buffer.order() == ByteOrder.BIG_ENDIAN : "Buffer must have BIG ENDIAN byte ordering";
7388
}
@@ -76,13 +91,23 @@ private void reBufferAt(long position)
7691
public long getFilePointer()
7792
{
7893
if (buffer == null) // closed already
79-
return rebufferer.fileLength();
94+
return fileLength;
8095
return current();
8196
}
8297

98+
protected BufferHolder getBufferHolder()
99+
{
100+
return bufferHolder;
101+
}
102+
103+
protected long getBufferHolderOffset()
104+
{
105+
return bufferHolderOffset;
106+
}
107+
83108
protected long current()
84109
{
85-
return bufferHolder.offset() + buffer.position();
110+
return bufferHolderOffset + buffer.position();
86111
}
87112

88113
public String getPath()
@@ -164,6 +189,7 @@ public void close()
164189
rebufferer.closeReader();
165190
buffer = null;
166191
bufferHolder = null;
192+
bufferHolderOffset = 0;
167193

168194
//For performance reasons we don't keep a reference to the file
169195
//channel so we don't close it
@@ -197,7 +223,7 @@ public void seek(long newPosition)
197223
if (buffer == null)
198224
throw new IllegalStateException("Attempted to seek in a closed RAR");
199225

200-
long bufferOffset = bufferHolder.offset();
226+
long bufferOffset = bufferHolderOffset;
201227
if (newPosition >= bufferOffset && newPosition < bufferOffset + buffer.limit())
202228
{
203229
buffer.position((int) (newPosition - bufferOffset));
@@ -274,14 +300,15 @@ public final String readLine() throws IOException
274300

275301
public long length()
276302
{
277-
return rebufferer.fileLength();
303+
return fileLength;
278304
}
279305

280306
public long getPosition()
281307
{
282308
return current();
283309
}
284310

311+
@VisibleForTesting
285312
public double getCrcCheckChance()
286313
{
287314
return rebufferer.getCrcCheckChance();

0 commit comments

Comments
 (0)