[GitHub] [lucene] jpountz commented on a diff in pull request #12381: Speed up NumericDocValuesWriter with index sorting

[via GitHub]

jpountz commented on code in PR #12381:
URL: https://github.com/apache/lucene/pull/12381#discussion_r1246499123


##########
lucene/core/src/java/org/apache/lucene/index/NumericDocValuesWriter.java:
##########
@@ -114,7 +121,12 @@ static DocValuesProducer getDocValuesProducer(
     final NumericDVs sorted;
     if (sortMap != null) {
       NumericDocValues oldValues = new BufferedNumericDocValues(values, 
docsWithField.iterator());
-      sorted = sortDocValues(sortMap.size(), sortMap, oldValues);
+      sorted =
+          sortDocValues(
+              sortMap.size(),
+              sortMap,
+              oldValues,
+              docsWithField.dense() && sortMap.size() == 
docsWithField.cardinality());

Review Comment:
   and likewise here?
   
   ```suggestion
                 docsWithField.dense());
   ```



##########
lucene/core/src/java/org/apache/lucene/index/NumericDocValuesWriter.java:
##########
@@ -234,10 +246,41 @@ public long cost() {
   static class NumericDVs {
     private final long[] values;
     private final BitSet docsWithField;
+    private final int maxDoc;
 
     NumericDVs(long[] values, BitSet docsWithField) {
       this.values = values;
       this.docsWithField = docsWithField;
+      this.maxDoc = values.length;
+    }
+
+    int maxDoc() {
+      return maxDoc;
+    }
+
+    private boolean advanceExact(int target) {
+      if (docsWithField != null) {
+        return docsWithField.get(target);
+      }
+      return true;
+    }
+
+    private int advance(int target) {
+      if (docsWithField != null) {
+        return docsWithField.nextSetBit(target);
+      }
+
+      if (target < maxDoc) {
+        return target;
+      }
+      return DocIdSetIterator.NO_MORE_DOCS;

Review Comment:
   Looking at the call site, this is only called when `target` is less than 
`maxDoc`, so thit could be simplified to just `return target` without the 
`target < maxDoc` check?



##########
lucene/core/src/java/org/apache/lucene/index/NormValuesWriter.java:
##########
@@ -76,7 +76,8 @@ public void flush(SegmentWriteState state, Sorter.DocMap 
sortMap, NormsConsumer
           NumericDocValuesWriter.sortDocValues(
               state.segmentInfo.maxDoc(),
               sortMap,
-              new BufferedNorms(values, docsWithField.iterator()));
+              new BufferedNorms(values, docsWithField.iterator()),
+              docsWithField.dense() && sortMap.size() == 
docsWithField.cardinality());

Review Comment:
   Only testing `dense()` should be correct right?
   
   ```suggestion
                 docsWithField.dense());
   ```



-- 
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: issues-unsubscr...@lucene.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org