Skip to content

Commit eeab098

Browse files
authored
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.
1 parent 979c33b commit eeab098

File tree

5 files changed

+9
-10
lines changed

5 files changed

+9
-10
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public DoubleValues getValues(LeafReaderContext leaf, DoubleValues scores) {
5454
LeafNumericFieldData leafData = (LeafNumericFieldData) fieldData.load(leaf);
5555
MutableDateTime joda = new MutableDateTime(0, DateTimeZone.UTC);
5656
NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues());
57-
return DoubleValues.withDefault(new DoubleValues() {
57+
return new DoubleValues() {
5858
@Override
5959
public double doubleValue() throws IOException {
6060
joda.setMillis((long)docValues.doubleValue());
@@ -65,7 +65,7 @@ public double doubleValue() throws IOException {
6565
public boolean advanceExact(int doc) throws IOException {
6666
return docValues.advanceExact(doc);
6767
}
68-
}, 0);
68+
};
6969
}
7070

7171
@Override

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.apache.lucene.expressions.SimpleBindings;
2424
import org.apache.lucene.expressions.js.JavascriptCompiler;
2525
import org.apache.lucene.expressions.js.VariableContext;
26-
import org.apache.lucene.queries.function.valuesource.DoubleConstValueSource;
2726
import org.apache.lucene.search.DoubleValuesSource;
2827
import org.apache.lucene.search.SortField;
2928
import org.elasticsearch.SpecialPermission;
@@ -507,7 +506,7 @@ private static void bindFromParams(@Nullable final Map<String, Object> params,
507506
// but if we were to reverse it, we could provide a way to supply dynamic defaults for documents missing the field?
508507
Object value = params.get(variable);
509508
if (value instanceof Number) {
510-
bindings.add(variable, new DoubleConstValueSource(((Number) value).doubleValue()).asDoubleValuesSource());
509+
bindings.add(variable, DoubleValuesSource.constant(((Number)value).doubleValue()));
511510
} else {
512511
throw new ParseException("Parameter [" + variable + "] must be a numeric type", 0);
513512
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ final class GeoEmptyValueSource extends FieldDataBasedDoubleValuesSource {
4040
public DoubleValues getValues(LeafReaderContext leaf, DoubleValues scores) {
4141
LeafGeoPointFieldData leafData = (LeafGeoPointFieldData) fieldData.load(leaf);
4242
final MultiGeoPointValues values = leafData.getGeoPointValues();
43-
return DoubleValues.withDefault(new DoubleValues() {
43+
return new DoubleValues() {
4444
@Override
4545
public double doubleValue() {
4646
return 1;
@@ -50,7 +50,7 @@ public double doubleValue() {
5050
public boolean advanceExact(int doc) throws IOException {
5151
return values.advanceExact(doc);
5252
}
53-
}, 0);
53+
};
5454
}
5555

5656
@Override

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ final class GeoLatitudeValueSource extends FieldDataBasedDoubleValuesSource {
4040
public DoubleValues getValues(LeafReaderContext leaf, DoubleValues scores) {
4141
LeafGeoPointFieldData leafData = (LeafGeoPointFieldData) fieldData.load(leaf);
4242
final MultiGeoPointValues values = leafData.getGeoPointValues();
43-
return DoubleValues.withDefault(new DoubleValues() {
43+
return new DoubleValues() {
4444
@Override
4545
public double doubleValue() throws IOException {
4646
return values.nextValue().getLat();
@@ -50,7 +50,7 @@ public double doubleValue() throws IOException {
5050
public boolean advanceExact(int doc) throws IOException {
5151
return values.advanceExact(doc);
5252
}
53-
}, 0);
53+
};
5454
}
5555

5656
@Override

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ final class GeoLongitudeValueSource extends FieldDataBasedDoubleValuesSource {
4040
public DoubleValues getValues(LeafReaderContext leaf, DoubleValues scores) {
4141
LeafGeoPointFieldData leafData = (LeafGeoPointFieldData) fieldData.load(leaf);
4242
final MultiGeoPointValues values = leafData.getGeoPointValues();
43-
return DoubleValues.withDefault(new DoubleValues() {
43+
return new DoubleValues() {
4444
@Override
4545
public double doubleValue() throws IOException {
4646
return values.nextValue().getLon();
@@ -50,7 +50,7 @@ public double doubleValue() throws IOException {
5050
public boolean advanceExact(int doc) throws IOException {
5151
return values.advanceExact(doc);
5252
}
53-
}, 0.0);
53+
};
5454
}
5555

5656
@Override

0 commit comments

Comments
 (0)