javanna commented on code in PR #12606:
URL: https://github.com/apache/lucene/pull/12606#discussion_r1341307014
##########
lucene/core/src/java/org/apache/lucene/search/AbstractKnnVectorQuery.java:
##########
@@ -79,10 +79,11 @@ public Query rewrite(IndexSearcher indexSearcher) throws
IOException {
}
TaskExecutor taskExecutor = indexSearcher.getTaskExecutor();
- TopDocs[] perLeafResults =
- (taskExecutor == null)
- ? sequentialSearch(reader.leaves(), filterWeight)
- : parallelSearch(reader.leaves(), filterWeight, taskExecutor);
+ List<TaskExecutor.Task<TopDocs>> tasks = new ArrayList<>();
+ for (LeafReaderContext context : reader.leaves()) {
+ tasks.add(taskExecutor.createTask(() -> searchLeaf(context,
filterWeight)));
+ }
+ TopDocs[] perLeafResults =
taskExecutor.invokeAll(tasks).toArray(TopDocs[]::new);
Review Comment:
there is a trade-off here: we create multiple tasks even if we are not
parallelizing. That is good for simplicity, yet it makes the assumption that
concurrency is not a corner case, but rather not having an executor is. We
could make the task executor API more complex to factor in whether a real
executor is provided or not, but I am not sure that's a good trade-off. I'd
like to assume that we are evolving Lucene to make use of concurrency more and
more, and at some point having an executor to parallelize is the default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
