Remove hppc from FetchSearchPhase #85188
Conversation
This commit clean up hppc in FetchSearchPhase class and all impacted classes. Relates elastic#84735
elasticsearchmachine
added
external-contributor
jtibshirani
added
the
:Search/Search
|
Pinging @elastic/es-search (Team:Search) |
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
There was a problem hiding this comment.
Thanks for the PR @OlivierCavadenti. I left a few requests.
| @@ -128,7 +127,7 @@ private void innerRun() throws Exception { | |||
| finishPhase.run(); | |||
| } else { | |||
| ScoreDoc[] scoreDocs = reducedQueryPhase.sortedTopDocs().scoreDocs(); | |||
| final IntArrayList[] docIdsToLoad = searchPhaseController.fillDocIdsToLoad(numShards, scoreDocs); | |||
| final ArrayList<Integer>[] docIdsToLoad = searchPhaseController.fillDocIdsToLoad(numShards, scoreDocs); | |||
There was a problem hiding this comment.
We should use List, not ArrayList, whenever passing these arround (ie in return types and local variables).
| this.contextId = contextId; | ||
| this.docIds = list.buffer; | ||
| this.size = list.size(); | ||
| this.docIds = docIds.stream().mapToInt(i -> i).toArray(); |
There was a problem hiding this comment.
This can use Integer::intValue in mapToInt
| this.docIds = list.buffer; | ||
| this.size = list.size(); | ||
| this.docIds = docIds.stream().mapToInt(i -> i).toArray(); | ||
| this.size = docIds.size(); |
There was a problem hiding this comment.
This size is no longer needed as a member, since now the array is sized correctly. Before the buffer from the hppc type could have been larger.
…archPhase # Conflicts: # server/src/main/java/org/elasticsearch/action/search/FetchSearchPhase.java # server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java # server/src/main/java/org/elasticsearch/action/search/SearchScrollQueryThenFetchAsyncAction.java
|
@elasticmachine ok to test |
There was a problem hiding this comment.
This looks good @OlivierCavadenti. Can you please address the compile failure I noted? Also, you'll want to sync with the latest upstream.
| public static IntArrayList[] fillDocIdsToLoad(int numShards, ScoreDoc[] shardDocs) { | ||
| IntArrayList[] docIdsToLoad = new IntArrayList[numShards]; | ||
| public static List<Integer>[] fillDocIdsToLoad(int numShards, ScoreDoc[] shardDocs) { | ||
| List<Integer>[] docIdsToLoad = (ArrayList<Integer>[]) new ArrayList[numShards]; |
There was a problem hiding this comment.
You'll need to annotate this to suppress the unchecked cast warning (and also just need List in the cast, not ArrayList).
@SuppressWarnings("unchecked")
List<Integer>[] docIdsToLoad = (List<Integer>[]) new ArrayList[numShards];
This commit clean up hppc in FetchSearchPhase class and all impacted classes.
Relates #84735