Cheaper handling of skipped shard iterators in AbstractSearchAsyncAction #124223
Conversation
original-brownbear
added
>non-issue
:Search Foundations/Search
elasticsearchmachine
added
the
Team:Search Foundations
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
| this.shardsIts = iterators; | ||
| outstandingShards = new AtomicInteger(shardsIts.size()); | ||
| outstandingShards = new AtomicInteger(shardsIts.size() - skipped); |
There was a problem hiding this comment.
Here it's slightly confusing which shardsIts it is - the parameter shardsIts as I see contains the skipped ones, but this.shardsIts does not. Maybe use a different name for the parameter? Also, isn't it iterators.size()?
There was a problem hiding this comment.
To think about it, do we really need iterators at all - can't we just work on this.shardsIts directly?
There was a problem hiding this comment.
It's just a little faster to have a dedicated local variable instead of looking up the field repeatedly, that's all :) There's no other reason to have a separate local variable.
But you're right the naming is horrific :O lets fix that in a follow-up but at least use iterators.size() here :) My idea/hope was actually to get rid of this.shardIts, it's somewhat unnecessary in the grand scheme of things :)
|
Thanks Stas! |
original-brownbear
added
the
auto-backport
💔 Backport failed
You can use sqren/backport to manually backport by running |
…ion (elastic#124223) No reason to blow up the size of `AbstractSearchAsyncAction` (and the code size of some methods that really only need the size of that collection) needlessly. Just keep the count, that's all we need. We can build the skipped shard list inline if need be (and do so in a cheaper way because we can build the search targets.
…ion (elastic#124223) No reason to blow up the size of `AbstractSearchAsyncAction` (and the code size of some methods that really only need the size of that collection) needlessly. Just keep the count, that's all we need. We can build the skipped shard list inline if need be (and do so in a cheaper way because we can build the search targets.
…ion (elastic#124223) No reason to blow up the size of `AbstractSearchAsyncAction` (and the code size of some methods that really only need the size of that collection) needlessly. Just keep the count, that's all we need. We can build the skipped shard list inline if need be (and do so in a cheaper way because we can build the search targets.
…ion (#124223) (#126533) No reason to blow up the size of `AbstractSearchAsyncAction` (and the code size of some methods that really only need the size of that collection) needlessly. Just keep the count, that's all we need. We can build the skipped shard list inline if need be (and do so in a cheaper way because we can build the search targets.
No reason to blow up the size of
AbstractSearchAsyncAction(and the code size of some methods that really only need the size of that collection) needlessly. Just keep the count, that's all we need.We can build the skipped shard list inline if need be (and do so in a cheaper way because we can build the search targets.