Skip to content

Commit

Permalink
Fix TextFieldMapper Retaining a Reference to its Builder (#77251) (#7…
Browse files Browse the repository at this point in the history 
…7268)

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 4, 2021
1 parent 3bd96b8 commit 61175bf
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 46 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 @@ -87,7 +87,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
103 changes: 66 additions & 37 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<Float> boost = Parameter.boostParam();
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
Expand All @@ -263,7 +258,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 @@ -825,7 +824,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 @@ -845,12 +858,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 @@ -1008,24 +1035,26 @@ 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.boost.toXContent(builder, includeDefaults);
this.builder.index.toXContent(builder, includeDefaults);
this.builder.store.toXContent(builder, includeDefaults);

final Builder b = (Builder) getMergeBuilder();
b.boost.toXContent(builder, includeDefaults);
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 61175bf

Please sign in to comment.