Skip to content

Commit

Permalink
Speed up FieldMapper construction/parsing/serialization (#86860)
Browse files Browse the repository at this point in the history
Speeding this up some more as it's now 50% of the bootstrap time of the many shards benchmarks.
Iterating an array here in all cases is quite a bit faster than iterating various kinds of lists
and doesn't complicate the code. Also removes a redundant call to `getValue()` for each parameter
during serialization.
  • Loading branch information
original-brownbear committed May 23, 2022
1 parent dbf37c9 commit 7a25453
Show file tree
Hide file tree
Showing 45 changed files with 116 additions and 139 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,8 @@ public Builder(String name, Version version, boolean ignoreMalformedByDefault, b
}

@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] {
indexed,
ignoreMalformed,
ignoreZValue,
Expand All @@ -266,8 +266,7 @@ protected List<Parameter<?>> getParameters() {
precision,
distanceErrorPct,
pointsOnly,
meta
);
meta };
}

public Builder coerce(boolean coerce) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAna
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { meta };
}

private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
import org.elasticsearch.xcontent.XContentParser.Token;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

Expand Down Expand Up @@ -71,8 +69,8 @@ public Builder(String name) {
}

@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(positiveScoreImpact, meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { positiveScoreImpact, meta };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.xcontent.XContentParser.Token;

import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

Expand Down Expand Up @@ -56,8 +55,8 @@ public Builder(String name) {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(positiveScoreImpact, meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { positiveScoreImpact, meta };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ public Builder metric(TimeSeriesParams.MetricType metric) {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(indexed, hasDocValues, stored, ignoreMalformed, meta, scalingFactor, coerce, nullValue, metric);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { indexed, hasDocValues, stored, ignoreMalformed, meta, scalingFactor, coerce, nullValue, metric };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAna
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] {
index,
store,
docValues,
Expand All @@ -169,8 +169,7 @@ protected List<Parameter<?>> getParameters() {
indexOptions,
norms,
termVectors,
meta
);
meta };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -72,8 +71,8 @@ public Builder(String name) {
}

@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(index, hasDocValues, store, analyzer, nullValue, enablePositionIncrements, meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { index, hasDocValues, store, analyzer, nullValue, enablePositionIncrements, meta };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
Expand Down Expand Up @@ -121,8 +120,8 @@ public Builder addRelation(String parent, Set<String> children) {
}

@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(eagerGlobalOrdinals, relations, meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { eagerGlobalOrdinals, relations, meta };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ static class Builder extends FieldMapper.Builder {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { meta };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import java.io.IOException;
import java.time.ZoneId;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

Expand Down Expand Up @@ -273,8 +272,8 @@ Builder ignoreAbove(int ignoreAbove) {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] {
indexed,
hasDocValues,
stored,
Expand All @@ -294,8 +293,7 @@ protected List<Parameter<?>> getParameters() {
hiraganaQuaternaryMode,
ignoreAbove,
nullValue,
meta
);
meta };
}

private CollatorParams collatorParams() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -101,8 +100,8 @@ public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAna
}

@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] {
store,
indexOptions,
norms,
Expand All @@ -112,8 +111,7 @@ protected List<Parameter<?>> getParameters() {
analyzers.searchAnalyzer,
analyzers.searchQuoteAnalyzer,
analyzers.positionIncrementGap,
meta
);
meta };
}

private AnnotatedTextFieldType buildFieldType(FieldType fieldType, MapperBuilderContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.elasticsearch.search.lookup.SearchLookup;

import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

Expand Down Expand Up @@ -60,8 +59,8 @@ public Builder(String name) {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(stored, meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { stored, meta };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ private Builder() {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(enabled);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { enabled };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ public Builder(String name, boolean hasDocValues) {
}

@Override
public List<Parameter<?>> getParameters() {
return List.of(meta, stored, hasDocValues);
public Parameter<?>[] getParameters() {
return new Parameter<?>[] { meta, stored, hasDocValues };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.time.ZoneId;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Supplier;
Expand Down Expand Up @@ -111,8 +110,8 @@ public Builder(String name, ScriptCompiler scriptCompiler, Version indexCreatedV
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(meta, docValues, indexed, nullValue, stored, script, onScriptError);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { meta, docValues, indexed, nullValue, stored, script, onScriptError };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ private static void validateInputLength(int maxInputLength) {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(analyzer, searchAnalyzer, preserveSeparators, preservePosInc, maxInputLength, contexts, meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { analyzer, searchAnalyzer, preserveSeparators, preservePosInc, maxInputLength, contexts, meta };
}

NamedAnalyzer buildAnalyzer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ public Builder() {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(enabled);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { enabled };
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -308,8 +307,8 @@ private FieldValues<Long> scriptValues() {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(index, docValues, store, format, locale, nullValue, ignoreMalformed, script, onScriptError, meta);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { index, docValues, store, format, locale, nullValue, ignoreMalformed, script, onScriptError, meta };
}

private Long parseNullValue(DateFieldType fieldType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public abstract class FieldMapper extends Mapper implements Cloneable {
public static final Setting<Boolean> COERCE_SETTING = Setting.boolSetting("index.mapping.coerce", false, Property.IndexScope);

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(FieldMapper.class);
@SuppressWarnings("rawtypes")
static final Parameter<?>[] EMPTY_PARAMETERS = new Parameter[0];

protected final MappedFieldType mappedFieldType;
protected final MultiFields multiFields;
Expand Down Expand Up @@ -616,7 +618,7 @@ public Parameter(
* Returns the current value of the parameter
*/
public T getValue() {
return isSet ? value : defaultValue.get();
return isSet ? value : getDefaultValue();
}

@Override
Expand All @@ -640,7 +642,7 @@ public void setValue(T value) {
}

public boolean isConfigured() {
return isSet && Objects.equals(value, defaultValue.get()) == false;
return isSet && Objects.equals(value, getDefaultValue()) == false;
}

/**
Expand Down Expand Up @@ -773,8 +775,9 @@ private void merge(FieldMapper toMerge, Conflicts conflicts) {
}

protected void toXContent(XContentBuilder builder, boolean includeDefaults) throws IOException {
if (serializerCheck.check(includeDefaults, isConfigured(), get())) {
serializer.serialize(builder, name, getValue());
T value = getValue();
if (serializerCheck.check(includeDefaults, isConfigured(), value)) {
serializer.serialize(builder, name, value);
}
}

Expand Down Expand Up @@ -1159,7 +1162,7 @@ private void validate() {
/**
* @return the list of parameters defined for this mapper
*/
protected abstract List<Parameter<?>> getParameters();
protected abstract Parameter<?>[] getParameters();

@Override
public abstract FieldMapper build(MapperBuilderContext context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.elasticsearch.index.query.SearchExecutionContext;

import java.util.Collections;
import java.util.List;

/**
* A mapper that indexes the field names of a document under <code>_field_names</code>. This mapper is typically useful in order
Expand Down Expand Up @@ -83,8 +82,8 @@ static class Builder extends MetadataFieldMapper.Builder {
}

@Override
protected List<Parameter<?>> getParameters() {
return List.of(enabled);
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { enabled };
}

@Override
Expand Down

0 comments on commit 7a25453

Please sign in to comment.