Cache DiscoveryNode#trimTier Result
#80179
Conversation
No need to recompute this over and over. This is the most expensive part of `FilterAllocationDecider` in benchmarking. Since we already cache the filter itself on the `IndexMetadata`, we can just cache the trimmed version also and `FilterAllocationDecider` effectively disappears from profiling. Also, made the filter immutable (which should as a side-effect also make it faster when iterating over it in the match method).
original-brownbear
added
>non-issue
:Distributed Coordination/Allocation
elasticmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
DaveCTurner
left a comment
DaveCTurner
left a comment
There was a problem hiding this comment.
Ok with me - I left a couple of nits.
| this.opType = opType; | ||
| this.filters = filters; | ||
| this.filters = Map.copyOf(filters); | ||
| this.trimmed = doTrim(this); |
There was a problem hiding this comment.
Nit: could we avoid even calling this when constructing the trimmed filters? Seems unnecessary to walk the filters twice just to check that trimming actually worked.
There was a problem hiding this comment.
Actually ... we can just avoid that walk altogether in 8 :) Missed that fact earlier, but we don't really need to walk the map in 8 because we're only filtering out a specific key (and null which I moved elsewhere but I think that filtering is unused ... just didn't want to change behavior here).
Simplified accordingly now
| private final OpType opType; | ||
|
|
||
| @Nullable | ||
| private final DiscoveryNodeFilters trimmed; |
There was a problem hiding this comment.
Naming nit: would be good to mention that it's the tier preference that was trimmed - withoutTierPreferences or similar perhaps?
There was a problem hiding this comment.
++ sounds good, renamed
|
Thanks David! |
4 participants
No need to recompute this over and over. This is the most expensive part of
FilterAllocationDeciderin benchmarking. Since we already cache the filter itselfon the
IndexMetadata, we can just cache the trimmed version also andFilterAllocationDecidereffectively disappears from profiling.Also, made the filter immutable (which should as a side-effect also make it faster
when iterating over it in the match method).
relates #77466