javanna commented on code in PR #12183:
URL: https://github.com/apache/lucene/pull/12183#discussion_r1328936157
##########
lucene/core/src/java/org/apache/lucene/search/TaskExecutor.java:
##########
@@ -64,4 +68,46 @@ final <T> List<T> invokeAll(Collection<RunnableFuture<T>>
tasks) throws IOExcept
}
return results;
}
+
+ /**
+ * Execute all the tasks provided as an argument concurrently only if it is
a {@link
+ * ThreadPoolExecutor} and the current thread invoking this is not a {@link
ThreadPoolExecutor}
+ * thread, else run all the tasks sequentially, wait for them to complete
and return the obtained
+ * results.
+ *
+ * @param tasks the tasks to execute
+ * @return a list containing the results from the tasks execution
+ * @param <T> the return type of the task execution
+ */
+ public final <T> List<T>
invokeAllWithThreadPoolExecutor(Collection<RunnableFuture<T>> tasks)
+ throws IOException {
+ boolean executeOnCallerThread =
+ StackWalker.getInstance(StackWalker.Option.SHOW_HIDDEN_FRAMES)
+ .walk(
+ (stream) ->
+ stream.anyMatch(
+ frame ->
+
frame.getClassName().contains(THREAD_POOL_EXECUTOR_WORKER_CLASS)
+ &&
frame.getMethodName().contains(RUN_METHOD)));
+ if (executor instanceof ThreadPoolExecutor && executeOnCallerThread ==
false) {
+ for (Runnable task : tasks) {
+ executor.execute(task);
+ }
+ } else {
+ for (Runnable task : tasks) {
+ task.run();
+ }
+ }
+ final List<T> results = new ArrayList<>();
+ for (Future<T> future : tasks) {
+ try {
+ results.add(future.get());
+ } catch (InterruptedException e) {
+ throw new ThreadInterruptedException(e);
+ } catch (ExecutionException e) {
+ throw IOUtils.rethrowAlways(e.getCause());
+ }
+ }
+ return results;
Review Comment:
This method still looks for specific usages of ThreadPoolExecutor. Could we
instead assume that whenever we are parallelizing, e.g. search a leaf slice, we
would no longer want to parallelize further as part of it? I would try to
detect this more generally. Also, I would incorporate that directly in the
existing invokeAll method, I don't see a reason why that should not be the
default and only behaviour.
I am wondering if there are other ways than using StackWalker, for instance
we could decorate the task to track which threads they are run into and disable
parallelization if invokeAll gets called from one of those threads that run has
been called against? I opened #12569 to showcase what I have in mind. Would be
great to get feedback about it.
--
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
