Skip to content

Commit 7610d9c

Browse files
gf2121gf2121
authored
Merge branch 'main' into perf_term_14445
2 parents acfbdb1 + d72021a commit 7610d9c

File tree

3 files changed

+65
-14
lines changed

3 files changed

+65
-14
lines changed

‎lucene/CHANGES.txt

+6-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,12 @@ Bug Fixes
114114
* GITHUB#14523, GITHUB#14530: Correct TermOrdValComparator competitive iterator so that it forces sparse
115115
field iteration to be at least scoring window baseline when doing intoBitSet. (Ben Trent, Adrien Grand)
116116

117-
* GITHUB#14445 : Provide better impacts for fields indexed with IndexOptions.DOCS GITHUB#14511 ( Aniketh Jain )
117+
* GITHUB#14445: Provide better impacts for fields indexed with IndexOptions.DOCS GITHUB#14511 (Aniketh Jain)
118+
119+
* GITHUB#14543: Fixed lead cost computations for bulk scorers of conjunctive
120+
queries that mix MUST and FILTER clauses, and disjunctive queries that
121+
configure a minimum number of matching SHOULD clauses.
122+
(Peter Alfonsi, Adrien Grand)
118123

119124
======================= Lucene 10.2.0 =======================
120125

‎lucene/core/src/java/org/apache/lucene/search/BooleanScorerSupplier.java

+18-10
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ final class BooleanScorerSupplier extends ScorerSupplier {
6969
this.maxDoc = maxDoc;
7070
}
7171

