Skip to content

Remove hppc from LocalCheckpointTracker#86073

Merged
rjernst merged 3 commits intoelastic:masterfrom
rjernst:hppc/checkpoint_tracker
Apr 25, 2022
Merged

Remove hppc from LocalCheckpointTracker#86073
rjernst merged 3 commits intoelastic:masterfrom
rjernst:hppc/checkpoint_tracker

Conversation

@rjernst
Copy link
Copy Markdown
Member

@rjernst rjernst commented Apr 21, 2022

The LocalCheckpointTracker keeps mappings between sequence number and a
bitsets, using hppc primitive maps. This commit converts these to use
standard HashMaps.

relates #84735

The LocalCheckpointTracker keeps mappings between sequence number and a
bitsets, using hppc primitive maps. This commit converts these to use
standard HashMaps.

relates elastic#84735
@rjernst rjernst added :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >refactoring v8.3.0 labels Apr 21, 2022
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Apr 21, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@rjernst rjernst mentioned this pull request Apr 21, 2022
43 tasks
Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM after some digging :)

I looked through some profiling of this code and think this is an ok change to make.
The code here is obviously not entirely trivial performance-wise, but boxing a long and some overhead for the larger map seems like a rather trivial cost increase in a method that costs ~<0.25% of an indexing operation to begin with. (somewhat stable number from a couple of profiles I examined).
If we actually find that we want to optimize this, we could e.g. use io.netty.util.collection.LongObjectHashMap or so easily but I don't believe it matters enough to pull in that kind of dependency here. But there's so so many things more expensive than this path in indexing to begin with that this really shouldn't be our focus IMO.

@rjernst rjernst merged commit e21303d into elastic:master Apr 25, 2022
@rjernst rjernst deleted the hppc/checkpoint_tracker branch April 25, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >refactoring Team:Distributed Meta label for distributed team. v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants