Skip to content

update readMap to avoid resizing map during reading#84045

Merged
idegtiarenko merged 5 commits intoelastic:masterfrom
idegtiarenko:spike_presize_maps_onreading
Feb 22, 2022
Merged

update readMap to avoid resizing map during reading#84045
idegtiarenko merged 5 commits intoelastic:masterfrom
idegtiarenko:spike_presize_maps_onreading

Conversation

@idegtiarenko
Copy link
Copy Markdown
Contributor

@idegtiarenko idegtiarenko commented Feb 16, 2022

This commit updates readMap to use helper method that presizes map
correctly. It guarantees no resizing when adding given numbers of
entries to a hashmap.

@idegtiarenko
update readMap to avoid resizing map during reading
27f4cc3 
This commit updates readMap to use helper method that presizes map
correctly. It guarantees no resizing when adding given numbers of
entries to a hashmap.
@idegtiarenko idegtiarenko added >enhancement Team:Distributed Meta label for distributed team. v8.2.0 labels Feb 16, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Feb 16, 2022

Pinging @elastic/es-distributed (Team:Distributed)

@idegtiarenko idegtiarenko mentioned this pull request Feb 16, 2022
Merged
@DaveCTurner
Copy link
Copy Markdown
Member

DaveCTurner commented Feb 16, 2022

Could we do the same thing for readOrderedMap too?

Is there any trickery we can perform to assert that the map really doesn't get resized while we're reading it?

@arteam
Copy link
Copy Markdown
Contributor

arteam commented Feb 16, 2022

I believe I didn't integrate the factory method for creating sized LinkedHashMaps 9e6def3 which I should have done!

@idegtiarenko
Copy link
Copy Markdown
Contributor Author

idegtiarenko commented Feb 16, 2022

Could we do the same thing for readOrderedMap too?

Will update. Looks like it has only 6 usages.

@idegtiarenko
Copy link
Copy Markdown
Contributor Author

idegtiarenko commented Feb 16, 2022

Is there any trickery we can perform to assert that the map really doesn't get resized while we're reading it?

I am not sure. java.util.HashMap#capacity is not public and in recent jdks we could not easily make it accessible if we would like to access it via reflection.

@arteam
Copy link
Copy Markdown
Contributor

arteam commented Feb 17, 2022

Submitted #84054

@idegtiarenko idegtiarenko added the :Core/Infra/Core Core issues without another label label Feb 21, 2022
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Feb 21, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Feb 21, 2022

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Feb 21, 2022

Hi @idegtiarenko, I've created a changelog YAML for you.

DaveCTurner
DaveCTurner approved these changes Feb 22, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM, let's come back to the question about asserting no resizing later

@idegtiarenko idegtiarenko merged commit d229ecb into elastic:master Feb 22, 2022
@idegtiarenko idegtiarenko deleted the spike_presize_maps_onreading branch February 22, 2022 10:56
probakowski pushed a commit to probakowski/elasticsearch that referenced this pull request Feb 23, 2022
@idegtiarenko @probakowski
update readMap to avoid resizing map during reading (elastic#84045)
ec5e89d 
This commit updates readMap to use helper method that presizes map
correctly. It guarantees no resizing when adding given numbers of
entries to a hashmap / linkedhashmap.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaveCTurner DaveCTurner DaveCTurner approved these changes

@arteam arteam Awaiting requested review from arteam

Assignees

No one assigned

Labels

:Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team Team:Distributed Meta label for distributed team. v8.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@idegtiarenko @elasticmachine @DaveCTurner @arteam @elasticsearchmachine