Browse Source

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.
Alan Woodward 5 years ago
parent
commit
eeab09891e

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

@@ -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

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

@@ -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;
@@ -507,7 +506,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);
         }

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

@@ -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

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

@@ -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

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

@@ -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