Skip to content

Commit 2b3e41b

Browse files
original-brownbearoriginal-brownbear
authored
Speedup XContent Serialization of Settings and Mappers (#82766)
In many-shards benchmarks serializing mappers and settings becomes fairly prominent during batched index creation or setting updates. Both mapping and setting serialization spent most of their time on `org.elasticsearch.xcontent.XContentBuilder#unknownValue` figuring out write type to serialize. This commit makes it so the mapper parameters get serialized by typed serializers (the generic XContentBuilder::field default we used will always link to `org.elasticsearch.xcontent.XContentBuilder#field(java.lang.String, java.lang.Object)` which is needlessly slow here when we know the type in the callsite creating the parameter instance). Also, for settings I added some educated guesses on the types expected that cover most real world scenarios (for the non-flat case it's probably all scenarios except for `null` setting values and that's the case that matters).
1 parent 6b6f06e commit 2b3e41b

File tree

27 files changed

+276
-111
lines changed

27 files changed

+276
-111
lines changed

‎libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilder.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,9 @@ public static XContentBuilder builder(XContentType xContentType, Set<String> inc
157157
dateTransformers.putAll(addlDateTransformers);
158158
}
159159

160-
WRITERS = Collections.unmodifiableMap(writers);
161-
HUMAN_READABLE_TRANSFORMERS = Collections.unmodifiableMap(humanReadableTransformer);
162-
DATE_TRANSFORMERS = Collections.unmodifiableMap(dateTransformers);
160+
WRITERS = Map.copyOf(writers);
161+
HUMAN_READABLE_TRANSFORMERS = Map.copyOf(humanReadableTransformer);
162+
DATE_TRANSFORMERS = Map.copyOf(dateTransformers);
163163
}
164164

165165
@FunctionalInterface
@@ -867,6 +867,29 @@ public XContentBuilder field(String name, Object value) throws IOException {
867867
return field(name).value(value);
868868
}
869869

870+
public XContentBuilder field(String name, Number value) throws IOException {
871+
field(name);
872+
if (value instanceof Short) {
873+
return value(value.shortValue());
874+
} else if (value instanceof Integer) {
875+
return value(value.intValue());
876+
} else if (value instanceof Long) {
877+
return value(value.longValue());
878+
} else if (value instanceof Float) {
879+
return value(value.floatValue());
880+
} else if (value instanceof Double) {
881+
return value(value.doubleValue());
882+
} else if (value instanceof BigInteger) {
883+
generator.writeNumber((BigInteger) value);
884+
return this;
885+
} else if (value instanceof BigDecimal) {
886+
generator.writeNumber((BigDecimal) value);
887+
return this;
888+
} else {
889+
return value(value);
890+
}
891+
}
892+
870893
public XContentBuilder array(String name, Object... values) throws IOException {
871894
return field(name).values(values, true);
872895
}

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

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.elasticsearch.legacygeo.builders.ShapeBuilder;
4343
import org.elasticsearch.legacygeo.parsers.ShapeParser;
4444
import org.elasticsearch.legacygeo.query.LegacyGeoShapeQueryProcessor;
45+
import org.elasticsearch.xcontent.XContentBuilder;
4546
import org.elasticsearch.xcontent.XContentParser;
4647
import org.locationtech.spatial4j.shape.Point;
4748
import org.locationtech.spatial4j.shape.Shape;
@@ -155,36 +156,64 @@ public static class Builder extends FieldMapper.Builder {
155156
false,
156157
() -> SpatialStrategy.RECURSIVE,
157158
(n, c, o) -> SpatialStrategy.fromString(o.toString()),
158-
m -> builder(m).strategy.get()
159+
m -> builder(m).strategy.get(),
160+
(b, f, v) -> b.field(f, v.getStrategyName()),
161+
SpatialStrategy::getStrategyName
159162
).deprecated();
160163
Parameter<String> tree = Parameter.stringParam("tree", false, m -> builder(m).tree.get(), Defaults.TREE).deprecated();
161164
Parameter<Integer> treeLevels = new Parameter<>(
162165
"tree_levels",
163166
false,
164167
() -> null,
165168
(n, c, o) -> o == null ? null : XContentMapValues.nodeIntegerValue(o),
166-
m -> builder(m).treeLevels.get()
169+
m -> builder(m).treeLevels.get(),
170+
(b, f, v) -> {
171+
if (v != null && v != 0) {
172+
b.field(f, v);
173+
} else {
174+
b.field(f, Defaults.defaultTreeLevel(tree.get()));
175+
}
176+
},
177+
Objects::toString
167178
).deprecated();
168179
Parameter<DistanceUnit.Distance> precision = new Parameter<>(
169180
"precision",
170181
false,
171182
() -> null,
172183
(n, c, o) -> o == null ? null : DistanceUnit.Distance.parseDistance(o.toString()),
173-
m -> builder(m).precision.get()
184+
m -> builder(m).precision.get(),
185+
(b, f, v) -> {
186+
if (v == null) {
187+
b.field(f, "50.0m");
188+
} else {
189+
b.field(f, v.toString());
190+
}
191+
},
192+
Objects::toString
174193
).deprecated();
175194
Parameter<Double> distanceErrorPct = new Parameter<>(
176195
"distance_error_pct",
177196
true,
178197
() -> null,
179198
(n, c, o) -> o == null ? null : XContentMapValues.nodeDoubleValue(o),
180-
m -> builder(m).distanceErrorPct.get()
199+
m -> builder(m).distanceErrorPct.get(),
200+
XContentBuilder::field,
201+
Objects::toString
181202
).deprecated().acceptsNull();
182203
Parameter<Boolean> pointsOnly = new Parameter<>(
183204
"points_only",
184205
false,
185206
() -> null,
186207
(n, c, o) -> XContentMapValues.nodeBooleanValue(o),
187-
m -> builder(m).pointsOnly.get()
208+
m -> builder(m).pointsOnly.get(),
209+
(b, f, v) -> {
210+
if (v == null) {
211+
b.field(f, strategy.get() == SpatialStrategy.TERM);
212+
} else {
213+
b.field(f, v);
214+
}
215+
},
216+
Objects::toString
188217
).deprecated().acceptsNull();
189218

