honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Don't double-wrap values (#54432) 

After commit #53661 converted the lang-expressions module to using
DoubleValuesSource, we've seen a performance regression for expressions
that use geopoints. Some investigation suggests that this may be due to
GeoLatitudeValueSource and GeoLongitudeValueSource wrapping their
per-document values in a DoubleValues.withDefault() class. Values exposed
via expressions already have a '0' default value, so this extra wrapping is
unnecessary, and is directly on the hot path. This commit removes the extra
wrapping.
This commit is contained in:
Alan Woodward 2020-03-31 10:49:26 +01:00
parent a0853628cd
commit 25a0addb17
5 changed files with 9 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java
View File

@ -54,7 +54,7 @@ class DateObjectValueSource extends FieldDataValueSource {
LeafNumericFieldData leafData = (LeafNumericFieldData) fieldData.load(leaf);
MutableDateTime joda = new MutableDateTime(0, DateTimeZone.UTC);
NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues());
return DoubleValues.withDefault(new DoubleValues() {
return new DoubleValues() {
@Override
public double doubleValue() throws IOException {
joda.setMillis((long)docValues.doubleValue());
@ -65,7 +65,7 @@ class DateObjectValueSource extends FieldDataValueSource {
public boolean advanceExact(int doc) throws IOException {
return docValues.advanceExact(doc);
}
}, 0);
};
}
@Override

3
modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java
View File

@ -23,7 +23,6 @@ import org.apache.lucene.expressions.Expression;
import org.apache.lucene.expressions.SimpleBindings;
import org.apache.lucene.expressions.js.JavascriptCompiler;
import org.apache.lucene.expressions.js.VariableContext;
import org.apache.lucene.queries.function.valuesource.DoubleConstValueSource;
import org.apache.lucene.search.DoubleValuesSource;
import org.apache.lucene.search.SortField;
import org.elasticsearch.SpecialPermission;
@ -512,7 +511,7 @@ public class ExpressionScriptEngine implements ScriptEngine {
// but if we were to reverse it, we could provide a way to supply dynamic defaults for documents missing the field?
Object value = params.get(variable);
if (value instanceof Number) {
bindings.add(variable, new DoubleConstValueSource(((Number) value).doubleValue()).asDoubleValuesSource());
bindings.add(variable, DoubleValuesSource.constant(((Number)value).doubleValue()));
} else {
throw new ParseException("Parameter [" + variable + "] must be a numeric type", 0);
}

4
modules/lang-expression/src/main/java/org/elasticsearch/script/expression/GeoEmptyValueSource.java
View File

@ -40,7 +40,7 @@ final class GeoEmptyValueSource extends FieldDataBasedDoubleValuesSource {
public DoubleValues getValues(LeafReaderContext leaf, DoubleValues scores) {
LeafGeoPointFieldData leafData = (LeafGeoPointFieldData) fieldData.load(leaf);
final MultiGeoPointValues values = leafData.getGeoPointValues();
return DoubleValues.withDefault(new DoubleValues() {
return new DoubleValues() {
@Override
public double doubleValue() {
return 1;
@ -50,7 +50,7 @@ final class GeoEmptyValueSource extends FieldDataBasedDoubleValuesSource {
public boolean advanceExact(int doc) throws IOException {
return values.advanceExact(doc);
}
}, 0);
};
}
@Override

4
modules/lang-expression/src/main/java/org/elasticsearch/script/expression/GeoLatitudeValueSource.java
View File

@ -40,7 +40,7 @@ final class GeoLatitudeValueSource extends FieldDataBasedDoubleValuesSource {
public DoubleValues getValues(LeafReaderContext leaf, DoubleValues scores) {
LeafGeoPointFieldData leafData = (LeafGeoPointFieldData) fieldData.load(leaf);
final MultiGeoPointValues values = leafData.getGeoPointValues();
return DoubleValues.withDefault(new DoubleValues() {
return new DoubleValues() {
@Override
public double doubleValue() throws IOException {
return values.nextValue().getLat();
@ -50,7 +50,7 @@ final class GeoLatitudeValueSource extends FieldDataBasedDoubleValuesSource {
public boolean advanceExact(int doc) throws IOException {
return values.advanceExact(doc);
}
}, 0);
};
}
@Override

4
modules/lang-expression/src/main/java/org/elasticsearch/script/expression/GeoLongitudeValueSource.java
View File

@ -40,7 +40,7 @@ final class GeoLongitudeValueSource extends FieldDataBasedDoubleValuesSource {
public DoubleValues getValues(LeafReaderContext leaf, DoubleValues scores) {
LeafGeoPointFieldData leafData = (LeafGeoPointFieldData) fieldData.load(leaf);
final MultiGeoPointValues values = leafData.getGeoPointValues();
return DoubleValues.withDefault(new DoubleValues() {
return new DoubleValues() {
@Override
public double doubleValue() throws IOException {
return values.nextValue().getLon();
@ -50,7 +50,7 @@ final class GeoLongitudeValueSource extends FieldDataBasedDoubleValuesSource {
public boolean advanceExact(int doc) throws IOException {
return values.advanceExact(doc);
}
}, 0.0);
};
}
@Override