Skip to content

Commit

Permalink
Fix TextFieldMapper Retaining a Reference to its Builder (#77251)
Browse files Browse the repository at this point in the history 
Fixes the text field mapper and the analyzers class that also retained parameter references that go really heavy.
Makes `TextFieldMapper` take hundreds of bytes compared to multiple kb per instance.

closes #73845
  • Loading branch information
@original-brownbear
original-brownbear authored Sep 3, 2021
1 parent c7130eb commit 096b8cc
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 45 deletions.
14 changes: 10 additions & 4 deletions .../mapper-extras/src/main/java/org/elasticsearch/index/mapper/MatchOnlyTextFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ public Builder(String name, IndexAnalyzers indexAnalyzers) {
public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAnalyzers) {
super(name);
this.indexCreatedVersion = indexCreatedVersion;
this.analyzers = new TextParams.Analyzers(indexAnalyzers, m -> ((MatchOnlyTextFieldMapper) m).analyzers);
this.analyzers = new TextParams.Analyzers(indexAnalyzers,
m -> ((MatchOnlyTextFieldMapper) m).indexAnalyzer,
m -> ((MatchOnlyTextFieldMapper) m).positionIncrementGap);
}

@Override
Expand Down Expand Up @@ -268,7 +270,9 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S
}

private final Version indexCreatedVersion;
private final TextParams.Analyzers analyzers;
private final IndexAnalyzers indexAnalyzers;
private final NamedAnalyzer indexAnalyzer;
private final int positionIncrementGap;
private final FieldType fieldType;

private MatchOnlyTextFieldMapper(
Expand All @@ -284,12 +288,14 @@ private MatchOnlyTextFieldMapper(
assert mappedFieldType.hasDocValues() == false;
this.fieldType = fieldType;
this.indexCreatedVersion = builder.indexCreatedVersion;
this.analyzers = builder.analyzers;
this.indexAnalyzers = builder.analyzers.indexAnalyzers;
this.indexAnalyzer = builder.analyzers.getIndexAnalyzer();
this.positionIncrementGap = builder.analyzers.positionIncrementGap.getValue();
}

@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), indexCreatedVersion, analyzers.indexAnalyzers).init(this);
return new Builder(simpleName(), indexCreatedVersion, indexAnalyzers).init(this);
}

