[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10372: Let applyAnd to be applied using different window sizes

["Jackie-Jiang (via GitHub)" <gi...@apache.org>]

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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org