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

Speed up the function_score query when scores are not needed. #12693

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -57,6 +57,11 @@ public Explanation explainScore(int docId, Explanation subQueryScore) {
};
}

@Override
public boolean needsScores() {
return false;
}

@Override
public String toString() {
return "boost[" + boost + "]";
Expand Down
Expand Up @@ -91,6 +91,11 @@ public Explanation explainScore(int docId, Explanation subQueryScore) {
};
}

@Override
public boolean needsScores() {
return false;
}

/**
* The Type class encapsulates the modification types that can be applied
* to the score/value product.
Expand Down
Expand Up @@ -44,7 +44,7 @@ public class FunctionScoreQuery extends Query {
public FunctionScoreQuery(Query subQuery, ScoreFunction function, Float minScore) {
this.subQuery = subQuery;
this.function = function;
this.combineFunction = function == null? combineFunction.MULT : function.getDefaultScoreCombiner();
this.combineFunction = function == null? CombineFunction.MULT : function.getDefaultScoreCombiner();
this.minScore = minScore;
}

Expand Down Expand Up @@ -87,19 +87,27 @@ public Query rewrite(IndexReader reader) throws IOException {

@Override
public Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException {
// TODO: needsScores
// if we don't need scores, just return the underlying weight?
Weight subQueryWeight = subQuery.createWeight(searcher, needsScores);
return new CustomBoostFactorWeight(this, subQueryWeight);
if (needsScores == false) {
return subQuery.createWeight(searcher, needsScores);
}

boolean subQueryNeedsScores =
combineFunction != CombineFunction.REPLACE // if we don't replace we need the original score
|| function == null // when the function is null, we just multiply the score, so we need it
|| function.needsScores(); // some scripts can replace with a script that returns eg. 1/_score
Weight subQueryWeight = subQuery.createWeight(searcher, subQueryNeedsScores);
return new CustomBoostFactorWeight(this, subQueryWeight, subQueryNeedsScores);
}

class CustomBoostFactorWeight extends Weight {

final Weight subQueryWeight;
final boolean needsScores;

public CustomBoostFactorWeight(Query parent, Weight subQueryWeight) throws IOException {
public CustomBoostFactorWeight(Query parent, Weight subQueryWeight, boolean needsScores) throws IOException {
super(parent);
this.subQueryWeight = subQueryWeight;
this.needsScores = needsScores;
}

@Override
Expand Down Expand Up @@ -129,7 +137,7 @@ public Scorer scorer(LeafReaderContext context, Bits acceptDocs) throws IOExcept
if (function != null) {
leafFunction = function.getLeafScoreFunction(context);
}
return new FunctionFactorScorer(this, subQueryScorer, leafFunction, maxBoost, combineFunction, minScore);
return new FunctionFactorScorer(this, subQueryScorer, leafFunction, maxBoost, combineFunction, minScore, needsScores);
}

@Override
Expand All @@ -150,16 +158,21 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio
static class FunctionFactorScorer extends CustomBoostFactorScorer {

private final LeafScoreFunction function;
private final boolean needsScores;

private FunctionFactorScorer(CustomBoostFactorWeight w, Scorer scorer, LeafScoreFunction function, float maxBoost, CombineFunction scoreCombiner, Float minScore)
private FunctionFactorScorer(CustomBoostFactorWeight w, Scorer scorer, LeafScoreFunction function, float maxBoost, CombineFunction scoreCombiner, Float minScore, boolean needsScores)
throws IOException {
super(w, scorer, maxBoost, scoreCombiner, minScore);
this.function = function;
this.needsScores = needsScores;
}

@Override
public float innerScore() throws IOException {
float score = scorer.score();
// Even if the weight is created with needsScores=false, it might
// be costly to call score(), so we explicitly check if scores
// are needed
float score = needsScores ? scorer.score() : 0f;
if (function == null) {
return subQueryBoost * score;
} else {
Expand Down
Expand Up @@ -81,4 +81,8 @@ public Explanation explainScore(int docId, Explanation subQueryScore) {
};
}

@Override
public boolean needsScores() {
return false;
}
}
Expand Up @@ -39,4 +39,11 @@ public CombineFunction getDefaultScoreCombiner() {
}

public abstract LeafScoreFunction getLeafScoreFunction(LeafReaderContext ctx) throws IOException;

/**
* Indicates if document scores are needed by this function.
*
* @return {@code true} if scores are needed.
*/
public abstract boolean needsScores();
}
Expand Up @@ -126,6 +126,13 @@ public Explanation explainScore(int docId, Explanation subQueryScore) throws IOE
};
}

@Override
public boolean needsScores() {
// Scripts might use _score so we return true here
// TODO: Make scripts able to tell us whether they use scores
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way: lucene expressions "know this"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we speak I'm working on another change to expose this. :-)

return true;
}

@Override
public String toString() {
return "script" + sScript.toString();
Expand Down
Expand Up @@ -71,6 +71,11 @@ public Explanation explainScore(int docId, Explanation subQueryScore) throws IOE
};
}

@Override
public boolean needsScores() {
return false;
}

public Explanation explainWeight() {
return Explanation.match(getWeight(), "weight");
}
Expand Down Expand Up @@ -99,5 +104,10 @@ public Explanation explainScore(int docId, Explanation subQueryScore) {
}
};
}

@Override
public boolean needsScores() {
return false;
}
}
}
Expand Up @@ -289,6 +289,11 @@ public GeoFieldDataScoreFunction(GeoPoint origin, double scale, double decay, do
this.fieldData = fieldData;
}

@Override
public boolean needsScores() {
return false;
}

@Override
protected NumericDoubleValues distance(LeafReaderContext context) {
final MultiGeoPointValues geoPointValues = fieldData.load(context).getGeoPointValues();
Expand Down Expand Up @@ -352,6 +357,11 @@ public NumericFieldDataScoreFunction(double origin, double scale, double decay,
this.origin = origin;
}

@Override
public boolean needsScores() {
return false;
}

@Override
protected NumericDoubleValues distance(LeafReaderContext context) {
final SortedNumericDoubleValues doubleValues = fieldData.load(context).getDoubleValues();
Expand Down