190219
Parameter<Map<String, String>> meta = Parameter.metaParam();
@@ -215,32 +244,10 @@ public Builder(String name, Version version, boolean ignoreMalformedByDefault, b
215244
if (version.onOrAfter(Version.V_7_0_0)) {
216245
this.strategy.alwaysSerialize();
217246
}
218-
this.strategy.setSerializer((b, f, v) -> b.field(f, v.getStrategyName()), SpatialStrategy::getStrategyName);
219247
// serialize treeLevels if treeLevels is configured, OR if defaults are requested and precision is not configured
220248
treeLevels.setSerializerCheck((id, ic, v) -> ic || (id && precision.get() == null));
221-
treeLevels.setSerializer((b, f, v) -> {
222-
if (v != null && v != 0) {
223-
b.field(f, v);
224-
} else {
225-
b.field(f, Defaults.defaultTreeLevel(tree.get()));
226-
}
227-
}, Objects::toString);
228249
// serialize precision if precision is configured, OR if defaults are requested and treeLevels is not configured
229250
precision.setSerializerCheck((id, ic, v) -> ic || (id && treeLevels.get() == null));
230-
precision.setSerializer((b, f, v) -> {
231-
if (v == null) {
232-
b.field(f, "50.0m");
233-
} else {
234-
b.field(f, v.toString());
235-
}
236-
}, Objects::toString);
237-
pointsOnly.setSerializer((b, f, v) -> {
238-
if (v == null) {
239-
b.field(f, strategy.get() == SpatialStrategy.TERM);
240-
} else {
241-
b.field(f, v);
242-
}
243-
}, Objects::toString);
244251
}
245252

246253
@Override

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.elasticsearch.search.DocValueFormat;
4545
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
4646
import org.elasticsearch.search.lookup.SearchLookup;
47+
import org.elasticsearch.xcontent.XContentBuilder;
4748
import org.elasticsearch.xcontent.XContentParser;
4849
import org.elasticsearch.xcontent.XContentParser.Token;
4950

@@ -55,6 +56,7 @@
5556
import java.util.Collections;
5657
import java.util.List;
5758
import java.util.Map;
59+
import java.util.Objects;
5860
import java.util.function.Supplier;
5961

