Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

update readMap to avoid resizing map during reading #84045

Merged
merged 5 commits into from
Feb 22, 2022
Merged

update readMap to avoid resizing map during reading #84045

merged 5 commits into from
Feb 22, 2022

Conversation

idegtiarenko
Copy link
Contributor

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 (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.2.0 labels Feb 16, 2022
@elasticmachine
Copy link
Collaborator

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

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

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
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
Contributor Author

Could we do the same thing for readOrderedMap too?

Will update. Looks like it has only 6 usages.

@idegtiarenko
Copy link
Contributor Author

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
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
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

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

@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 (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. 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