From 7c6237a9127cc3171098a00351a3dda15a7a3219 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 8 Oct 2024 12:50:16 +0200 Subject: [PATCH] Make MaxScoreAccumulator use primitive long instead Object return (#13866) An object return inside hot code like this is needlessly wasteful. Escape analysis doesn't catch this one and we end up allocating many GB of throwaway objects during benchmark runs. We might as well use two utility methods and accumulate the raw value. --- .../lucene/search/MaxScoreAccumulator.java | 29 ++++------- .../lucene/search/TopFieldCollector.java | 11 +++-- .../lucene/search/TopScoreDocCollector.java | 9 ++-- .../search/TestMaxScoreAccumulator.java | 48 +++++++------------ .../lucene/search/TestTopDocsCollector.java | 22 ++++----- .../lucene/search/TestTopFieldCollector.java | 22 ++++----- 6 files changed, 57 insertions(+), 84 deletions(-) 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 5940a80a961..eac33dbf039 100644 --- a/lucene/core/src/java/org/apache/lucene/search/MaxScoreAccumulator.java +++ b/lucene/core/src/java/org/apache/lucene/search/MaxScoreAccumulator.java @@ -35,8 +35,8 @@ final class MaxScoreAccumulator { } /** - * Return the max encoded DocAndScore in a way that is consistent with {@link - * DocAndScore#compareTo}. + * Return the max encoded docId and score found in the two longs, following the encoding in {@link + * #accumulate}. */ private static long maxEncode(long v1, long v2) { float score1 = Float.intBitsToFloat((int) (v1 >> 32)); @@ -57,26 +57,15 @@ final class MaxScoreAccumulator { acc.accumulate(encode); } - 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 DocAndScore(docId, score); + public static float toScore(long value) { + return Float.intBitsToFloat((int) (value >> 32)); } - record DocAndScore(int docId, float score) implements Comparable { + public static int docId(long value) { + return (int) value; + } - @Override - public int compareTo(DocAndScore o) { - int cmp = Float.compare(score, o.score); - if (cmp == 0) { - // tie-break on doc id, lower id has the priority - return Integer.compare(o.docId, docId); - } - return cmp; - } + long getRaw() { + return acc.get(); } } 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 114797f44cb..eac31bf89d0 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopFieldCollector.java @@ -24,7 +24,6 @@ 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; /** @@ -366,10 +365,12 @@ public abstract class TopFieldCollector extends TopDocsCollector { // 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(); + long maxMinScore = minScoreAcc.getRaw(); + float score; + if (maxMinScore != Long.MIN_VALUE + && (score = MaxScoreAccumulator.toScore(maxMinScore)) > minCompetitiveScore) { + scorer.setMinCompetitiveScore(score); + minCompetitiveScore = score; totalHitsRelation = TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO; } } 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 b951aaa7f89..3469276982b 100644 --- a/lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java +++ b/lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java @@ -18,7 +18,6 @@ package org.apache.lucene.search; import java.io.IOException; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.MaxScoreAccumulator.DocAndScore; /** * A {@link Collector} implementation that collects the top-scoring hits, returning them as a {@link @@ -226,13 +225,13 @@ public abstract class TopScoreDocCollector extends TopDocsCollector { protected void updateGlobalMinCompetitiveScore(Scorable scorer) throws IOException { assert minScoreAcc != null; - DocAndScore maxMinScore = minScoreAcc.get(); - if (maxMinScore != null) { + long maxMinScore = minScoreAcc.getRaw(); + if (maxMinScore != Long.MIN_VALUE) { // 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 id that is // smaller than the ids in the current leaf - float score = - docBase >= maxMinScore.docId() ? Math.nextUp(maxMinScore.score()) : maxMinScore.score(); + float score = MaxScoreAccumulator.toScore(maxMinScore); + score = docBase >= MaxScoreAccumulator.docId(maxMinScore) ? Math.nextUp(score) : score; if (score > minCompetitiveScore) { assert hitsThresholdChecker.isThresholdReached(); scorer.setMinCompetitiveScore(score); 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 d816d419c4c..56160971931 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestMaxScoreAccumulator.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestMaxScoreAccumulator.java @@ -23,44 +23,28 @@ public class TestMaxScoreAccumulator extends LuceneTestCase { public void testSimple() { MaxScoreAccumulator acc = new MaxScoreAccumulator(); acc.accumulate(0, 0f); - assertEquals(0f, acc.get().score(), 0); - assertEquals(0, acc.get().docId(), 0); + assertEquals(0f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(0, MaxScoreAccumulator.docId(acc.getRaw()), 0); acc.accumulate(10, 0f); - assertEquals(0f, acc.get().score(), 0); - assertEquals(0, acc.get().docId(), 0); + assertEquals(0f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(0, MaxScoreAccumulator.docId(acc.getRaw()), 0); acc.accumulate(100, 1000f); - assertEquals(1000f, acc.get().score(), 0); - assertEquals(100, acc.get().docId(), 0); + assertEquals(1000f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(100, MaxScoreAccumulator.docId(acc.getRaw()), 0); acc.accumulate(1000, 5f); - assertEquals(1000f, acc.get().score(), 0); - assertEquals(100, acc.get().docId(), 0); + assertEquals(1000f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(100, MaxScoreAccumulator.docId(acc.getRaw()), 0); acc.accumulate(99, 1000f); - assertEquals(1000f, acc.get().score(), 0); - assertEquals(99, acc.get().docId(), 0); + assertEquals(1000f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(99, MaxScoreAccumulator.docId(acc.getRaw()), 0); acc.accumulate(1000, 1001f); - assertEquals(1001f, acc.get().score(), 0); - assertEquals(1000, acc.get().docId(), 0); + assertEquals(1001f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(1000, MaxScoreAccumulator.docId(acc.getRaw()), 0); acc.accumulate(10, 1001f); - assertEquals(1001f, acc.get().score(), 0); - assertEquals(10, acc.get().docId(), 0); + assertEquals(1001f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(10, MaxScoreAccumulator.docId(acc.getRaw()), 0); acc.accumulate(100, 1001f); - assertEquals(1001f, acc.get().score(), 0); - assertEquals(10, acc.get().docId(), 0); - } - - public void testRandom() { - MaxScoreAccumulator acc = new MaxScoreAccumulator(); - int numDocs = atLeast(100); - int maxDocs = atLeast(10000); - MaxScoreAccumulator.DocAndScore max = new MaxScoreAccumulator.DocAndScore(-1, -1); - for (int i = 0; i < numDocs; i++) { - MaxScoreAccumulator.DocAndScore res = - new MaxScoreAccumulator.DocAndScore(random().nextInt(maxDocs), random().nextFloat()); - acc.accumulate(res.docId(), res.score()); - if (res.compareTo(max) > 0) { - max = res; - } - } - assertEquals(max, acc.get()); + assertEquals(1001f, MaxScoreAccumulator.toScore(acc.getRaw()), 0); + assertEquals(10, MaxScoreAccumulator.docId(acc.getRaw()), 0); } } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestTopDocsCollector.java b/lucene/core/src/test/org/apache/lucene/search/TestTopDocsCollector.java index d4df59f2f72..14b51ca214e 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestTopDocsCollector.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestTopDocsCollector.java @@ -519,47 +519,47 @@ public class TestTopDocsCollector extends LuceneTestCase { scorer.score = 3; leafCollector.collect(0); - assertNull(minValueChecker.get()); + assertEquals(Long.MIN_VALUE, minValueChecker.getRaw()); assertNull(scorer.minCompetitiveScore); scorer2.score = 6; leafCollector2.collect(0); - assertNull(minValueChecker.get()); + assertEquals(Long.MIN_VALUE, minValueChecker.getRaw()); assertNull(scorer2.minCompetitiveScore); scorer.score = 2; leafCollector.collect(1); - assertEquals(2f, minValueChecker.get().score(), 0f); + assertEquals(2f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(Math.nextUp(2f), scorer.minCompetitiveScore, 0f); assertNull(scorer2.minCompetitiveScore); scorer2.score = 9; leafCollector2.collect(1); - assertEquals(6f, minValueChecker.get().score(), 0f); + assertEquals(6f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(Math.nextUp(2f), scorer.minCompetitiveScore, 0f); assertEquals(Math.nextUp(6f), scorer2.minCompetitiveScore, 0f); scorer2.score = 7; leafCollector2.collect(2); - assertEquals(minValueChecker.get().score(), 7f, 0f); + assertEquals(MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 7f, 0f); assertEquals(Math.nextUp(2f), scorer.minCompetitiveScore, 0f); assertEquals(Math.nextUp(7f), scorer2.minCompetitiveScore, 0f); scorer2.score = 1; leafCollector2.collect(3); - assertEquals(minValueChecker.get().score(), 7f, 0f); + assertEquals(MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 7f, 0f); assertEquals(Math.nextUp(2f), scorer.minCompetitiveScore, 0f); assertEquals(Math.nextUp(7f), scorer2.minCompetitiveScore, 0f); scorer.score = 10; leafCollector.collect(2); - assertEquals(minValueChecker.get().score(), 7f, 0f); + assertEquals(MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 7f, 0f); assertEquals(7f, scorer.minCompetitiveScore, 0f); assertEquals(Math.nextUp(7f), scorer2.minCompetitiveScore, 0f); scorer.score = 11; leafCollector.collect(3); - assertEquals(minValueChecker.get().score(), 10, 0f); + assertEquals(MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 10, 0f); assertEquals(Math.nextUp(10f), scorer.minCompetitiveScore, 0f); assertEquals(Math.nextUp(7f), scorer2.minCompetitiveScore, 0f); @@ -571,19 +571,19 @@ public class TestTopDocsCollector extends LuceneTestCase { scorer3.score = 1f; leafCollector3.collect(0); - assertEquals(10f, minValueChecker.get().score(), 0f); + assertEquals(10f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(Math.nextUp(10f), scorer3.minCompetitiveScore, 0f); scorer.score = 11; leafCollector.collect(4); - assertEquals(11f, minValueChecker.get().score(), 0f); + assertEquals(11f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(Math.nextUp(11f), scorer.minCompetitiveScore, 0f); assertEquals(Math.nextUp(7f), scorer2.minCompetitiveScore, 0f); assertEquals(Math.nextUp(10f), scorer3.minCompetitiveScore, 0f); scorer3.score = 2f; leafCollector3.collect(1); - assertEquals(minValueChecker.get().score(), 11f, 0f); + assertEquals(MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 11f, 0f); assertEquals(Math.nextUp(11f), scorer.minCompetitiveScore, 0f); assertEquals(Math.nextUp(7f), scorer2.minCompetitiveScore, 0f); assertEquals(Math.nextUp(11f), scorer3.minCompetitiveScore, 0f); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollector.java b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollector.java index cd6f0ac079d..c507eb0f647 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollector.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollector.java @@ -577,47 +577,47 @@ public class TestTopFieldCollector extends LuceneTestCase { scorer.score = 3; leafCollector.collect(0); - assertNull(minValueChecker.get()); + assertEquals(Long.MIN_VALUE, minValueChecker.getRaw()); assertNull(scorer.minCompetitiveScore); scorer2.score = 6; leafCollector2.collect(0); - assertNull(minValueChecker.get()); + assertEquals(Long.MIN_VALUE, minValueChecker.getRaw()); assertNull(scorer2.minCompetitiveScore); scorer.score = 2; leafCollector.collect(1); - assertEquals(2f, minValueChecker.get().score(), 0f); + assertEquals(2f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(2f, scorer.minCompetitiveScore, 0f); assertNull(scorer2.minCompetitiveScore); scorer2.score = 9; leafCollector2.collect(1); - assertEquals(6f, minValueChecker.get().score(), 0f); + assertEquals(6f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(2f, scorer.minCompetitiveScore, 0f); assertEquals(6f, scorer2.minCompetitiveScore, 0f); scorer2.score = 7; leafCollector2.collect(2); - assertEquals(7f, minValueChecker.get().score(), 0f); + assertEquals(7f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(2f, scorer.minCompetitiveScore, 0f); assertEquals(7f, scorer2.minCompetitiveScore, 0f); scorer2.score = 1; leafCollector2.collect(3); - assertEquals(7f, minValueChecker.get().score(), 0f); + assertEquals(7f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(2f, scorer.minCompetitiveScore, 0f); assertEquals(7f, scorer2.minCompetitiveScore, 0f); scorer.score = 10; leafCollector.collect(2); - assertEquals(7f, minValueChecker.get().score(), 0f); + assertEquals(7f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(7f, scorer.minCompetitiveScore, 0f); assertEquals(7f, scorer2.minCompetitiveScore, 0f); scorer.score = 11; leafCollector.collect(3); - assertEquals(10f, minValueChecker.get().score(), 0f); + assertEquals(10f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(10f, scorer.minCompetitiveScore, 0f); assertEquals(7f, scorer2.minCompetitiveScore, 0f); @@ -629,19 +629,19 @@ public class TestTopFieldCollector extends LuceneTestCase { scorer3.score = 1f; leafCollector3.collect(0); - assertEquals(10f, minValueChecker.get().score(), 0f); + assertEquals(10f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(10f, scorer3.minCompetitiveScore, 0f); scorer.score = 11; leafCollector.collect(4); - assertEquals(11f, minValueChecker.get().score(), 0f); + assertEquals(11f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(11f, scorer.minCompetitiveScore, 0f); assertEquals(7f, scorer2.minCompetitiveScore, 0f); assertEquals(10f, scorer3.minCompetitiveScore, 0f); scorer3.score = 2f; leafCollector3.collect(1); - assertEquals(11f, minValueChecker.get().score(), 0f); + assertEquals(11f, MaxScoreAccumulator.toScore(minValueChecker.getRaw()), 0f); assertEquals(11f, scorer.minCompetitiveScore, 0f); assertEquals(7f, scorer2.minCompetitiveScore, 0f); assertEquals(11f, scorer3.minCompetitiveScore, 0f);