Jackie-Jiang commented on code in PR #10372:
URL: https://github.com/apache/pinot/pull/10372#discussion_r1129923530
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/ScanBasedDocIdIterator.java:
##########
@@ -34,10 +36,17 @@
*/
public interface ScanBasedDocIdIterator extends BlockDocIdIterator {
+ MutableRoaringBitmap applyAnd(BatchIterator batchIterator, OptionalInt
firstDoc, OptionalInt lastDoc);
Review Comment:
Trying to understand why do we need to add this API. The `BatchIterator`
should always be returned from the roaring bitmap. If you need to pass
`RoaringBitmap` here, you may modify the existing `applyAnd()` to take an
`ImmutableBitmapDataProvider`.
In order to apply a different start/end to the bitmap, we may do an AND
operation to the bitmap with the range.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/MVScanDocIdIterator.java:
##########
@@ -47,15 +47,17 @@ public final class MVScanDocIdIterator implements
ScanBasedDocIdIterator {
private int _nextDocId = 0;
private long _numEntriesScanned = 0L;
+ private final int _batchSize;
- public MVScanDocIdIterator(PredicateEvaluator predicateEvaluator, DataSource
dataSource, int numDocs) {
+ public MVScanDocIdIterator(PredicateEvaluator predicateEvaluator, DataSource
dataSource, int numDocs, int batchSize) {
Review Comment:
Custom batch size might not apply here since this batch is only used when
iterating over the bitmap, where 256 might be the optimal one. It won't reduce
the time of index access
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java:
##########
@@ -44,16 +46,19 @@ public final class SVScanDocIdIterator implements
ScanBasedDocIdIterator {
private final ForwardIndexReaderContext _readerContext;
private final int _numDocs;
private final ValueMatcher _valueMatcher;
- private final int[] _batch = new int[OPTIMAL_ITERATOR_BATCH_SIZE];
+ private final int[] _batch;
private int _firstMismatch;
private int _cursor;
private final int _cardinality;
private int _nextDocId = 0;
private long _numEntriesScanned = 0L;
+ private final int _batchSize;
Review Comment:
(nit) Keep the final variables together for readability
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java:
##########
@@ -87,7 +94,7 @@ public int next() {
int limit;
int batchSize = 0;
do {
- limit = Math.min(_numDocs - _nextDocId, OPTIMAL_ITERATOR_BATCH_SIZE);
+ limit = Math.min(_numDocs - _nextDocId, _batch.length);
Review Comment:
(nit)
```suggestion
limit = Math.min(_numDocs - _nextDocId, _batchSize);
```
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java:
##########
@@ -280,11 +295,12 @@ public int matchValues(int limit, int[] docIds) {
private class DictIdMatcherAndNullHandler implements ValueMatcher {
- private final int[] _buffer = new int[OPTIMAL_ITERATOR_BATCH_SIZE];
+ private final int[] _buffer;
private final ImmutableRoaringBitmap _nullBitmap;
- public DictIdMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
+ public DictIdMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap, int
batchSize) {
Review Comment:
(minor) We don't need to pass it since it is a local variable
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
