From d0d0ec7fc38c4ff6ee9d033b316253736be998d7 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 15 May 2026 12:08:46 -0600 Subject: [PATCH 1/3] Remove dead code and simplify SortedDocIDMerger - Drop the `else if (queue.size() > 0)` guard in next(): the queue is guaranteed non-empty when that branch is reached, since an empty queue sets queueMinDocID to NO_MORE_DOCS and any valid nextDoc would have taken the fast path. Add a comment explaining the invariant. - Simplify reset() by handling the first sub before the loop, removing the `boolean first` flag and the dead `current = null` assignment. --- .../org/apache/lucene/index/DocIDMerger.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java b/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java index bf01fcdd497c..eab40de454d1 100644 --- a/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java +++ b/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java @@ -20,6 +20,7 @@ import static org.apache.lucene.search.DocIdSetIterator.NO_MORE_DOCS; import java.io.IOException; +import java.util.Iterator; import java.util.List; import org.apache.lucene.search.DocIdSetIterator; // javadocs import org.apache.lucene.util.PriorityQueue; @@ -166,16 +167,15 @@ private void setQueueMinDocID() { public void reset() throws IOException { // caller may not have fully consumed the queue: queue.clear(); - current = null; - boolean first = true; - for (T sub : subs) { - if (first) { - // by setting mappedDocID = -1, this entry is guaranteed to be the top of the queue - // so the first call to next() will advance it - sub.mappedDocID = -1; - current = sub; - first = false; - } else if (sub.nextMappedDoc() != NO_MORE_DOCS) { + Iterator it = subs.iterator(); + T sub = it.next(); + // by setting mappedDocID = -1, this entry is guaranteed to be less than any real doc ID + // so the first call to next() will advance it + sub.mappedDocID = -1; + current = sub; + while (it.hasNext()) { + sub = it.next(); + if (sub.nextMappedDoc() != NO_MORE_DOCS) { queue.add(sub); } // else all docs in this sub were deleted; do not add it to the queue! } @@ -197,7 +197,9 @@ public T next() throws IOException { } else { current = queue.pop(); } - } else if (queue.size() > 0) { + } else { + // queue cannot be empty here: if it were, queueMinDocID == NO_MORE_DOCS, and any valid + // nextDoc would satisfy nextDoc < queueMinDocID, taking the fast path above assert queueMinDocID == queue.top().mappedDocID; assert nextDoc > queueMinDocID; T newCurrent = queue.top(); From df4526674fbf95b03140fa4b935a761303cb348f Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 15 May 2026 12:22:50 -0600 Subject: [PATCH 2/3] Change --- .../org/apache/lucene/index/DocIDMerger.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java b/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java index eab40de454d1..463dfee7ff91 100644 --- a/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java +++ b/lucene/core/src/java/org/apache/lucene/index/DocIDMerger.java @@ -20,7 +20,6 @@ import static org.apache.lucene.search.DocIdSetIterator.NO_MORE_DOCS; import java.io.IOException; -import java.util.Iterator; import java.util.List; import org.apache.lucene.search.DocIdSetIterator; // javadocs import org.apache.lucene.util.PriorityQueue; @@ -167,15 +166,16 @@ private void setQueueMinDocID() { public void reset() throws IOException { // caller may not have fully consumed the queue: queue.clear(); - Iterator it = subs.iterator(); - T sub = it.next(); - // by setting mappedDocID = -1, this entry is guaranteed to be less than any real doc ID - // so the first call to next() will advance it - sub.mappedDocID = -1; - current = sub; - while (it.hasNext()) { - sub = it.next(); - if (sub.nextMappedDoc() != NO_MORE_DOCS) { + current = null; + boolean first = true; + for (T sub : subs) { + if (first) { + // by setting mappedDocID = -1, this entry is guaranteed to be the top of the queue + // so the first call to next() will advance it + sub.mappedDocID = -1; + current = sub; + first = false; + } else if (sub.nextMappedDoc() != NO_MORE_DOCS) { queue.add(sub); } // else all docs in this sub were deleted; do not add it to the queue! } From c11005d1811b6cd8246bd3580f75d6e0ab68097f Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Fri, 15 May 2026 12:24:34 -0600 Subject: [PATCH 3/3] Changes --- lucene/CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 80ba771596a9..8a66aff382be 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -457,6 +457,8 @@ Other * GITHUB#15960: Move parent field from DWPT to IndexingChain. (Tim Brooks) +* GITHUB#16073: Simplify SortedDocIDMerger.next(). (Tim Brooks) + ======================= Lucene 10.4.0 ======================= API Changes