You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by be...@apache.org on 2026/05/15 16:29:46 UTC

(lucene) branch main updated: Defer Sorter.DocMap packing until after flush (#16048)

This is an automated email from the ASF dual-hosted git repository.

benwtrent pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/lucene.git


The following commit(s) were added to refs/heads/main by this push:
     new de92f115a31 Defer Sorter.DocMap packing until after flush (#16048)
de92f115a31 is described below

commit de92f115a3125624bf7c7141b8e700efa9a89427
Author: Tim Brooks <ti...@timbrooks.org>
AuthorDate: Fri May 15 10:29:41 2026 -0600

    Defer Sorter.DocMap packing until after flush (#16048)
    
    * Defer Sorter.DocMap packing until after flush
    
    IndexingChain calls sortMap.oldToNew heavily during flush — per posting
    in FreqProxTermsWriter, per (field, doc) in DV writers, per vector in
    KNN writers. The previous implementation built oldToNew as a
    PackedLongValues, which is slow for that random-access hot path.
    
    Build oldToNew as an int[] during flush, then pack to PackedLongValues
    before storing on FlushedSegment, which is retained long-term on
    ReadersAndUpdates for sorted segments with seg-private updates.
    
    * Changes
---
 lucene/CHANGES.txt                                 |  1 +
 .../lucene/index/DocumentsWriterPerThread.java     | 10 ++-
 .../org/apache/lucene/index/IndexingChain.java     |  8 +-
 .../src/java/org/apache/lucene/index/Sorter.java   | 89 ++++++++++++++++------
 4 files changed, 75 insertions(+), 33 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 3f65f1956cb..80ba771596a 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -379,6 +379,7 @@ Optimizations
 * GITHUB#16061, GITHUB#16070: Improve cost estimation in SortedSetDocValuesRangeQuery when using DocValuesSkipper
   and the field is dense and is the primary sort of the index to reduce the number of doc values visited. (Ignacio Vera)
 
+* GITHUB#16048: Defer Sorter.DocMap packing until after flush (Tim Brooks)
 
 Bug Fixes
 ---------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java b/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
index 342a2359f5c..bb4c444b1cb 100644
--- a/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
+++ b/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
@@ -481,7 +481,7 @@ final class DocumentsWriterPerThread implements Accountable, Lock {
           "DWPT",
           "flush postings as segment " + flushState.segmentInfo.name + " numDocs=" + numDocsInRAM);
     }
-    final Sorter.DocMap sortMap;
+    final Sorter.PackableDocMap packableSortMap;
     try {
       DocIdSetIterator softDeletedDocs;
       if (indexWriterConfig.getSoftDeletesField() != null) {
@@ -489,7 +489,7 @@ final class DocumentsWriterPerThread implements Accountable, Lock {
       } else {
         softDeletedDocs = null;
       }
-      sortMap = indexingChain.flush(flushState);
+      packableSortMap = indexingChain.flush(flushState);
       if (softDeletedDocs == null) {
         flushState.softDelCountOnFlush = 0;
       } else {
@@ -570,8 +570,10 @@ final class DocumentsWriterPerThread implements Accountable, Lock {
               segmentDeletes,
               flushState.liveDocs,
               flushState.delCountOnFlush,
-              sortMap);
-      sealFlushedSegment(fs, sortMap, flushNotifications);
+              packableSortMap != null
+                  ? packableSortMap.pack()
+                  : null); // Use a packed version as the lifetime of FlushedSegment is long.
+      sealFlushedSegment(fs, packableSortMap, flushNotifications);
       if (infoStream.isEnabled("DWPT")) {
         infoStream.message(
             "DWPT",
diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexingChain.java b/lucene/core/src/java/org/apache/lucene/index/IndexingChain.java
index 3a022127bd2..c6997f0d0e7 100644
--- a/lucene/core/src/java/org/apache/lucene/index/IndexingChain.java
+++ b/lucene/core/src/java/org/apache/lucene/index/IndexingChain.java
@@ -237,7 +237,7 @@ final class IndexingChain implements Accountable {
     };
   }
 
-  private Sorter.DocMap maybeSortSegment(SegmentWriteState state) throws IOException {
+  private Sorter.PackableDocMap maybeSortSegment(SegmentWriteState state) throws IOException {
     Sort indexSort = state.segmentInfo.getIndexSort();
     if (indexSort == null) {
       return null;
@@ -283,15 +283,15 @@ final class IndexingChain implements Accountable {
     }
     Sorter sorter = new Sorter(indexSort);
     // returns null if the documents are already sorted
-    return sorter.sort(
+    return sorter.sortAndLeaveUnpacked(
         state.segmentInfo.maxDoc(), comparators.toArray(IndexSorter.DocComparator[]::new));
   }
 
-  Sorter.DocMap flush(SegmentWriteState state) throws IOException {
+  Sorter.PackableDocMap flush(SegmentWriteState state) throws IOException {
 
     // NOTE: caller (DocumentsWriterPerThread) handles
     // aborting on any exception from this method
-    Sorter.DocMap sortMap = maybeSortSegment(state);
+    Sorter.PackableDocMap sortMap = maybeSortSegment(state);
     int maxDoc = state.segmentInfo.maxDoc();
     long t0 = System.nanoTime();
     writeNorms(state, sortMap);
diff --git a/lucene/core/src/java/org/apache/lucene/index/Sorter.java b/lucene/core/src/java/org/apache/lucene/index/Sorter.java
index 619496d108d..c6bbe0e8e8e 100644
--- a/lucene/core/src/java/org/apache/lucene/index/Sorter.java
+++ b/lucene/core/src/java/org/apache/lucene/index/Sorter.java
@@ -129,7 +129,7 @@ public final class Sorter {
   }
 
   /** Computes the old-to-new permutation over the given comparator. */
-  private static Sorter.DocMap sort(final int maxDoc, IndexSorter.DocComparator comparator) {
+  private static PackableDocMap sort(final int maxDoc, IndexSorter.DocComparator comparator) {
     // check if the index is sorted
     boolean sorted = true;
     for (int i = 1; i < maxDoc; ++i) {
@@ -168,30 +168,7 @@ public final class Sorter {
       docs[(int) newToOld.get(i)] = i;
     } // docs is now the oldToNew mapping
 
-    final PackedLongValues.Builder oldToNewBuilder =
-        PackedLongValues.monotonicBuilder(PackedInts.COMPACT);
-    for (int i = 0; i < maxDoc; ++i) {
-      oldToNewBuilder.add(docs[i]);
-    }
-    final PackedLongValues oldToNew = oldToNewBuilder.build();
-
-    return new Sorter.DocMap() {
-
-      @Override
-      public int oldToNew(int docID) {
-        return (int) oldToNew.get(docID);
-      }
-
-      @Override
-      public int newToOld(int docID) {
-        return (int) newToOld.get(docID);
-      }
-
-      @Override
-      public int size() {
-        return maxDoc;
-      }
-    };
+    return new PackableDocMap(docs, newToOld, maxDoc);
   }
 
   /**
@@ -241,6 +218,12 @@ public final class Sorter {
   }
 
   DocMap sort(int maxDoc, IndexSorter.DocComparator[] comparators) throws IOException {
+    PackableDocMap packableDocMap = sortAndLeaveUnpacked(maxDoc, comparators);
+    return packableDocMap != null ? packableDocMap.pack() : packableDocMap;
+  }
+
+  PackableDocMap sortAndLeaveUnpacked(int maxDoc, IndexSorter.DocComparator[] comparators)
+      throws IOException {
     final IndexSorter.DocComparator comparator =
         (docID1, docID2) -> {
           for (int i = 0; i < comparators.length; i++) {
@@ -270,4 +253,60 @@ public final class Sorter {
   public String toString() {
     return getID();
   }
+
+  /** A {@link DocMap} that can keep oldToNew in either packed or unpacked form. */
+  static final class PackableDocMap extends DocMap {
+
+    private final PackedLongValues newToOld;
+    private final int maxDoc;
+
+    private final int[] oldToNewUnpacked;
+    private final PackedLongValues oldToNewPacked;
+
+    private PackableDocMap(int[] oldToNewUnpacked, PackedLongValues newToOld, int maxDoc) {
+      this.oldToNewUnpacked = oldToNewUnpacked;
+      this.oldToNewPacked = null;
+      this.newToOld = newToOld;
+      this.maxDoc = maxDoc;
+    }
+
+    private PackableDocMap(PackedLongValues oldToNewPacked, PackedLongValues newToOld, int maxDoc) {
+      this.oldToNewUnpacked = null;
+      this.oldToNewPacked = oldToNewPacked;
+      this.newToOld = newToOld;
+      this.maxDoc = maxDoc;
+    }
+
+    @Override
+    public int oldToNew(int docID) {
+      if (oldToNewUnpacked != null) {
+        return oldToNewUnpacked[docID];
+      } else {
+        return (int) oldToNewPacked.get(docID);
+      }
+    }
+
+    @Override
+    public int newToOld(int docID) {
+      return (int) newToOld.get(docID);
+    }
+
+    /** Pack the internal representations into more compact forms. No-op if already packed. */
+    DocMap pack() {
+      if (oldToNewPacked != null) {
+        return this;
+      }
+      final PackedLongValues.Builder oldToNewBuilder =
+          PackedLongValues.monotonicBuilder(PackedInts.COMPACT);
+      for (int i = 0; i < maxDoc; ++i) {
+        oldToNewBuilder.add(oldToNewUnpacked[i]);
+      }
+      return new PackableDocMap(oldToNewBuilder.build(), newToOld, maxDoc);
+    }
+
+    @Override
+    public int size() {
+      return maxDoc;
+    }
+  }
 }