From b623f6363d818982aa280989d2ae756944375a17 Mon Sep 17 00:00:00 2001 From: jimczi Date: Thu, 10 Oct 2019 17:56:51 +0200 Subject: [PATCH] address review and add an entry to the changes --- lucene/CHANGES.txt | 3 +++ .../lucene/search/MaxScoreAccumulator.java | 16 +++++++------- .../lucene/search/TopFieldCollector.java | 22 +++++++++++-------- .../lucene/search/TopScoreDocCollector.java | 3 ++- .../search/TestMaxScoreAccumulator.java | 4 ++-- 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index bb11aeae804..1ca3ce6c8c1 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -58,6 +58,9 @@ Improvements * LUCENE-8213: LRUQueryCache#doCache now uses IndexSearcher's Executor (if present) to asynchronously cache heavy queries (Atri Sharma) +* LUCENE-8992: TopFieldCollector and TopScoreDocCollector can now share minimum scores across leaves + concurrently. (Jim Ferenczi) + Bug fixes * LUCENE-8663: NRTCachingDirectory.slowFileExists may open a file while diff --git a/lucene/core/src/java/org/apache/lucene/search/MaxScoreAccumulator.java b/lucene/core/src/java/org/apache/lucene/search/MaxScoreAccumulator.java index a25406d0cdb..0d172d9ab4e 100644 --- a/lucene/core/src/java/org/apache/lucene/search/MaxScoreAccumulator.java +++ b/lucene/core/src/java/org/apache/lucene/search/MaxScoreAccumulator.java @@ -37,31 +37,31 @@ final class MaxScoreAccumulator { void accumulate(int docID, float score) { assert docID >= 0 && score >= 0; - long encode = (((long) Float.floatToIntBits(score)) << 32) | (docID & 0xffffffffL); + long encode = (((long) Float.floatToIntBits(score)) << 32) | docID; acc.accumulate(encode); } - Result get() { + DocAndScore get() { long value = acc.get(); if (value == Long.MIN_VALUE) { return null; } float score = Float.intBitsToFloat((int) (value >> 32)); int docID = (int) value; - return new Result(docID, score); + return new DocAndScore(docID, score); } - static class Result implements Comparable { + static class DocAndScore implements Comparable { final int docID; final float score; - Result(int docID, float score) { + DocAndScore(int docID, float score) { this.docID = docID; this.score = score; } @Override - public int compareTo(Result o) { + public int compareTo(DocAndScore o) { int cmp = Float.compare(score, o.score); if (cmp == 0) { return Integer.compare(docID, o.docID); @@ -73,14 +73,14 @@ final class MaxScoreAccumulator { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - Result result = (Result) o; + DocAndScore result = (DocAndScore) o; return docID == result.docID && Float.compare(result.score, score) == 0; } @Override public String toString() { - return "Max{" + + return "DocAndScore{" + "docID=" + docID + ", score=" + score + '}'; diff --git a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java index 5fe9fbcf92d..1a0b4563db3 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java @@ -27,6 +27,7 @@ import java.util.Objects; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.ReaderUtil; import org.apache.lucene.search.FieldValueHitQueue.Entry; +import org.apache.lucene.search.MaxScoreAccumulator.DocAndScore; import org.apache.lucene.search.TotalHits.Relation; /** @@ -153,8 +154,8 @@ public abstract class TopFieldCollector extends TopDocsCollector { collectedAllCompetitiveHits = true; } } else if (totalHitsRelation == Relation.EQUAL_TO) { - // we just reached totalHitsThreshold, we can start setting the min - // competitive score now + // we can start setting the min competitive score if the + // threshold is reached for the first time here. updateMinCompetitiveScore(scorer); } return; @@ -255,10 +256,10 @@ public abstract class TopFieldCollector extends TopDocsCollector { } else { collectedAllCompetitiveHits = true; } - } else if (totalHitsRelation == Relation.EQUAL_TO) { - // we just reached totalHitsThreshold, we can start setting the min - // competitive score now - updateMinCompetitiveScore(scorer); + } else if (totalHitsRelation == TotalHits.Relation.EQUAL_TO) { + // we can start setting the min competitive score if the + // threshold is reached for the first time here. + updateMinCompetitiveScore(scorer); } return; } @@ -362,7 +363,10 @@ public abstract class TopFieldCollector extends TopDocsCollector { assert minScoreAcc != null; if (canSetMinScore && hitsThresholdChecker.isThresholdReached()) { - MaxScoreAccumulator.Result maxMinScore = minScoreAcc.get(); + // we can start checking the global maximum score even + // if the local queue is not full because the threshold + // is reached. + DocAndScore maxMinScore = minScoreAcc.get(); if (maxMinScore != null && maxMinScore.score > minCompetitiveScore) { scorer.setMinCompetitiveScore(maxMinScore.score); minCompetitiveScore = maxMinScore.score; @@ -373,8 +377,8 @@ public abstract class TopFieldCollector extends TopDocsCollector { protected void updateMinCompetitiveScore(Scorable scorer) throws IOException { if (canSetMinScore - && hitsThresholdChecker.isThresholdReached() - && queueFull) { + && queueFull + && hitsThresholdChecker.isThresholdReached()) { assert bottom != null && firstComparator != null; float minScore = firstComparator.value(bottom.slot); if (minScore > minCompetitiveScore) { diff --git a/lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java b/lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java index 02281a44e6e..e9036e52523 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.Collection; import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.MaxScoreAccumulator.DocAndScore; /** * A {@link Collector} implementation that collects the top-scoring hits, @@ -301,7 +302,7 @@ public abstract class TopScoreDocCollector extends TopDocsCollector { protected void updateGlobalMinCompetitiveScore(Scorable scorer) throws IOException { assert minScoreAcc != null; - MaxScoreAccumulator.Result maxMinScore = minScoreAcc.get(); + DocAndScore maxMinScore = minScoreAcc.get(); if (maxMinScore != null) { // since we tie-break on doc id and collect in doc id order we can require // the next float if the global minimum score is set on a document that is diff --git a/lucene/core/src/test/org/apache/lucene/search/TestMaxScoreAccumulator.java b/lucene/core/src/test/org/apache/lucene/search/TestMaxScoreAccumulator.java index dc0dbc6722f..c656d47748b 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestMaxScoreAccumulator.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestMaxScoreAccumulator.java @@ -44,9 +44,9 @@ public class TestMaxScoreAccumulator extends LuceneTestCase { MaxScoreAccumulator acc = new MaxScoreAccumulator(); int numDocs = atLeast(100); int maxDocs = atLeast(10000); - MaxScoreAccumulator.Result max = new MaxScoreAccumulator.Result(-1, -1); + MaxScoreAccumulator.DocAndScore max = new MaxScoreAccumulator.DocAndScore(-1, -1); for (int i = 0; i < numDocs; i++) { - MaxScoreAccumulator.Result res = new MaxScoreAccumulator.Result(random().nextInt(maxDocs), random().nextFloat()); + MaxScoreAccumulator.DocAndScore res = new MaxScoreAccumulator.DocAndScore(random().nextInt(maxDocs), random().nextFloat()); acc.accumulate(res.docID, res.score); if (res.compareTo(max) > 0) { max = res;