Make shardIndexMap in AbstractSearchAsyncAction a local variable #117168
Conversation
We only need this rather large map in `run`, lets create it on the fly there to save the rather large redundant field and save on state and complexity in general.
original-brownbear
added
>non-issue
:Search Foundations/Search
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
elasticsearchmachine
added
Team:Search Foundations
| final Map<SearchShardIterator, Integer> shardIndexMap = Maps.newHashMapWithExpectedSize(shardIterators.length); | ||
| for (int i = 0; i < shardIterators.length; i++) { | ||
| shardIndexMap.put(shardIterators[i], i); | ||
| } |
There was a problem hiding this comment.
This is fine because run is always every called once on a specific async action instance?
There was a problem hiding this comment.
Yea, the results array among other things is pretty much single-use :)
| } | ||
| this.shardIndexMap = Collections.unmodifiableMap(shardMap); | ||
| this.shardIterators = searchIterators.toArray(SearchShardIterator[]::new); | ||
| Arrays.sort(shardIterators); |
There was a problem hiding this comment.
can we remove the map while leaving the rest as it was?
There was a problem hiding this comment.
Sure I was just annoyed by the needless extra copy and figured it's not worth an individual PR? :)
There was a problem hiding this comment.
is timSort vs Arrays.sort on purpose?
There was a problem hiding this comment.
I don't think we have a timSort for arrays in Lucene? + the Lucene timSort seems to be somewhat obsolete now anyway potentially with List#sort being a thing in recent Java versions and likely more efficient?
-> results are unchanged and this seems to be the canonical solution now :)?
…stic#117168) We only need this rather large map in `run`, lets create it on the fly there to save the rather large redundant field and save on state and complexity in general.
…stic#117168) (elastic#121408) We only need this rather large map in `run`, lets create it on the fly there to save the rather large redundant field and save on state and complexity in general.
We only need this rather large map in
run, lets create it on the fly there to save the rather large redundant field and save on state and complexity in general. Also, this saves one round of copying the shard list which is always nice for large numbers of shards.