maedhroz commented on code in PR #2989:
URL: https://github.com/apache/cassandra/pull/2989#discussion_r1430381757
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/SegmentRamBuffer.java:
##########
@@ -73,9 +73,9 @@ public long add(int segmentRowId, byte[] value)
return (trie.sizeOnHeap() - initialSizeOnHeap) +
(postingsAccumulator.heapAllocations() - reducerHeapSize);
}
- public BlockBalancedTreeIterator iterator()
+ public SegmentRamIterator iterator()
{
- return
BlockBalancedTreeIterator.fromTrieIterator(trie.entrySet().iterator(),
bytesPerValue);
+ return SegmentRamIterator.fromTrieIterator(trie.entrySet().iterator());
Review Comment:
What if we inlined `fromTrieIterator()` here and moved `fromTermsIterator()`
to `TermsIterator`, and then just got rid of the `SegmentRamIterator` in favor
of `Iterator<Pair<ByteComparable, PostingList>>`? `SegmentRamBuffer` isn't
bloated by any means, and we could even make it `Iterable` in that case, given
it already provides `iterator()`. `SegmentRamIterator` is really just half
"marker" interface and half utility class for static factories.
If `Iterator<Pair<ByteComparable, PostingList>>` feels to complicated, the
solution there has probably aways been to stop using `Pair` and have something
like `IndexEntry` or `TermAndPostings` (or something else that communicates
well).
--
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]