72+
private long computeShouldCost() {
73+
final Collection<ScorerSupplier> optionalScorers = subs.get(Occur.SHOULD);
74+
return ScorerUtil.costWithMinShouldMatch(
75+
optionalScorers.stream().mapToLong(ScorerSupplier::cost),
76+
optionalScorers.size(),
77+
minShouldMatch);
78+
}
79+
7280
private long computeCost() {
7381
OptionalLong minRequiredCost =
7482
Stream.concat(subs.get(Occur.MUST).stream(), subs.get(Occur.FILTER).stream())
@@ -77,12 +85,7 @@ private long computeCost() {
7785
if (minRequiredCost.isPresent() && minShouldMatch == 0) {
7886
return minRequiredCost.getAsLong();
7987
} else {
80-
final Collection<ScorerSupplier> optionalScorers = subs.get(Occur.SHOULD);
81-
final long shouldCost =
82-
ScorerUtil.costWithMinShouldMatch(
83-
optionalScorers.stream().mapToLong(ScorerSupplier::cost),
84-
optionalScorers.size(),
85-
minShouldMatch);
88+
final long shouldCost = computeShouldCost();
8689
return Math.min(minRequiredCost.orElse(Long.MAX_VALUE), shouldCost);
8790
}
8891
}
@@ -293,9 +296,10 @@ BulkScorer optionalBulkScorer() throws IOException {
293296
return new MaxScoreBulkScorer(maxDoc, optionalScorers, null);
294297
}
295298

299+
long shouldCost = computeShouldCost();
296300
List<Scorer> optional = new ArrayList<Scorer>();
297301
for (ScorerSupplier ss : subs.get(Occur.SHOULD)) {
298-
optional.add(ss.get(Long.MAX_VALUE));
302+
optional.add(ss.get(shouldCost));
299303
}
300304

301305
return new BooleanScorer(optional, Math.max(1, minShouldMatch), scoreMode.needsScores());
@@ -359,10 +363,14 @@ private BulkScorer requiredBulkScorer() throws IOException {
359363
return scorer;
360364
}
361365

362-
long leadCost =
366+
long mustLeadCost =
363367
subs.get(Occur.MUST).stream().mapToLong(ScorerSupplier::cost).min().orElse(Long.MAX_VALUE);
364-
leadCost =
365-
subs.get(Occur.FILTER).stream().mapToLong(ScorerSupplier::cost).min().orElse(leadCost);
368+
long filterLeadCost =
369+
subs.get(Occur.FILTER).stream()
370+
.mapToLong(ScorerSupplier::cost)
371+
.min()
372+
.orElse(Long.MAX_VALUE);
373+
long leadCost = Math.min(mustLeadCost, filterLeadCost);
366374

367375
List<Scorer> requiredNoScoring = new ArrayList<>();
368376
for (ScorerSupplier ss : subs.get(Occur.FILTER)) {

‎lucene/core/src/test/org/apache/lucene/search/TestBoolean2ScorerSupplier.java

+41-3
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,19 @@ private static class FakeScorerSupplier extends ScorerSupplier {
105105
@Override
106106
public Scorer get(long leadCost) throws IOException {
107107
if (this.leadCost != null) {
108-
assertEquals(this.toString(), this.leadCost.longValue(), leadCost);
108+
if (this.leadCost < this.cost) {
109+
// If the expected lead cost is less than the cost, ie. another clause is leading
110+
// iteration, then the exact lead cost must be provided.
111+
assertEquals(
112+
this.toString() + " actual leadCost=" + leadCost,
113+
this.leadCost.longValue(),
114+
leadCost);
115+
} else {
116+
// Otherwise the lead cost may be provided as the cost of this very clause or as
117+
// Long.MAX_VALUE (typically for bulk scorers), both signaling that this clause is leading
118+
// iteration.
119+
assertTrue(this.toString() + " actual leadCost=" + leadCost, leadCost >= this.leadCost);
120+
}
109121
}
110122
return new FakeScorer(cost);
111123
}
@@ -269,9 +281,10 @@ public void testDuelCost() throws Exception {
269281

270282
// test the tester...
271283
public void testFakeScorerSupplier() {
272-
FakeScorerSupplier randomAccessSupplier = new FakeScorerSupplier(random().nextInt(100), 30);
284+
FakeScorerSupplier randomAccessSupplier =
285+
new FakeScorerSupplier(TestUtil.nextInt(random(), 31, 100), 30);
273286
expectThrows(AssertionError.class, () -> randomAccessSupplier.get(70));
274-
FakeScorerSupplier sequentialSupplier = new FakeScorerSupplier(random().nextInt(100), 70);
287+
FakeScorerSupplier sequentialSupplier = new FakeScorerSupplier(random().nextInt(70), 70);
275288
expectThrows(AssertionError.class, () -> sequentialSupplier.get(30));
276289
}
277290

@@ -289,6 +302,9 @@ public void testConjunctionLeadCost() throws IOException {
289302
new BooleanScorerSupplier(
290303
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
291304
.get(Long.MAX_VALUE); // triggers assertions as a side-effect
305+
new BooleanScorerSupplier(
306+
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
307+
.bulkScorer(); // triggers assertions as a side-effect
292308

293309
subs = new EnumMap<>(Occur.class);
294310
for (Occur occur : Occur.values()) {
@@ -315,6 +331,9 @@ public void testDisjunctionLeadCost() throws IOException {
315331
new BooleanScorerSupplier(
316332
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
317333
.get(100); // triggers assertions as a side-effect
334+
new BooleanScorerSupplier(
335+
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
336+
.bulkScorer(); // triggers assertions as a side-effect
318337

319338
subs.get(Occur.SHOULD).clear();
320339
subs.get(Occur.SHOULD).add(new FakeScorerSupplier(42, 20));
@@ -338,6 +357,9 @@ public void testDisjunctionWithMinShouldMatchLeadCost() throws IOException {
338357
new BooleanScorerSupplier(
339358
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
340359
.get(100); // triggers assertions as a side-effect
360+
new BooleanScorerSupplier(
361+
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
362+
.bulkScorer(); // triggers assertions as a side-effect
341363

342364
subs = new EnumMap<>(Occur.class);
343365
for (Occur occur : Occur.values()) {
@@ -364,6 +386,9 @@ public void testDisjunctionWithMinShouldMatchLeadCost() throws IOException {
364386
new BooleanScorerSupplier(
365387
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
366388
.get(100); // triggers assertions as a side-effect
389+
new BooleanScorerSupplier(
390+
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 2, 100)
391+
.bulkScorer(); // triggers assertions as a side-effect
367392

368393
subs = new EnumMap<>(Occur.class);
369394
for (Occur occur : Occur.values()) {
@@ -377,6 +402,9 @@ public void testDisjunctionWithMinShouldMatchLeadCost() throws IOException {
377402
new BooleanScorerSupplier(
378403
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 3, 100)
379404
.get(100); // triggers assertions as a side-effect
405+
new BooleanScorerSupplier(
406+
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 3, 100)
407+
.bulkScorer(); // triggers assertions as a side-effect
380408
}
381409

382410
public void testProhibitedLeadCost() throws IOException {
@@ -391,6 +419,9 @@ public void testProhibitedLeadCost() throws IOException {
391419
new BooleanScorerSupplier(
392420
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
393421
.get(100); // triggers assertions as a side-effect
422+
new BooleanScorerSupplier(
423+
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
424+
.bulkScorer(); // triggers assertions as a side-effect
394425

395426
subs.get(Occur.MUST).clear();
396427
subs.get(Occur.MUST_NOT).clear();
@@ -399,6 +430,9 @@ public void testProhibitedLeadCost() throws IOException {
399430
new BooleanScorerSupplier(
400431
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
401432
.get(100); // triggers assertions as a side-effect
433+
new BooleanScorerSupplier(
434+
new FakeWeight(), subs, RandomPicks.randomFrom(random(), ScoreMode.values()), 0, 100)
435+
.bulkScorer(); // triggers assertions as a side-effect
402436

403437
subs.get(Occur.MUST).clear();
404438
subs.get(Occur.MUST_NOT).clear();
@@ -420,13 +454,17 @@ public void testMixedLeadCost() throws IOException {
420454
subs.get(Occur.SHOULD).add(new FakeScorerSupplier(30, 42));
421455
new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
422456
.get(100); // triggers assertions as a side-effect
457+
new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
458+
.bulkScorer(); // triggers assertions as a side-effect
423459

424460
subs.get(Occur.MUST).clear();
425461
subs.get(Occur.SHOULD).clear();
426462
subs.get(Occur.MUST).add(new FakeScorerSupplier(42, 42));
427463
subs.get(Occur.SHOULD).add(new FakeScorerSupplier(80, 42));
428464
new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
429465
.get(100); // triggers assertions as a side-effect
466+
new BooleanScorerSupplier(new FakeWeight(), subs, ScoreMode.COMPLETE, 0, 100)
467+
.bulkScorer(); // triggers assertions as a side-effect
430468

431469
subs.get(Occur.MUST).clear();
432470
subs.get(Occur.SHOULD).clear();

0 commit comments

Comments
 (0)