shubhamvishu commented on code in PR #12799:
URL: https://github.com/apache/lucene/pull/12799#discussion_r1396959558
##########
lucene/core/src/java/org/apache/lucene/util/hnsw/HnswConcurrentMergeBuilder.java:
##########
@@ -77,42 +75,17 @@ public OnHeapHnswGraph build(int maxOrd) throws IOException
{
HNSW_COMPONENT,
"build graph from " + maxOrd + " vectors, with " + workers.length +
" workers");
}
- List<Future<?>> futures = new ArrayList<>();
+ List<Callable<Void>> futures = new ArrayList<>();
for (int i = 0; i < workers.length; i++) {
int finalI = i;
futures.add(
- exec.submit(
- () -> {
- try {
- workers[finalI].run(maxOrd);
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- }));
- }
- Throwable exc = null;
- for (Future<?> future : futures) {
- try {
- future.get();
- } catch (InterruptedException e) {
- var newException = new ThreadInterruptedException(e);
- if (exc == null) {
- exc = newException;
- } else {
- exc.addSuppressed(newException);
- }
- } catch (ExecutionException e) {
- if (exc == null) {
- exc = e.getCause();
- } else {
- exc.addSuppressed(e.getCause());
- }
- }
- }
- if (exc != null) {
- // The error handling was copied from TaskExecutor. should we just use
TaskExecutor instead?
- throw IOUtils.rethrowAlways(exc);
+ () -> {
+ workers[finalI].run(maxOrd);
+ return null;
+ });
}
+ TaskExecutor taskExecutor = new TaskExecutor(exec);
Review Comment:
I think with this we are only creating 1 task executor instance per knn
field because the concurrent graph building happens in
`HnswConcurrentMergeBuilder#build`(where we make use of concurrency) so moving
the task executor creation above in the hierarchy(to `ConcurrentHnswMerger`)
shouldn't make any difference as we would still be creating 1 instance per knn
field or maybe I'm wrong?
I'm happy to change it if this is not the case or if you feel thats still
better approach to pass the task executor from above instead of executor
service?
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
