Browse Source

Make most Instances of o.e.c.Explict Singletons (#82270)

Noticed that we have ~60M of duplicate instances for the boolean
variety of these on heap at a time on data nodes during many shards
benchmarking. I expect most of these are somewhat short-lived but
they're created on the sometimes very hot mapper parsing path
so I think it's nice to clean these up to make profiling less noisy.
Armin Braun 3 năm trước cách đây
mục cha
commit
d9e3f1bfa2
17 tập tin đã thay đổi với 55 bổ sung40 xóa
  1. 1 1
      modules/legacy-geo/src/main/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapper.java
  2. 1 1
      plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java
  3. 9 0
      server/src/main/java/org/elasticsearch/common/Explicit.java
  4. 3 0
      server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java
  5. 2 2
      server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java
  6. 3 1
      server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java
  7. 1 1
      server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
  8. 2 3
      server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
  9. 1 1
      server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java
  10. 1 1
      server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java
  11. 2 3
      server/src/main/java/org/elasticsearch/index/mapper/MetadataFieldMapper.java
  12. 6 6
      server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java
  13. 2 2
      server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java
  14. 0 1
      server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java
  15. 19 15
      server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java
  16. 1 1
      server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperValidationTests.java
  17. 1 1
      server/src/test/java/org/elasticsearch/index/mapper/MappingLookupTests.java

+ 1 - 1
modules/legacy-geo/src/main/java/org/elasticsearch/legacygeo/mapper/LegacyGeoShapeFieldMapper.java

@@ -266,7 +266,7 @@ public class LegacyGeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<
         }
 
         public Builder coerce(boolean coerce) {
-            this.coerce.setValue(new Explicit<>(coerce, true));
+            this.coerce.setValue(Explicit.explicitBoolean(coerce));
             return this;
         }
 

+ 1 - 1
plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java

@@ -63,7 +63,7 @@ public class SizeFieldMapper extends MetadataFieldMapper {
     }
 
     public static final TypeParser PARSER = new ConfigurableTypeParser(
-        c -> new SizeFieldMapper(new Explicit<>(false, false), new SizeFieldType()),
+        c -> new SizeFieldMapper(Explicit.IMPLICIT_FALSE, new SizeFieldType()),
         c -> new Builder()
     );
 

+ 9 - 0
server/src/main/java/org/elasticsearch/common/Explicit.java

