From 98a6a9ac1452bfb146d97a004d614b520586129d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 3 Sep 2021 18:44:11 +0200 Subject: [PATCH] Fix TextFieldMapper Retaining a Reference to its Builder (#77251) 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 --- .../mapper/MatchOnlyTextFieldMapper.java | 14 ++- .../mapper/SearchAsYouTypeFieldMapper.java | 6 +- .../AnnotatedTextFieldMapper.java | 4 +- .../index/mapper/TextFieldMapper.java | 103 +++++++++++------- .../index/mapper/TextParams.java | 7 +- 5 files changed, 88 insertions(+), 46 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/MatchOnlyTextFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/MatchOnlyTextFieldMapper.java index a4805b318f21b..6792793820c2a 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/MatchOnlyTextFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/MatchOnlyTextFieldMapper.java @@ -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 @@ -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( @@ -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 diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java index c78cdc673f311..987f229c65ee5 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java @@ -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 diff --git a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java index f322d1314ae7b..21e2194201dc3 100644 --- a/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java +++ b/plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java @@ -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 diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java index 7683e4648309f..c1bb50708d5f0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java @@ -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; @@ -229,27 +225,26 @@ public static class Builder extends FieldMapper.Builder { private final Version indexCreatedVersion; - private final Parameter index = Parameter.indexParam(m -> builder(m).index.getValue(), true); - private final Parameter store = Parameter.storeParam(m -> builder(m).store.getValue(), false); + private final Parameter index = Parameter.indexParam(m -> ((TextFieldMapper) m).index, true); + private final Parameter store = Parameter.storeParam(m -> ((TextFieldMapper) m).store, false); - final Parameter similarity - = TextParams.similarity(m -> builder(m).similarity.getValue()); + final Parameter similarity = TextParams.similarity(m -> ((TextFieldMapper) m).similarity); - final Parameter indexOptions = TextParams.indexOptions(m -> builder(m).indexOptions.getValue()); - final Parameter norms = TextParams.norms(true, m -> builder(m).norms.getValue()); - final Parameter termVectors = TextParams.termVectors(m -> builder(m).termVectors.getValue()); + final Parameter indexOptions = TextParams.indexOptions(m -> ((TextFieldMapper) m).indexOptions); + final Parameter norms = TextParams.norms(true, m -> ((TextFieldMapper) m).norms); + final Parameter termVectors = TextParams.termVectors(m -> ((TextFieldMapper) m).termVectors); final Parameter fieldData - = Parameter.boolParam("fielddata", true, m -> builder(m).fieldData.getValue(), false); + = Parameter.boolParam("fielddata", true, m -> ((TextFieldMapper) m).fieldData, false); final Parameter 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 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 indexPhrases - = Parameter.boolParam("index_phrases", false, m -> builder(m).indexPhrases.getValue(), false); + = Parameter.boolParam("index_phrases", false, m -> ((TextFieldMapper) m).indexPhrases, false); final Parameter 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 boost = Parameter.boostParam(); private final Parameter> meta = Parameter.metaParam(); @@ -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) { @@ -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; @@ -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 @@ -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); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java index 3061c67cf694c..91e1b16c22aed 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TextParams.java @@ -36,9 +36,10 @@ public static final class Analyzers { public final IndexAnalyzers indexAnalyzers; public Analyzers(IndexAnalyzers indexAnalyzers, - Function analyzerInitFunction) { + Function analyzerInitFunction, + Function 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)); @@ -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 + "]");