kaivalnp commented on code in PR #16778:
URL: https://github.com/apache/lucene/pull/16778#discussion_r4342548291


##########
lucene/core/src/java/org/apache/lucene/util/hnsw/HnswConcurrentMergeBuilder.java:
##########
@@ -132,16 +133,46 @@ public OnHeapHnswGraph build(int maxOrd) throws 
IOException {
                 (rebalanceEndNs - rebalanceStartNs) / 1_000_000.0));
       }
     }
-    List<Callable<Void>> futures = new ArrayList<>();
-    for (int i = 0; i < workers.length; i++) {
+
+    // Reserve first worker for calling thread: only create Futures for 
remaining workers
+    List<Callable<Void>> tasks = new ArrayList<>(workers.length - 1);
+    for (int i = 1; i < workers.length; i++) {
       int finalI = i;
-      futures.add(
+      tasks.add(
           () -> {
             workers[finalI].run(maxOrd);
             return null;
           });
     }
-    taskExecutor.invokeAll(futures);
+
+    final List<Future<Void>> started = new ArrayList<>(workers.length);
+    try {
+      int start = workers[0].getStartPos(maxOrd);
+      int end;
+      while (start != -1) {
+        // Use this thread for the first worker
+        end = start + workers[0].batchSize;
+        if (end >= maxOrd) {
+          end = maxOrd; // No need to recruit more workers if this is the last 
batch
+        } else {
+          // Recruit other threads for workers that haven't started yet
+          while (started.size() < tasks.size()) {
+            var maybeFuture = 
taskExecutor.tryInvoke(tasks.get(started.size()));

Review Comment:
   Should we also 
"[cancel](https://github.com/apache/lucene/blob/330ee57b167ef0be2935cfeac00f532297a55f97/lucene/core/src/java/org/apache/lucene/search/TaskExecutor.java#L182)"
 all jobs if any of them fail?



##########
lucene/core/src/java/org/apache/lucene/search/TaskExecutor.java:
##########
@@ -115,7 +116,46 @@ public <T> List<T> invokeAll(Collection<Callable<T>> 
callables) throws IOExcepti
     return collectResults(futures);
   }
 
-  private static <T> List<T> collectResults(List<RunnableFuture<T>> futures) 
throws IOException {
+  /// Execute `callable` on the backing [Executor], but only if it runs on 
another thread.

Review Comment:
   nit: `/** ... */` for javadocs?



-- 
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]

Reply via email to