@@ -22,9 +22,18 @@ import java.util.Objects;
  */
 public class Explicit<T> {
 
+    public static final Explicit<Boolean> EXPLICIT_TRUE = new Explicit<>(true, true);
+    public static final Explicit<Boolean> EXPLICIT_FALSE = new Explicit<>(false, true);
+    public static final Explicit<Boolean> IMPLICIT_TRUE = new Explicit<>(true, false);
+    public static final Explicit<Boolean> IMPLICIT_FALSE = new Explicit<>(false, false);
+
     private final T value;
     private final boolean explicit;
 
+    public static Explicit<Boolean> explicitBoolean(boolean value) {
+        return value ? EXPLICIT_TRUE : EXPLICIT_FALSE;
+    }
+
     /**
      * Create a value with an indication if this was an explicit choice
      * @param value a setting value

+ 3 - 0
server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java

@@ -581,6 +581,9 @@ public class XContentMapValues {
     }
 
     public static boolean nodeBooleanValue(Object node) {
+        if (node instanceof Boolean) {
+            return (Boolean) node;
+        }
         return Booleans.parseBoolean(node.toString());
     }
 

+ 2 - 2
server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java

@@ -152,8 +152,8 @@ public abstract class AbstractGeometryFieldMapper<T> extends FieldMapper {
         String onScriptError
     ) {
         super(simpleName, mappedFieldType, Collections.emptyMap(), multiFields, copyTo, true, onScriptError);
-        this.ignoreMalformed = new Explicit<>(false, true);
-        this.ignoreZValue = new Explicit<>(false, true);
+        this.ignoreMalformed = Explicit.EXPLICIT_FALSE;
+        this.ignoreZValue = Explicit.EXPLICIT_FALSE;
         this.parser = parser;
     }
 

+ 3 - 1
server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java

@@ -24,11 +24,13 @@ public abstract class AbstractShapeGeometryFieldMapper<T> extends AbstractGeomet
         return Parameter.explicitBoolParam("coerce", true, initializer, coerceByDefault);
     }
 
+    private static final Explicit<Orientation> IMPLICIT_RIGHT = new Explicit<>(Orientation.RIGHT, false);
+
     public static Parameter<Explicit<Orientation>> orientationParam(Function<FieldMapper, Explicit<Orientation>> initializer) {
         return new Parameter<>(
             "orientation",
             true,
-            () -> new Explicit<>(Orientation.RIGHT, false),
+            () -> IMPLICIT_RIGHT,
             (n, c, o) -> new Explicit<>(Orientation.fromString(o.toString()), true),
             initializer
         ).setSerializer((b, f, v) -> b.field(f, v.value()), v -> v.value().toString());

+ 1 - 1
server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -896,7 +896,7 @@ public final class DocumentParser {
 
     private static class NoOpObjectMapper extends ObjectMapper {
         NoOpObjectMapper(String name, String fullPath) {
-            super(name, fullPath, new Explicit<>(true, false), Dynamic.RUNTIME, Collections.emptyMap());
+            super(name, fullPath, Explicit.IMPLICIT_TRUE, Dynamic.RUNTIME, Collections.emptyMap());
         }
     }
 

+ 2 - 3
server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

@@ -874,12 +874,11 @@ public abstract class FieldMapper extends Mapper implements Cloneable {
             Function<FieldMapper, Explicit<Boolean>> initializer,
             boolean defaultValue
         ) {
-            Explicit<Boolean> defaultExplicit = new Explicit<>(defaultValue, false);
             return new Parameter<>(
                 name,
                 updateable,
-                () -> defaultExplicit,
-                (n, c, o) -> new Explicit<>(XContentMapValues.nodeBooleanValue(o), true),
+                defaultValue ? () -> Explicit.IMPLICIT_TRUE : () -> Explicit.IMPLICIT_FALSE,
+                (n, c, o) -> Explicit.explicitBoolean(XContentMapValues.nodeBooleanValue(o)),
                 initializer
             ).setSerializer((b, n, v) -> b.field(n, v.value()), v -> Boolean.toString(v.value()));
         }

+ 1 - 1
server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java

@@ -43,7 +43,7 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper {
     public static class Defaults {
         public static final String NAME = FieldNamesFieldMapper.NAME;
 
-        public static final Explicit<Boolean> ENABLED = new Explicit<>(true, false);
+        public static final Explicit<Boolean> ENABLED = Explicit.IMPLICIT_TRUE;
         public static final FieldType FIELD_TYPE = new FieldType();
 
         static {

+ 1 - 1
server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java

@@ -78,7 +78,7 @@ public class GeoShapeFieldMapper extends AbstractShapeGeometryFieldMapper<Geomet
         }
 
         public Builder ignoreZValue(boolean ignoreZValue) {
-            this.ignoreZValue.setValue(new Explicit<>(ignoreZValue, true));
+            this.ignoreZValue.setValue(Explicit.explicitBoolean(ignoreZValue));
             return this;
         }
 

+ 2 - 3
server/src/main/java/org/elasticsearch/index/mapper/MetadataFieldMapper.java

@@ -52,12 +52,11 @@ public abstract class MetadataFieldMapper extends FieldMapper {
         Function<FieldMapper, Explicit<Boolean>> initializer,
         boolean defaultValue
     ) {
-        Explicit<Boolean> defaultExplicit = new Explicit<>(defaultValue, false);
         return new Parameter<>(
             name,
             true,
-            () -> defaultExplicit,
-            (n, c, o) -> new Explicit<>(XContentMapValues.nodeBooleanValue(o), true),
+            defaultValue ? () -> Explicit.IMPLICIT_TRUE : () -> Explicit.IMPLICIT_FALSE,
+            (n, c, o) -> Explicit.explicitBoolean(XContentMapValues.nodeBooleanValue(o)),
             initializer
         ).setSerializer((b, n, v) -> b.field(n, v.value()), v -> Boolean.toString(v.value()));
     }

+ 6 - 6
server/src/main/java/org/elasticsearch/index/mapper/NestedObjectMapper.java

@@ -29,8 +29,8 @@ public class NestedObjectMapper extends ObjectMapper {
 
     public static class Builder extends ObjectMapper.Builder {
 
-        private Explicit<Boolean> includeInRoot = new Explicit<>(false, false);
-        private Explicit<Boolean> includeInParent = new Explicit<>(false, false);
+        private Explicit<Boolean> includeInRoot = Explicit.IMPLICIT_FALSE;
+        private Explicit<Boolean> includeInParent = Explicit.IMPLICIT_FALSE;
         private final Version indexCreatedVersion;
 
         public Builder(String name, Version indexCreatedVersion) {
@@ -39,12 +39,12 @@ public class NestedObjectMapper extends ObjectMapper {
         }
 
         Builder includeInRoot(boolean includeInRoot) {
-            this.includeInRoot = new Explicit<>(includeInRoot, true);
+            this.includeInRoot = Explicit.explicitBoolean(includeInRoot);
             return this;
         }
 
         Builder includeInParent(boolean includeInParent) {
-            this.includeInParent = new Explicit<>(includeInParent, true);
+            this.includeInParent = Explicit.explicitBoolean(includeInParent);
             return this;
         }
 
@@ -122,7 +122,7 @@ public class NestedObjectMapper extends ObjectMapper {
     }
 
     public void setIncludeInParent(boolean includeInParent) {
-        this.includeInParent = new Explicit<>(includeInParent, true);
+        this.includeInParent = Explicit.explicitBoolean(includeInParent);
     }
 
     public boolean isIncludeInRoot() {
@@ -130,7 +130,7 @@ public class NestedObjectMapper extends ObjectMapper {
     }
 
     public void setIncludeInRoot(boolean includeInRoot) {
-        this.includeInRoot = new Explicit<>(includeInRoot, true);
+        this.includeInRoot = Explicit.explicitBoolean(includeInRoot);
     }
 
     public Map<String, Mapper> getChildren() {

+ 2 - 2
server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java

@@ -62,7 +62,7 @@ public class ObjectMapper extends Mapper implements Cloneable {
 
     public static class Builder extends Mapper.Builder {
 
-        protected Explicit<Boolean> enabled = new Explicit<>(true, false);
+        protected Explicit<Boolean> enabled = Explicit.IMPLICIT_TRUE;
 
         protected Dynamic dynamic;
 
@@ -73,7 +73,7 @@ public class ObjectMapper extends Mapper implements Cloneable {
         }
 
         public Builder enabled(boolean enabled) {
-            this.enabled = new Explicit<>(enabled, true);
+            this.enabled = Explicit.explicitBoolean(enabled);
             return this;
         }
 

+ 0 - 1
server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java

@@ -54,7 +54,6 @@ public class RangeFieldMapper extends FieldMapper {
     public static final boolean DEFAULT_INCLUDE_LOWER = true;
 
     public static class Defaults {
-        public static final Explicit<Boolean> COERCE = new Explicit<>(true, false);
         public static final DateFormatter DATE_FORMATTER = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER;
     }
 

+ 19 - 15
server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java

@@ -52,19 +52,23 @@ public class RootObjectMapper extends ObjectMapper {
     static final String TOXCONTENT_SKIP_RUNTIME = "skip_runtime";
 
     public static class Defaults {
-        public static final DateFormatter[] DYNAMIC_DATE_TIME_FORMATTERS = new DateFormatter[] {
-            DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER,
-            DateFormatter.forPattern("yyyy/MM/dd HH:mm:ss||yyyy/MM/dd||epoch_millis") };
-        public static final boolean DATE_DETECTION = true;
-        public static final boolean NUMERIC_DETECTION = false;
+        public static final Explicit<DateFormatter[]> DYNAMIC_DATE_TIME_FORMATTERS = new Explicit<>(
+            new DateFormatter[] {
+                DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER,
+                DateFormatter.forPattern("yyyy/MM/dd HH:mm:ss||yyyy/MM/dd||epoch_millis") },
+            false
+        );
+        public static final Explicit<Boolean> DATE_DETECTION = Explicit.IMPLICIT_TRUE;
+        public static final Explicit<Boolean> NUMERIC_DETECTION = Explicit.IMPLICIT_FALSE;
+        private static final Explicit<DynamicTemplate[]> DYNAMIC_TEMPLATES = new Explicit<>(new DynamicTemplate[0], false);
     }
 
     public static class Builder extends ObjectMapper.Builder {
+        protected Explicit<DynamicTemplate[]> dynamicTemplates = Defaults.DYNAMIC_TEMPLATES;
+        protected Explicit<DateFormatter[]> dynamicDateTimeFormatters = Defaults.DYNAMIC_DATE_TIME_FORMATTERS;
 
-        protected Explicit<DynamicTemplate[]> dynamicTemplates = new Explicit<>(new DynamicTemplate[0], false);
-        protected Explicit<DateFormatter[]> dynamicDateTimeFormatters = new Explicit<>(Defaults.DYNAMIC_DATE_TIME_FORMATTERS, false);
-        protected Explicit<Boolean> dateDetection = new Explicit<>(Defaults.DATE_DETECTION, false);
-        protected Explicit<Boolean> numericDetection = new Explicit<>(Defaults.NUMERIC_DETECTION, false);
+        protected Explicit<Boolean> dateDetection = Defaults.DATE_DETECTION;
+        protected Explicit<Boolean> numericDetection = Defaults.NUMERIC_DETECTION;
         protected Map<String, RuntimeField> runtimeFields;
 
         public Builder(String name) {
@@ -213,10 +217,10 @@ public class RootObjectMapper extends ObjectMapper {
                 builder.dynamicTemplates(templates);
                 return true;
             } else if (fieldName.equals("date_detection")) {
-                builder.dateDetection = new Explicit<>(nodeBooleanValue(fieldNode, "date_detection"), true);
+                builder.dateDetection = Explicit.explicitBoolean(nodeBooleanValue(fieldNode, "date_detection"));
                 return true;
             } else if (fieldName.equals("numeric_detection")) {
-                builder.numericDetection = new Explicit<>(nodeBooleanValue(fieldNode, "numeric_detection"), true);
+                builder.numericDetection = Explicit.explicitBoolean(nodeBooleanValue(fieldNode, "numeric_detection"));
                 return true;
             } else if (fieldName.equals("runtime")) {
                 if (fieldNode instanceof Map) {
@@ -273,10 +277,10 @@ public class RootObjectMapper extends ObjectMapper {
         // for dynamic updates, no need to carry root-specific options, we just
         // set everything to their implicit default value so that they are not
         // applied at merge time
-        copy.dynamicTemplates = new Explicit<>(new DynamicTemplate[0], false);
-        copy.dynamicDateTimeFormatters = new Explicit<>(Defaults.DYNAMIC_DATE_TIME_FORMATTERS, false);
-        copy.dateDetection = new Explicit<>(Defaults.DATE_DETECTION, false);
-        copy.numericDetection = new Explicit<>(Defaults.NUMERIC_DETECTION, false);
+        copy.dynamicTemplates = Defaults.DYNAMIC_TEMPLATES;
+        copy.dynamicDateTimeFormatters = Defaults.DYNAMIC_DATE_TIME_FORMATTERS;
+        copy.dateDetection = Defaults.DATE_DETECTION;
+        copy.numericDetection = Defaults.NUMERIC_DETECTION;
         // also no need to carry the already defined runtime fields, only new ones need to be added
         copy.runtimeFields.clear();
         return copy;

+ 1 - 1
server/src/test/java/org/elasticsearch/index/mapper/FieldAliasMapperValidationTests.java

@@ -162,7 +162,7 @@ public class FieldAliasMapperValidationTests extends ESTestCase {
     }
 
     private static ObjectMapper createObjectMapper(String name) {
-        return new ObjectMapper(name, name, new Explicit<>(true, false), ObjectMapper.Dynamic.FALSE, emptyMap());
+        return new ObjectMapper(name, name, Explicit.IMPLICIT_TRUE, ObjectMapper.Dynamic.FALSE, emptyMap());
     }
 
     private static NestedObjectMapper createNestedObjectMapper(String name) {

+ 1 - 1
server/src/test/java/org/elasticsearch/index/mapper/MappingLookupTests.java

@@ -76,7 +76,7 @@ public class MappingLookupTests extends ESTestCase {
         ObjectMapper objectMapper = new ObjectMapper(
             "object",
             "object",
-            new Explicit<>(true, true),
+            Explicit.EXPLICIT_TRUE,
             ObjectMapper.Dynamic.TRUE,
             Collections.singletonMap("object.subfield", fieldMapper)
         );