@Override
Expand Down
6 changes: 5 additions & 1 deletion ...apper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,11 @@ public static class Builder extends FieldMapper.Builder {

public Builder(String name, IndexAnalyzers indexAnalyzers) {
super(name);
this.analyzers = new TextParams.Analyzers(indexAnalyzers, m -> builder(m).analyzers);
this.analyzers = new TextParams.Analyzers(
indexAnalyzers,
m -> builder(m).analyzers.getIndexAnalyzer(),
m -> builder(m).analyzers.positionIncrementGap.getValue()
);
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion .../src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ public static class Builder extends FieldMapper.Builder {

public Builder(String name, IndexAnalyzers indexAnalyzers) {
super(name);
this.analyzers = new TextParams.Analyzers(indexAnalyzers, m -> builder(m).analyzers);
this.analyzers = new TextParams.Analyzers(indexAnalyzers,
m -> builder(m).analyzers.getIndexAnalyzer(),
m -> builder(m).analyzers.positionIncrementGap.getValue());
}

@Override
Expand Down
100 changes: 64 additions & 36 deletions server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ public static class Defaults {
public static final int POSITION_INCREMENT_GAP = 100;
}

private static Builder builder(FieldMapper in) {
return ((TextFieldMapper) in).builder;
}

private static final class PrefixConfig implements ToXContent {
final int minChars;
final int maxChars;
Expand Down Expand Up @@ -229,27 +225,26 @@ public static class Builder extends FieldMapper.Builder {

private final Version indexCreatedVersion;

private final Parameter<Boolean> index = Parameter.indexParam(m -> builder(m).index.getValue(), true);
private final Parameter<Boolean> store = Parameter.storeParam(m -> builder(m).store.getValue(), false);
private final Parameter<Boolean> index = Parameter.indexParam(m -> ((TextFieldMapper) m).index, true);
private final Parameter<Boolean> store = Parameter.storeParam(m -> ((TextFieldMapper) m).store, false);

final Parameter<SimilarityProvider> similarity
= TextParams.similarity(m -> builder(m).similarity.getValue());
final Parameter<SimilarityProvider> similarity = TextParams.similarity(m -> ((TextFieldMapper) m).similarity);

final Parameter<String> indexOptions = TextParams.indexOptions(m -> builder(m).indexOptions.getValue());
final Parameter<Boolean> norms = TextParams.norms(true, m -> builder(m).norms.getValue());
final Parameter<String> termVectors = TextParams.termVectors(m -> builder(m).termVectors.getValue());
final Parameter<String> indexOptions = TextParams.indexOptions(m -> ((TextFieldMapper) m).indexOptions);
final Parameter<Boolean> norms = TextParams.norms(true, m -> ((TextFieldMapper) m).norms);
final Parameter<String> termVectors = TextParams.termVectors(m -> ((TextFieldMapper) m).termVectors);

final Parameter<Boolean> fieldData
= Parameter.boolParam("fielddata", true, m -> builder(m).fieldData.getValue(), false);
= Parameter.boolParam("fielddata", true, m -> ((TextFieldMapper) m).fieldData, false);
final Parameter<FielddataFrequencyFilter> freqFilter = new Parameter<>("fielddata_frequency_filter", true,
() -> DEFAULT_FILTER, TextFieldMapper::parseFrequencyFilter, m -> builder(m).freqFilter.getValue());
() -> DEFAULT_FILTER, TextFieldMapper::parseFrequencyFilter, m -> ((TextFieldMapper) m).freqFilter);
final Parameter<Boolean> eagerGlobalOrdinals
= Parameter.boolParam("eager_global_ordinals", true, m -> builder(m).eagerGlobalOrdinals.getValue(), false);
= Parameter.boolParam("eager_global_ordinals", true, m -> ((TextFieldMapper) m).eagerGlobalOrdinals, false);

final Parameter<Boolean> indexPhrases
= Parameter.boolParam("index_phrases", false, m -> builder(m).indexPhrases.getValue(), false);
= Parameter.boolParam("index_phrases", false, m -> ((TextFieldMapper) m).indexPhrases, false);
final Parameter<PrefixConfig> indexPrefixes = new Parameter<>("index_prefixes", false,
() -> null, TextFieldMapper::parsePrefixConfig, m -> builder(m).indexPrefixes.getValue()).acceptsNull();
() -> null, TextFieldMapper::parsePrefixConfig, m -> ((TextFieldMapper) m).indexPrefixes).acceptsNull();

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

Expand All @@ -262,7 +257,11 @@ public Builder(String name, IndexAnalyzers indexAnalyzers) {
public Builder(String name, Version indexCreatedVersion, IndexAnalyzers indexAnalyzers) {
super(name);
this.indexCreatedVersion = indexCreatedVersion;
this.analyzers = new TextParams.Analyzers(indexAnalyzers, m -> builder(m).analyzers);
this.analyzers = new TextParams.Analyzers(
indexAnalyzers,
m -> ((TextFieldMapper) m).indexAnalyzer,
m -> (((TextFieldMapper) m).positionIncrementGap)
);
}

public Builder index(boolean index) {
Expand Down Expand Up @@ -820,7 +819,21 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, S

}

private final Builder builder;
private final Version indexCreatedVersion;
private final boolean index;
private final boolean store;
private final String indexOptions;
private final boolean norms;
private final String termVectors;
private final SimilarityProvider similarity;
private final NamedAnalyzer indexAnalyzer;
private final IndexAnalyzers indexAnalyzers;
private final int positionIncrementGap;
private final boolean eagerGlobalOrdinals;
private final PrefixConfig indexPrefixes;
private final FielddataFrequencyFilter freqFilter;
private final boolean fieldData;
private final boolean indexPhrases;
private final FieldType fieldType;
private final SubFieldInfo prefixFieldInfo;
private final SubFieldInfo phraseFieldInfo;
Expand All @@ -840,12 +853,26 @@ protected TextFieldMapper(String simpleName, FieldType fieldType,
this.fieldType = fieldType;
this.prefixFieldInfo = prefixFieldInfo;
this.phraseFieldInfo = phraseFieldInfo;
this.builder = builder;
this.indexCreatedVersion = builder.indexCreatedVersion;
this.indexAnalyzer = builder.analyzers.getIndexAnalyzer();
this.indexAnalyzers = builder.analyzers.indexAnalyzers;
this.positionIncrementGap = builder.analyzers.positionIncrementGap.getValue();
this.index = builder.index.getValue();
this.store = builder.store.getValue();
this.similarity = builder.similarity.getValue();
this.indexOptions = builder.indexOptions.getValue();
this.norms = builder.norms.getValue();
this.termVectors = builder.termVectors.getValue();
this.eagerGlobalOrdinals = builder.eagerGlobalOrdinals.getValue();
this.indexPrefixes = builder.indexPrefixes.getValue();
this.freqFilter = builder.freqFilter.getValue();
this.fieldData = builder.fieldData.get();
this.indexPhrases = builder.indexPhrases.getValue();
}

@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), builder.indexCreatedVersion, builder.analyzers.indexAnalyzers).init(this);
return new Builder(simpleName(), indexCreatedVersion, indexAnalyzers).init(this);
}

@Override
Expand Down Expand Up @@ -1003,23 +1030,24 @@ protected void doXContentBody(XContentBuilder builder, Params params) throws IOE
// this is a pain, but we have to do this to maintain BWC
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
builder.field("type", contentType());
this.builder.index.toXContent(builder, includeDefaults);
this.builder.store.toXContent(builder, includeDefaults);
final Builder b = (Builder) getMergeBuilder();
b.index.toXContent(builder, includeDefaults);
b.store.toXContent(builder, includeDefaults);
this.multiFields.toXContent(builder, params);
this.copyTo.toXContent(builder, params);
this.builder.meta.toXContent(builder, includeDefaults);
this.builder.indexOptions.toXContent(builder, includeDefaults);
this.builder.termVectors.toXContent(builder, includeDefaults);
this.builder.norms.toXContent(builder, includeDefaults);
this.builder.analyzers.indexAnalyzer.toXContent(builder, includeDefaults);
this.builder.analyzers.searchAnalyzer.toXContent(builder, includeDefaults);
this.builder.analyzers.searchQuoteAnalyzer.toXContent(builder, includeDefaults);
this.builder.similarity.toXContent(builder, includeDefaults);
this.builder.eagerGlobalOrdinals.toXContent(builder, includeDefaults);
this.builder.analyzers.positionIncrementGap.toXContent(builder, includeDefaults);
this.builder.fieldData.toXContent(builder, includeDefaults);
this.builder.freqFilter.toXContent(builder, includeDefaults);
this.builder.indexPrefixes.toXContent(builder, includeDefaults);
this.builder.indexPhrases.toXContent(builder, includeDefaults);
b.meta.toXContent(builder, includeDefaults);
b.indexOptions.toXContent(builder, includeDefaults);
b.termVectors.toXContent(builder, includeDefaults);
b.norms.toXContent(builder, includeDefaults);
b.analyzers.indexAnalyzer.toXContent(builder, includeDefaults);
b.analyzers.searchAnalyzer.toXContent(builder, includeDefaults);
b.analyzers.searchQuoteAnalyzer.toXContent(builder, includeDefaults);
b.similarity.toXContent(builder, includeDefaults);
b.eagerGlobalOrdinals.toXContent(builder, includeDefaults);
b.analyzers.positionIncrementGap.toXContent(builder, includeDefaults);
b.fieldData.toXContent(builder, includeDefaults);
b.freqFilter.toXContent(builder, includeDefaults);
b.indexPrefixes.toXContent(builder, includeDefaults);
b.indexPhrases.toXContent(builder, includeDefaults);
}
}
7 changes: 4 additions & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/TextParams.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ public static final class Analyzers {
public final IndexAnalyzers indexAnalyzers;

public Analyzers(IndexAnalyzers indexAnalyzers,
Function<FieldMapper, Analyzers> analyzerInitFunction) {
Function<FieldMapper, NamedAnalyzer> analyzerInitFunction,
Function<FieldMapper, Integer> positionGapInitFunction) {
this.indexAnalyzer = Parameter.analyzerParam("analyzer", false,
m -> analyzerInitFunction.apply(m).indexAnalyzer.get(), indexAnalyzers::getDefaultIndexAnalyzer)
analyzerInitFunction, indexAnalyzers::getDefaultIndexAnalyzer)
.setSerializerCheck((id, ic, a) -> id || ic ||
Objects.equals(a, getSearchAnalyzer()) == false || Objects.equals(a, getSearchQuoteAnalyzer()) == false)
.addValidator(a -> a.checkAllowedInMode(AnalysisMode.INDEX_TIME));
Expand Down Expand Up @@ -68,7 +69,7 @@ public Analyzers(IndexAnalyzers indexAnalyzers,
})
.addValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME));
this.positionIncrementGap = Parameter.intParam("position_increment_gap", false,
m -> analyzerInitFunction.apply(m).positionIncrementGap.get(), TextFieldMapper.Defaults.POSITION_INCREMENT_GAP)
positionGapInitFunction, TextFieldMapper.Defaults.POSITION_INCREMENT_GAP)
.addValidator(v -> {
if (v < 0) {
throw new MapperParsingException("[position_increment_gap] must be positive, got [" + v + "]");
Expand Down

0 comments on commit 096b8cc

Please sign in to comment.