Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make inlining decisions a bit more predictable in our main queries. #14023

Merged
merged 7 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lucene/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ Optimizations
* GITHUB#14021: WANDScorer now computes scores on the fly, which helps prevent
advancing "tail" clauses in many cases. (Adrien Grand)

* GITHUB#14023: Make JVM inlining decisions more predictable in our main
queries. (Adrien Grand)

Bug Fixes
---------------------
* GITHUB#13832: Fixed an issue where the DefaultPassageFormatter.format method did not format passages as intended
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ final class BlockMaxConjunctionBulkScorer extends BulkScorer {
private final Scorer[] scorers;
private final DocIdSetIterator[] iterators;
private final DocIdSetIterator lead1, lead2;
private final Scorer scorer1, scorer2;
private final Scorable scorer1, scorer2;
private final DocAndScore scorable = new DocAndScore();
private final double[] sumOfOtherClauses;
private final int maxDoc;
Expand All @@ -51,10 +51,10 @@ final class BlockMaxConjunctionBulkScorer extends BulkScorer {
Arrays.sort(this.scorers, Comparator.comparingLong(scorer -> scorer.iterator().cost()));
this.iterators =
Arrays.stream(this.scorers).map(Scorer::iterator).toArray(DocIdSetIterator[]::new);
lead1 = iterators[0];
lead2 = iterators[1];
scorer1 = this.scorers[0];
scorer2 = this.scorers[1];
lead1 = ScorerUtil.likelyImpactsEnum(iterators[0]);
lead2 = ScorerUtil.likelyImpactsEnum(iterators[1]);
scorer1 = ScorerUtil.likelyTermScorer(this.scorers[0]);
scorer2 = ScorerUtil.likelyTermScorer(this.scorers[1]);
this.sumOfOtherClauses = new double[this.scorers.length];
for (int i = 0; i < sumOfOtherClauses.length; i++) {
sumOfOtherClauses[i] = Double.POSITIVE_INFINITY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public int count() throws IOException {
this.needsScores = needsScores;
LongArrayList costs = new LongArrayList(scorers.size());
for (Scorer scorer : scorers) {
DisiWrapper w = new DisiWrapper(scorer);
DisiWrapper w = new DisiWrapper(scorer, false);
costs.add(w.cost);
final DisiWrapper evicted = tail.insertWithOverflow(w);
if (evicted != null) {
Expand All @@ -177,7 +177,7 @@ private void scoreDisiWrapperIntoBitSet(DisiWrapper w, Bits acceptDocs, int min,
Bucket[] buckets = BooleanScorer.this.buckets;

DocIdSetIterator it = w.iterator;
Scorer scorer = w.scorer;
Scorable scorer = w.scorer;
int doc = w.doc;
if (doc < min) {
doc = it.advance(min);
Expand Down Expand Up @@ -300,7 +300,7 @@ private void scoreWindowSingleScorer(
if (doc < windowMin) {
doc = it.advance(windowMin);
}
collector.setScorer(w.scorer);
collector.setScorer(w.scorable);
for (; doc < end; doc = it.nextDoc()) {
if (acceptDocs == null || acceptDocs.get(doc)) {
collector.collect(doc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
*/
final class ConjunctionBulkScorer extends BulkScorer {

private final Scorer[] scoringScorers;
private final Scorable[] scoringScorers;
private final DocIdSetIterator lead1, lead2;
private final List<DocIdSetIterator> others;
private final Scorable scorable;
Expand All @@ -45,21 +45,22 @@ final class ConjunctionBulkScorer extends BulkScorer {
allScorers.addAll(requiredScoring);
allScorers.addAll(requiredNoScoring);

this.scoringScorers = requiredScoring.toArray(Scorer[]::new);
this.scoringScorers =
requiredScoring.stream().map(ScorerUtil::likelyTermScorer).toArray(Scorable[]::new);
List<DocIdSetIterator> iterators = new ArrayList<>();
for (Scorer scorer : allScorers) {
iterators.add(scorer.iterator());
}
Collections.sort(iterators, Comparator.comparingLong(DocIdSetIterator::cost));
lead1 = iterators.get(0);
lead2 = iterators.get(1);
lead1 = ScorerUtil.likelyPostingsEnum(iterators.get(0));
lead2 = ScorerUtil.likelyPostingsEnum(iterators.get(1));
others = List.copyOf(iterators.subList(2, iterators.size()));
scorable =
new Scorable() {
@Override
public float score() throws IOException {
double score = 0;
for (Scorer scorer : scoringScorers) {
for (Scorable scorer : scoringScorers) {
score += scorer.score();
}
return (float) score;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ private ConjunctionDISI(List<? extends DocIdSetIterator> iterators) {
// Sort the array the first time to allow the least frequent DocsEnum to
// lead the matching.
CollectionUtil.timSort(iterators, (o1, o2) -> Long.compare(o1.cost(), o2.cost()));
lead1 = iterators.get(0);
lead2 = iterators.get(1);
others = iterators.subList(2, iterators.size()).toArray(new DocIdSetIterator[0]);
lead1 = ScorerUtil.likelyPostingsEnum(iterators.get(0));
lead2 = ScorerUtil.likelyPostingsEnum(iterators.get(1));
others = iterators.subList(2, iterators.size()).toArray(DocIdSetIterator[]::new);
}

private int doNext(int doc) throws IOException {
Expand Down
16 changes: 12 additions & 4 deletions lucene/core/src/java/org/apache/lucene/search/DisiWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package org.apache.lucene.search;

import java.util.Objects;

/**
* Wrapper used in {@link DisiPriorityQueue}.
*
Expand All @@ -24,6 +26,7 @@
public class DisiWrapper {
public final DocIdSetIterator iterator;
public final Scorer scorer;
public final Scorable scorable;
public final long cost;
public final float matchCost; // the match cost for two-phase iterators, 0 otherwise
public int doc; // the current doc, used for comparison
Expand All @@ -42,15 +45,20 @@ public class DisiWrapper {
// for MaxScoreBulkScorer
float maxWindowScore;

public DisiWrapper(Scorer scorer) {
this.scorer = scorer;
this.iterator = scorer.iterator();
public DisiWrapper(Scorer scorer, boolean impacts) {
this.scorer = Objects.requireNonNull(scorer);
this.scorable = ScorerUtil.likelyTermScorer(scorer);
if (impacts) {
this.iterator = ScorerUtil.likelyImpactsEnum(scorer.iterator());
} else {
this.iterator = ScorerUtil.likelyPostingsEnum(scorer.iterator());
}
this.cost = iterator.cost();
this.doc = -1;
this.twoPhaseView = scorer.twoPhaseIterator();

if (twoPhaseView != null) {
approximation = twoPhaseView.approximation();
approximation = ScorerUtil.likelyPostingsEnum(twoPhaseView.approximation());
matchCost = twoPhaseView.matchCost();
} else {
approximation = iterator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ protected float score(DisiWrapper topList) throws IOException {
float scoreMax = 0;
double otherScoreSum = 0;
for (DisiWrapper w = topList; w != null; w = w.next) {
float subScore = w.scorer.score();
float subScore = w.scorable.score();
if (subScore >= scoreMax) {
otherScoreSum += scoreMax;
scoreMax = subScore;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected DisjunctionScorer(List<Scorer> subScorers, ScoreMode scoreMode) throws
}
this.subScorers = new DisiPriorityQueue(subScorers.size());
for (Scorer scorer : subScorers) {
final DisiWrapper w = new DisiWrapper(scorer);
final DisiWrapper w = new DisiWrapper(scorer, false);
this.subScorers.add(w);
}
this.needsScores = scoreMode != ScoreMode.COMPLETE_NO_SCORES;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected float score(DisiWrapper topList) throws IOException {
double score = 0;

for (DisiWrapper w = topList; w != null; w = w.next) {
score += w.scorer.score();
score += w.scorable.score();
}
return (float) score;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.lucene.search;

import java.io.IOException;

/** Wrapper around a {@link DocIdSetIterator}. */
public class FilterDocIdSetIterator extends DocIdSetIterator {

protected final DocIdSetIterator in;

/** Sole constructor. */
public FilterDocIdSetIterator(DocIdSetIterator in) {
this.in = in;
}

@Override
public int docID() {
return in.docID();
}

@Override
public int nextDoc() throws IOException {
return in.nextDoc();
}

@Override
public int advance(int target) throws IOException {
return in.advance(target);
}

@Override
public long cost() {
return in.cost();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ protected IndriDisjunctionScorer(List<Scorer> subScorersList, ScoreMode scoreMod
this.subScorersList = subScorersList;
this.subScorers = new DisiPriorityQueue(subScorersList.size());
for (Scorer scorer : subScorersList) {
final DisiWrapper w = new DisiWrapper(scorer);
final DisiWrapper w = new DisiWrapper(scorer, false);
this.subScorers.add(w);
}
this.approximation = new DisjunctionDISIApproximation(this.subScorers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ final class MaxScoreBulkScorer extends BulkScorer {
int i = 0;
long cost = 0;
for (Scorer scorer : scorers) {
DisiWrapper w = new DisiWrapper(scorer);
DisiWrapper w = new DisiWrapper(scorer, true);
cost += w.cost;
allScorers[i++] = w;
}
Expand Down Expand Up @@ -168,7 +168,7 @@ private void scoreInnerWindowSingleEssentialClause(
if (acceptDocs != null && acceptDocs.get(doc) == false) {
continue;
}
scoreNonEssentialClauses(collector, doc, top.scorer.score(), firstEssentialScorer);
scoreNonEssentialClauses(collector, doc, top.scorable.score(), firstEssentialScorer);
}
top.doc = top.iterator.docID();
essentialQueue.updateTop();
Expand Down Expand Up @@ -196,7 +196,7 @@ private void scoreInnerWindowAsConjunction(LeafCollector collector, Bits acceptD
continue;
}

double score = lead1.scorer.score();
double score = lead1.scorable.score();

// We specialize handling the second best scorer, which seems to help a bit with performance.
// But this is the exact same logic as in the below for loop.
Expand All @@ -215,7 +215,7 @@ private void scoreInnerWindowAsConjunction(LeafCollector collector, Bits acceptD
continue;
}

score += lead2.scorer.score();
score += lead2.scorable.score();

for (int i = allScorers.length - 3; i >= firstRequiredScorer; --i) {
if ((float) MathUtil.sumUpperBound(score + maxScoreSums[i], allScorers.length)
Expand All @@ -233,7 +233,7 @@ private void scoreInnerWindowAsConjunction(LeafCollector collector, Bits acceptD
lead1.doc = lead1.iterator.advance(Math.min(w.doc, max));
continue outer;
}
score += w.scorer.score();
score += w.scorable.score();
}

scoreNonEssentialClauses(collector, lead1.doc, score, firstRequiredScorer);
Expand All @@ -254,7 +254,7 @@ private void scoreInnerWindowMultipleEssentialClauses(
if (acceptDocs == null || acceptDocs.get(doc)) {
final int i = doc - innerWindowMin;
windowMatches[i >>> 6] |= 1L << i;
windowScores[i] += top.scorer.score();
windowScores[i] += top.scorable.score();
}
}
top.doc = top.iterator.docID();
Expand Down Expand Up @@ -344,7 +344,7 @@ private void scoreNonEssentialClauses(
scorer.doc = scorer.iterator.advance(doc);
}
if (scorer.doc == doc) {
score += scorer.scorer.score();
score += scorer.scorable.score();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ protected boolean lessThan(PostingsEnum a, PostingsEnum b) {
DisiPriorityQueue subs = new DisiPriorityQueue(highFrequencyTerms.size() + 1);
for (DocIdSetIterator disi : highFrequencyTerms) {
Scorer s = wrapWithDummyScorer(this, disi);
subs.add(new DisiWrapper(s));
subs.add(new DisiWrapper(s, false));
}
Scorer s = wrapWithDummyScorer(this, otherTerms.build().iterator());
subs.add(new DisiWrapper(s));
subs.add(new DisiWrapper(s, false));

return new WeightOrDocIdSetIterator(new DisjunctionDISIApproximation(subs));
}
Expand Down
Loading