Skip to content

Fix TextFieldMapper Retaining a Reference to its Builder#77251

Merged
original-brownbear merged 1 commit intoelastic:masterfrom
original-brownbear:fix-text-field-mapper
Sep 3, 2021
Merged

Fix TextFieldMapper Retaining a Reference to its Builder#77251
original-brownbear merged 1 commit intoelastic:masterfrom
original-brownbear:fix-text-field-mapper

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

Fixes the text field mapper and the analyzers class that also retained parameter references that go really heavy.
Makes TextFieldMapper take ~200b for the text fields in the beats mapping, compared to ~5kb per instance before.

closes #73845

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
@original-brownbear original-brownbear added >bug :Search Foundations/Mapping Index mappings, including merging and defining field types v8.0.0 v7.16.0 labels Sep 3, 2021
builder.field("type", contentType());
this.builder.index.toXContent(builder, includeDefaults);
this.builder.store.toXContent(builder, includeDefaults);
final Builder b = (Builder) getMergeBuilder();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the nicest approach to this I guess, but it should create the correct serialization as far as I understand?

@original-brownbear original-brownbear marked this pull request as ready for review September 3, 2021 15:06
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Sep 3, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Copy Markdown
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@original-brownbear
Copy link
Copy Markdown
Contributor Author

Thanks Alan!

@original-brownbear original-brownbear merged commit 096b8cc into elastic:master Sep 3, 2021
@original-brownbear original-brownbear deleted the fix-text-field-mapper branch September 3, 2021 16:44
wjp719 added a commit to wjp719/elasticsearch that referenced this pull request Sep 4, 2021
* master: (128 commits)
  Mute DieWithDignityIT (elastic#77283)
  Fix randomization in MlNodeShutdownIT (elastic#77281)
  Add target_node_name for REPLACE shutdown type (elastic#77151)
  [DOCS] Adds information about version compatibility headers (elastic#77096)
  Fix template equals when mappings are wrapped (elastic#77008)
  Fix TextFieldMapper Retaining a Reference to its Builder (elastic#77251)
  Move die with dignity to be a test module (elastic#77136)
  Update task names for rest compatiblity (elastic#75267)
  [ML] adjusting bwc serialization for elastic#77256 (elastic#77257)
  Move `index.hidden` from Static to Dynamic settings (elastic#77218)
  Handle cgroups v2 in `OsProbe` (elastic#77128)
  Choose postings format from FieldMapper instead of MappedFieldType (elastic#77234)
  Add segment sorter for data streams (elastic#75195)
  Update skip after backport (elastic#77212)
  [ML] adding new defer_definition_decompression parameter to put trained model API (elastic#77189)
  [ML] Fix bug in inference stats persister for when feature reset is called
  Only check replicas in cancelling existing recoveries. (elastic#60564)
  Format `AbstractFilteringTestCase` (elastic#77217)
  [DOCS] Fixes line breaks. (elastic#77248)
  Convert 'routing' values in REST API tests to strings
  ...

# Conflicts:
#	server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java
original-brownbear added a commit that referenced this pull request Sep 4, 2021
…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
@original-brownbear original-brownbear restored the fix-text-field-mapper branch April 18, 2023 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v7.16.0 v8.0.0-alpha2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce memory footprint of TextFieldMapper

4 participants