6062
/** A {@link FieldMapper} for scaled floats. Values are internally multiplied
@@ -84,7 +86,9 @@ public static class Builder extends FieldMapper.Builder {
8486
false,
8587
() -> null,
8688
(n, c, o) -> XContentMapValues.nodeDoubleValue(o),
87-
m -> toType(m).scalingFactor
89+
m -> toType(m).scalingFactor,
90+
XContentBuilder::field,
91+
Objects::toString
8892
).addValidator(v -> {
8993
if (v == null) {
9094
throw new IllegalArgumentException("Field [scaling_factor] is required");
@@ -98,7 +102,9 @@ public static class Builder extends FieldMapper.Builder {
98102
false,
99103
() -> null,
100104
(n, c, o) -> o == null ? null : XContentMapValues.nodeDoubleValue(o),
101-
m -> toType(m).nullValue
105+
m -> toType(m).nullValue,
106+
XContentBuilder::field,
107+
Objects::toString
102108
).acceptsNull();
103109

104110
private final Parameter<Map<String, String>> meta = Parameter.metaParam();

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapper.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@
2121
import org.elasticsearch.index.mapper.NumberFieldMapper;
2222
import org.elasticsearch.index.mapper.ValueFetcher;
2323
import org.elasticsearch.index.query.SearchExecutionContext;
24+
import org.elasticsearch.xcontent.XContentBuilder;
2425

2526
import java.io.IOException;
2627
import java.util.Arrays;
2728
import java.util.List;
2829
import java.util.Map;
30+
import java.util.Objects;
2931

3032
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeIntegerValue;
3133

@@ -52,7 +54,9 @@ public static class Builder extends FieldMapper.Builder {
5254
false,
5355
() -> null,
5456
(n, c, o) -> o == null ? null : nodeIntegerValue(o),
55-
m -> toType(m).nullValue
57+
m -> toType(m).nullValue,
58+
XContentBuilder::field,
59+
Objects::toString
5660
).acceptsNull();
5761
private final Parameter<Boolean> enablePositionIncrements = Parameter.boolParam(
5862
"enable_position_increments",

‎modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Iterator;
4545
import java.util.List;
4646
import java.util.Map;
47+
import java.util.Objects;
4748
import java.util.Set;
4849
import java.util.function.Supplier;
4950
import java.util.stream.Collectors;
@@ -100,7 +101,9 @@ public static class Builder extends FieldMapper.Builder {
100101
true,
101102
Collections::emptyList,
102103
(n, c, o) -> Relations.parse(o),
103-
m -> toType(m).relations
104+
m -> toType(m).relations,
105+
XContentBuilder::field,
106+
Objects::toString
104107
).setMergeValidator(ParentJoinFieldMapper::checkRelationsConflicts);
105108

106109
final Parameter<Map<String, String>> meta = Parameter.metaParam();

‎server/src/main/java/org/elasticsearch/common/settings/Settings.java‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -615,17 +615,48 @@ public static Builder builder() {
615615
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
616616
Settings settings = SettingsFilter.filterSettings(params, this);
617617
if (params.paramAsBoolean("flat_settings", false) == false) {
618-
for (Map.Entry<String, Object> entry : settings.getAsStructuredMap().entrySet()) {
619-
builder.field(entry.getKey(), entry.getValue());
620-
}
618+
toXContentFlat(builder, settings);
621619
} else {
622-
for (Map.Entry<String, Object> entry : settings.settings.entrySet()) {
623-
builder.field(entry.getKey(), entry.getValue());
624-
}
620+
toXContent(builder, settings);
625621
}
626622
return builder;
627623
}
628624

625+
@SuppressWarnings("unchecked")
626+
private static void toXContent(XContentBuilder builder, Settings settings) throws IOException {
627+
for (Map.Entry<String, Object> entry : settings.settings.entrySet()) {
628+
final Object value = entry.getValue();
629+
final String key = entry.getKey();
630+
if (value instanceof String) {
631+
// most setting values are string
632+
builder.field(key, (String) value);
633+
} else if (value instanceof List) {
634+
// all setting lists are lists of String so we can save the expensive type detection in the builder
635+
builder.stringListField(key, (List<String>) value);
636+
} else {
637+
// this should be rare, let the builder figure out the type
638+
builder.field(key, value);
639+
}
640+
}
641+
}
642+
643+
@SuppressWarnings("unchecked")
644+
private void toXContentFlat(XContentBuilder builder, Settings settings) throws IOException {
645+
for (Map.Entry<String, Object> entry : settings.getAsStructuredMap().entrySet()) {
646+
final Object value = entry.getValue();
647+
final String key = entry.getKey();
648+
if (value instanceof String) {
649+
builder.field(key, (String) value);
650+
} else if (value instanceof Map) {
651+
// lots of maps in flattened settings so far cheaper to check here then to have the XContent builder figure out the type
652+
builder.field(key).map((Map<String, ?>) value);
653+
} else {
654+
// this should be rare, let the builder figure out the type
655+
builder.field(key, value);
656+
}
657+
}
658+
}
659+
629660
/**
630661
* Parsers the generated xcontent from {@link Settings#toXContent(XContentBuilder, Params)} into a new Settings object.
631662
* Note this method requires the parser to either be positioned on a null token or on

‎server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.xcontent.XContentParser;
1616

1717
import java.io.IOException;
18+
import java.util.Objects;
1819
import java.util.function.Consumer;
1920
import java.util.function.Function;
2021
import java.util.function.Supplier;
@@ -25,9 +26,10 @@ public abstract class AbstractPointGeometryFieldMapper<T> extends AbstractGeomet
2526
public static <T> Parameter<T> nullValueParam(
2627
Function<FieldMapper, T> initializer,
2728
TriFunction<String, MappingParserContext, Object, T> parser,
28-
Supplier<T> def
29+
Supplier<T> def,
30+
Serializer<T> serializer
2931
) {
30-
return new Parameter<T>("null_value", false, def, parser, initializer);
32+
return new Parameter<T>("null_value", false, def, parser, initializer, serializer, Objects::toString);
3133
}
3234

3335
protected final T nullValue;

‎server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.elasticsearch.script.Script;
2323
import org.elasticsearch.script.ScriptContext;
2424
import org.elasticsearch.search.lookup.SearchLookup;
25+
import org.elasticsearch.xcontent.XContentBuilder;
2526

2627
import java.time.ZoneId;
2728
import java.util.ArrayList;
@@ -218,7 +219,9 @@ abstract static class Builder<Factory> extends RuntimeField.Builder {
218219
true,
219220
() -> null,
220221
RuntimeField::parseScript,
221-
RuntimeField.initializerNotSupported()
222+
RuntimeField.initializerNotSupported(),
223+
XContentBuilder::field,
224+
Objects::toString
222225
).setSerializerCheck((id, ic, v) -> ic);
223226

224227
Builder(String name, ScriptContext<Factory> scriptContext) {

‎server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ public static Parameter<Explicit<Orientation>> orientationParam(Function<FieldMa
3232
true,
3333
() -> IMPLICIT_RIGHT,
3434
(n, c, o) -> new Explicit<>(Orientation.fromString(o.toString()), true),
35-
initializer
36-
).setSerializer((b, f, v) -> b.field(f, v.value()), v -> v.value().toString());
35+
initializer,
36+
(b, f, v) -> b.field(f, v.value()),
37+
v -> v.value().toString()
38+
);
3739
}
3840

3941
public abstract static class AbstractShapeGeometryFieldType<T> extends AbstractGeometryFieldType<T> {

‎server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.elasticsearch.search.DocValueFormat;
3333
import org.elasticsearch.search.lookup.FieldValues;
3434
import org.elasticsearch.search.lookup.SearchLookup;
35+
import org.elasticsearch.xcontent.XContentBuilder;
3536
import org.elasticsearch.xcontent.XContentParser;
3637

3738
import java.io.IOException;
@@ -80,7 +81,9 @@ public static class Builder extends FieldMapper.Builder {
8081
false,
8182
() -> null,
8283
(n, c, o) -> o == null ? null : XContentMapValues.nodeBooleanValue(o),
83-
m -> toType(m).nullValue
84+
m -> toType(m).nullValue,
85+
XContentBuilder::field,
86+
Objects::toString
8487
).acceptsNull();
8588

8689
private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).script);

0 commit comments

Comments
 (0)