adelapena commented on code in PR #2989:
URL: https://github.com/apache/cassandra/pull/2989#discussion_r1426655902
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/SegmentBuilder.java:
##########
@@ -78,31 +75,36 @@ public abstract class SegmentBuilder
int rowCount = 0;
int maxSegmentRowId = -1;
- public static class BlockBalancedTreeSegmentBuilder extends SegmentBuilder
+ public static abstract class RamBufferSegmentBuilder extends SegmentBuilder
{
- private final byte[] scratch;
- private final BlockBalancedTreeRamBuffer trieBuffer;
+ protected final SegmentRamBuffer segmentRamBuffer;
- public BlockBalancedTreeSegmentBuilder(IndexTermType indexTermType,
NamedMemoryLimiter limiter)
+ protected RamBufferSegmentBuilder(IndexTermType indexTermType,
NamedMemoryLimiter namedMemoryLimiter)
{
- super(indexTermType, limiter);
+ super(indexTermType,namedMemoryLimiter);
- scratch = new byte[indexTermType.fixedSizeOf()];
- trieBuffer = new
BlockBalancedTreeRamBuffer(indexTermType.fixedSizeOf());
- totalBytesAllocated = this.trieBuffer.memoryUsed();
+ segmentRamBuffer = new SegmentRamBuffer();
+ totalBytesAllocated = this.segmentRamBuffer.memoryUsed();
}
@Override
public boolean isEmpty()
{
- return trieBuffer.numRows() == 0;
+ return segmentRamBuffer.numRows() == 0;
+ }
+ }
+
+ public static class BlockBalancedTreeSegmentBuilder extends
RamBufferSegmentBuilder
+ {
+ public BlockBalancedTreeSegmentBuilder(IndexTermType indexTermType,
NamedMemoryLimiter limiter)
+ {
+ super(indexTermType, limiter);
}
@Override
protected long addInternal(ByteBuffer term, int segmentRowId)
{
- indexTermType.toComparableBytes(term, scratch);
- return trieBuffer.add(segmentRowId, scratch);
+ return segmentRamBuffer.add(v ->
indexTermType.asComparableBytes(term, v), term.limit(), segmentRowId);
Review Comment:
The only difference between the two implementations of `addInternal` in the
children of `RamBufferSegmentBuilder` is how the `ByteSource` term is built.
Maybe we could make that part abstract in `RamBufferSegmentBuilder` and provide
a common implementation of `addInternal` in `RamBufferSegmentBuilder`. So
`RamBufferSegmentBuilder` would contain:
```java
protected abstract ByteSource asBytes(ByteBuffer term,
ByteComparable.Version version);
@Override
protected long addInternal(ByteBuffer term, int segmentRowId)
{
return segmentRamBuffer.add(v -> asBytes(term, v), term.limit(),
segmentRowId);
}
```
Then in `BlockBalancedTreeSegmentBuilder`:
```java
@Override
protected ByteSource asBytes(ByteBuffer term, ByteComparable.Version version)
{
return indexTermType.asComparableBytes(term, version);
}
```
And in `LiteralSegmentBuilder`:
```java
@Override
protected ByteSource asBytes(ByteBuffer term, ByteComparable.Version version)
{
return ByteSource.fixedLength(term);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
