You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ch...@apache.org on 2025/04/25 15:27:31 UTC
(lucene) branch branch_10x updated: Fix leadCost calculation in BooleanScorerSupplier.requiredBulkScorer (#14543)
This is an automated email from the ASF dual-hosted git repository.
chegar pushed a commit to branch branch_10x
in repository https://gitbox.apache.org/repos/asf/lucene.git
The following commit(s) were added to refs/heads/branch_10x by this push:
new 6ad8a9673da Fix leadCost calculation in BooleanScorerSupplier.requiredBulkScorer (#14543)
6ad8a9673da is described below
commit 6ad8a9673da8c97d45f19bcea0163ffc4877c57c
Author: Peter Alfonsi <pe...@gmail.com>
AuthorDate: Fri Apr 25 08:23:42 2025 -0700
Fix leadCost calculation in BooleanScorerSupplier.requiredBulkScorer (#14543)
Fixes #14542 by setting leadCost in BooleanScorerSupplier.requiredBulkScorer to the minimum of both MUST and FILTER clauses' costs. This bug caused performance regressions in some queries. More details are in the original issue.
---
lucene/CHANGES.txt | 5 +++
.../lucene/search/BooleanScorerSupplier.java | 28 +++++++++-----
.../lucene/search/TestBoolean2ScorerSupplier.java | 44 ++++++++++++++++++++--
3 files changed, 64 insertions(+), 13 deletions(-)
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 4c6e0f136f7..abdda667cb3 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -61,6 +61,11 @@ Bug Fixes
* GITHUB#14523, GITHUB#14530: Correct TermOrdValComparator competitive iterator so that it forces sparse
field iteration to be at least scoring window baseline when doing intoBitSet. (Ben Trent, Adrien Grand)
+* GITHUB#14543: Fixed lead cost computations for bulk scorers of conjunctive
+ queries that mix MUST and FILTER clauses, and disjunctive queries that
+ configure a minimum number of matching SHOULD clauses.
+ (Peter Alfonsi, Adrien Grand)
+
======================= Lucene 10.2.0 =======================
API Changes
diff --git a/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java b/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java
index a59815fd08e..e584b1f869f 100644
--- a/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java
+++ b/lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java
@@ -69,6 +69,14 @@ final class BooleanScorerSupplier extends ScorerSupplier {
this.maxDoc = maxDoc;
}
+ private long computeShouldCost() {
+ final Collection<ScorerSupplier> optionalScorers = subs.get(Occur.SHOULD);
+ return ScorerUtil.costWithMinShouldMatch(
+ optionalScorers.stream().mapToLong(ScorerSupplier::cost),
+ optionalScorers.size(),
+ minShouldMatch);
+ }
+
private long computeCost() {
OptionalLong minRequiredCost =
Stream.concat(subs.get(Occur.MUST).stream(), subs.get(Occur.FILTER).stream())
@@ -77,12 +85,7 @@ final class BooleanScorerSupplier extends ScorerSupplier {
if (minRequiredCost.isPresent() && minShouldMatch == 0) {
return minRequiredCost.getAsLong();
} else {
- final Collection<ScorerSupplier> optionalScorers = subs.get(Occur.SHOULD);
- final long shouldCost =
- ScorerUtil.costWithMinShouldMatch(
- optionalScorers.stream().mapToLong(ScorerSupplier::cost),
- optionalScorers.size(),
- minShouldMatch);
+ final long shouldCost = computeShouldCost();
return Math.min(minRequiredCost.orElse(Long.MAX_VALUE), shouldCost);
}
}
@@ -293,9 +296,10 @@ final class BooleanScorerSupplier extends ScorerSupplier {
return new MaxScoreBulkScorer(maxDoc, optionalScorers, null);
}
+ long shouldCost = computeShouldCost();
List<Scorer> optional = new ArrayList<Scorer>();
for (ScorerSupplier ss : subs.get(Occur.SHOULD)) {
- optional.add(ss.get(Long.MAX_VALUE));
+ optional.add(ss.get(shouldCost));
}
return new BooleanScorer(optional, Math.max(1, minShouldMatch), scoreMode.needsScores());
@@ -359,10 +363,14 @@ final class BooleanScorerSupplier extends ScorerSupplier {
return scorer;
}
- long leadCost =
+ long mustLeadCost =
subs.get(Occur.MUST).stream().mapToLong(ScorerSupplier::cost).min().orElse(Long.MAX_VALUE);
- leadCost =
- subs.get(Occur.FILTER).stream().mapToLong(ScorerSupplier::cost).min().orElse(leadCost);
+ long filterLeadCost =
+ subs.get(Occur.FILTER).stream()
+ .mapToLong(ScorerSupplier::cost)
+ .min()
+ .orElse(Long.MAX_VALUE);
+ long leadCost = Math.min(mustLeadCost, filterLeadCost);
List<Scorer> requiredNoScoring = new ArrayList<>();
for (ScorerSupplier ss : subs.get(Occur.FILTER)) {
diff --git a/lucene/core/src/test/org/apache/lucene/search/TestBoolean2ScorerSupplier.java b/lucene/core/src/test/org/apache/lucene/search/TestBoolean2ScorerSupplier.java
index 780b6ea5681..906a4bbe025 100644
--- a/lucene/core/src/test/org/apache/lucene/search/TestBoolean2ScorerSupplier.java
+++ b/lucene/core/src/test/org/apache/lucene/search/TestBoolean2ScorerSupplier.java
@@ -105,7 +105,19 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
@Override
public Scorer get(long leadCost) throws IOException {
if (this.leadCost != null) {
- assertEquals(this.toString(), this.leadCost.longValue(), leadCost);
+ if (this.leadCost < this.cost) {
+ // If the expected lead cost is less than the cost, ie. another clause is leading
+ // iteration, then the exact lead cost must be provided.
+ assertEquals(
+ this.toString() + " actual leadCost=" + leadCost,
+ this.leadCost.longValue(),
+ leadCost);
+ } else {
+ // Otherwise the lead cost may be provided as the cost of this very clause or as
+ // Long.MAX_VALUE (typically for bulk scorers), both signaling that this clause is leading
+ // iteration.
+ assertTrue(this.toString() + " actual leadCost=" + leadCost, leadCost >= this.leadCost);
+ }
}
return new FakeScorer(cost);
}
@@ -269,9 +281,10 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
// test the tester...
public void testFakeScorerSupplier() {
- FakeScorerSupplier randomAccessSupplier = new FakeScorerSupplier(random().nextInt(100), 30);
+ FakeScorerSupplier randomAccessSupplier =
+ new FakeScorerSupplier(TestUtil.nextInt(random(), 31, 100), 30);
expectThrows(AssertionError.class, () -> randomAccessSupplier.get(70));
- FakeScorerSupplier sequentialSupplier = new FakeScorerSupplier(random().nextInt(100), 70);
+ FakeScorerSupplier sequentialSupplier = new FakeScorerSupplier(random().nextInt(70), 70);
expectThrows(AssertionError.class, () -> sequentialSupplier.get(30));
}
@@ -289,6 +302,9 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
new BooleanScorerSupplier(
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
.get(Long.MAX_VALUE); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(
+ new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs = new EnumMap<>(Occur.class);
for (Occur occur : Occur.values()) {
@@ -315,6 +331,9 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
new BooleanScorerSupplier(
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(
+ new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs.get(Occur.SHOULD).clear();
subs.get(Occur.SHOULD).add(new FakeScorerSupplier(42, 20));
@@ -338,6 +357,9 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
new BooleanScorerSupplier(
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(
+ new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs = new EnumMap<>(Occur.class);
for (Occur occur : Occur.values()) {
@@ -364,6 +386,9 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
new BooleanScorerSupplier(
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(
+ new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs = new EnumMap<>(Occur.class);
for (Occur occur : Occur.values()) {
@@ -377,6 +402,9 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
new BooleanScorerSupplier(
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 3, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(
+ new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 3, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
}
public void testProhibitedLeadCost() throws IOException {
@@ -391,6 +419,9 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
new BooleanScorerSupplier(
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(
+ new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs.get(Occur.MUST).clear();
subs.get(Occur.MUST_NOT).clear();
@@ -399,6 +430,9 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
new BooleanScorerSupplier(
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(
+ new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs.get(Occur.MUST).clear();
subs.get(Occur.MUST_NOT).clear();
@@ -420,6 +454,8 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
subs.get(Occur.SHOULD).add(new FakeScorerSupplier(30, 42));
new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs.get(Occur.MUST).clear();
subs.get(Occur.SHOULD).clear();
@@ -427,6 +463,8 @@ public class TestBoolean2ScorerSupplier extends LuceneTestCase {
subs.get(Occur.SHOULD).add(new FakeScorerSupplier(80, 42));
new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
.get(100); // triggers assertions as a side-effect
+ new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
+ .bulkScorer(); // triggers assertions as a side-effect
subs.get(Occur.MUST).clear();
subs.get(Occur.SHOULD).clear();