From 914e2641658b71638f9f4c940fe9f63b1a67ce7d Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 27 Jul 2018 11:06:13 +0200 Subject: [PATCH] LUCENE-8428: PriorityQueue takes sentinels via a Supplier as a constructor argument. --- lucene/CHANGES.txt | 4 + .../org/apache/lucene/search/HitQueue.java | 20 ++-- .../search/spans/SpanPositionQueue.java | 2 +- .../org/apache/lucene/util/PriorityQueue.java | 108 ++++++++---------- .../lucene/facet/TopOrdAndFloatQueue.java | 2 +- .../lucene/facet/TopOrdAndIntQueue.java | 2 +- .../lucene/search/TermAutomatonScorer.java | 4 +- 7 files changed, 68 insertions(+), 74 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index e7f15aa37b6..2876fa72ad2 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -163,6 +163,10 @@ API Changes: * LUCENE-7314: Graduate LatLonPoint and query classes to core (Nick Knize) +* LUCENE-8428: The way that oal.util.PriorityQueue creates sentinel objects has + been changed from a protected method to a java.util.function.Supplier as a + constructor argument. (Adrien Grand) + Bug Fixes: * LUCENE-8380: UTF8TaxonomyWriterCache inconsistency. (Ruslan Torobaev, Dawid Weiss) diff --git a/lucene/core/src/java/org/apache/lucene/search/HitQueue.java b/lucene/core/src/java/org/apache/lucene/search/HitQueue.java index 8868c2b0829..071f4bf85a9 100644 --- a/lucene/core/src/java/org/apache/lucene/search/HitQueue.java +++ b/lucene/core/src/java/org/apache/lucene/search/HitQueue.java @@ -58,20 +58,20 @@ final class HitQueue extends PriorityQueue { * the requested size of this queue. * @param prePopulate * specifies whether to pre-populate the queue with sentinel values. - * @see #getSentinelObject() */ HitQueue(int size, boolean prePopulate) { - super(size, prePopulate); + super(size, () -> { + if (prePopulate) { + // Always set the doc Id to MAX_VALUE so that it won't be favored by + // lessThan. This generally should not happen since if score is not NEG_INF, + // TopScoreDocCollector will always add the object to the queue. + return new ScoreDoc(Integer.MAX_VALUE, Float.NEGATIVE_INFINITY); + } else { + return null; + } + }); } - @Override - protected ScoreDoc getSentinelObject() { - // Always set the doc Id to MAX_VALUE so that it won't be favored by - // lessThan. This generally should not happen since if score is not NEG_INF, - // TopScoreDocCollector will always add the object to the queue. - return new ScoreDoc(Integer.MAX_VALUE, Float.NEGATIVE_INFINITY); - } - @Override protected final boolean lessThan(ScoreDoc hitA, ScoreDoc hitB) { if (hitA.score == hitB.score) diff --git a/lucene/core/src/java/org/apache/lucene/search/spans/SpanPositionQueue.java b/lucene/core/src/java/org/apache/lucene/search/spans/SpanPositionQueue.java index 2d2bd16c555..a82261cf828 100644 --- a/lucene/core/src/java/org/apache/lucene/search/spans/SpanPositionQueue.java +++ b/lucene/core/src/java/org/apache/lucene/search/spans/SpanPositionQueue.java @@ -21,7 +21,7 @@ import org.apache.lucene.util.PriorityQueue; class SpanPositionQueue extends PriorityQueue { SpanPositionQueue(int maxSize) { - super(maxSize, false); // do not prepopulate + super(maxSize); // do not prepopulate } protected boolean lessThan(Spans s1, Spans s2) { diff --git a/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java b/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java index ef08c4abb1c..3c96dc56315 100644 --- a/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java +++ b/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java @@ -18,6 +18,7 @@ package org.apache.lucene.util; import java.util.Iterator; import java.util.NoSuchElementException; +import java.util.function.Supplier; /** @@ -26,10 +27,9 @@ import java.util.NoSuchElementException; * require log(size) time but the remove() cost implemented here is linear. * *

- * NOTE: This class will pre-allocate a full array of length - * maxSize+1 if instantiated via the - * {@link #PriorityQueue(int,boolean)} constructor with prepopulate - * set to true. + * NOTE: This class pre-allocates an array of length {@code maxSize+1} + * and pre-fills it with elements if instantiated via the + * {@link #PriorityQueue(int,Supplier)} constructor. * * NOTE: Iteration order is not specified. * @@ -40,11 +40,47 @@ public abstract class PriorityQueue implements Iterable { private final int maxSize; private final T[] heap; + /** + * Create an empty priority queue of the configured size. + */ public PriorityQueue(int maxSize) { - this(maxSize, true); + this(maxSize, () -> null); } - public PriorityQueue(int maxSize, boolean prepopulate) { + /** + * Create a priority queue that is pre-filled with sentinel objects, so that + * the code which uses that queue can always assume it's full and only change + * the top without attempting to insert any new object.
+ * + * Those sentinel values should always compare worse than any non-sentinel + * value (i.e., {@link #lessThan} should always favor the + * non-sentinel values).
+ * + * By default, the supplier returns null, which means the queue will not be + * filled with sentinel values. Otherwise, the value returned will be used to + * pre-populate the queue.
+ * + * If this method is extended to return a non-null value, then the following + * usage pattern is recommended: + * + * 

+   * PriorityQueue<MyObject> pq = new MyQueue<MyObject>(numHits);
+   * // save the 'top' element, which is guaranteed to not be null.
+   * MyObject pqTop = pq.top();
+   * <...>
+   * // now in order to add a new element, which is 'better' than top (after
+   * // you've verified it is better), it is as simple as:
+   * pqTop.change().
+   * pqTop = pq.updateTop();
+   *
+ * + * NOTE: the given supplier will be called {@code maxSize} times, + * relying on a new object to be returned and will not check if it's null again. + * Therefore you should ensure any call to this method creates a new instance and + * behaves consistently, e.g., it cannot return null if it previously returned + * non-null and all returned instances must {@link #lessThan compare equal}. + */ + public PriorityQueue(int maxSize, Supplier sentinelObjectSupplier) { final int heapSize; if (0 == maxSize) { // We allocate 1 extra to avoid if statement in top() @@ -65,16 +101,14 @@ public abstract class PriorityQueue implements Iterable { this.heap = h; this.maxSize = maxSize; - if (prepopulate) { - // If sentinel objects are supported, populate the queue with them - T sentinel = getSentinelObject(); - if (sentinel != null) { - heap[1] = sentinel; - for (int i = 2; i < heap.length; i++) { - heap[i] = getSentinelObject(); - } - size = maxSize; + // If sentinel objects are supported, populate the queue with them + T sentinel = sentinelObjectSupplier.get(); + if (sentinel != null) { + heap[1] = sentinel; + for (int i = 2; i < heap.length; i++) { + heap[i] = sentinelObjectSupplier.get(); } + size = maxSize; } } @@ -84,50 +118,6 @@ public abstract class PriorityQueue implements Iterable { */ protected abstract boolean lessThan(T a, T b); - /** - * This method can be overridden by extending classes to return a sentinel - * object which will be used by the {@link PriorityQueue#PriorityQueue(int,boolean)} - * constructor to fill the queue, so that the code which uses that queue can always - * assume it's full and only change the top without attempting to insert any new - * object.
- * - * Those sentinel values should always compare worse than any non-sentinel - * value (i.e., {@link #lessThan} should always favor the - * non-sentinel values).
- * - * By default, this method returns null, which means the queue will not be - * filled with sentinel values. Otherwise, the value returned will be used to - * pre-populate the queue. Adds sentinel values to the queue.
- * - * If this method is extended to return a non-null value, then the following - * usage pattern is recommended: - * - * 
-   * // extends getSentinelObject() to return a non-null value.
-   * PriorityQueue<MyObject> pq = new MyQueue<MyObject>(numHits);
-   * // save the 'top' element, which is guaranteed to not be null.
-   * MyObject pqTop = pq.top();
-   * <...>
-   * // now in order to add a new element, which is 'better' than top (after
-   * // you've verified it is better), it is as simple as:
-   * pqTop.change().
-   * pqTop = pq.updateTop();
-   *
- * - * NOTE: if this method returns a non-null value, it will be called by - * the {@link PriorityQueue#PriorityQueue(int,boolean)} constructor - * {@link #size()} times, relying on a new object to be returned and will not - * check if it's null again. Therefore you should ensure any call to this - * method creates a new instance and behaves consistently, e.g., it cannot - * return null if it previously returned non-null. - * - * @return the sentinel object to use to pre-populate the queue, or null if - * sentinel objects are not supported. - */ - protected T getSentinelObject() { - return null; - } - /** * Adds an Object to a PriorityQueue in log(size) time. If one tries to add * more objects than maxSize from initialize an diff --git a/lucene/facet/src/java/org/apache/lucene/facet/TopOrdAndFloatQueue.java b/lucene/facet/src/java/org/apache/lucene/facet/TopOrdAndFloatQueue.java index d0fe8a9cd87..d4072cd8197 100644 --- a/lucene/facet/src/java/org/apache/lucene/facet/TopOrdAndFloatQueue.java +++ b/lucene/facet/src/java/org/apache/lucene/facet/TopOrdAndFloatQueue.java @@ -38,7 +38,7 @@ public class TopOrdAndFloatQueue extends PriorityQueue { public DocIDQueue(int maxSize) { - super(maxSize, false); + super(maxSize); } @Override @@ -99,7 +99,7 @@ class TermAutomatonScorer extends Scorer { * position. */ private static class PositionQueue extends PriorityQueue { public PositionQueue(int maxSize) { - super(maxSize, false); + super(maxSize); } @Override