[PATCH] D61115: Parallel: only allow the first TaskGroup to run tasks parallelly

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 25 01:05:46 PDT 2019


MaskRay added a comment.

> Generally, it doesn't make much sense to have more than one thread pool in a process

The existing `Executor::getDefaultExecutor()` implements the thread pool. I agree that one thread pool suffices, so we parallelize the outermost loop and serialize inner loops.

Note a thread pool doesn't solve the problem. `Latch::sync()` makes one thread block (before the function returns there must be a synchronization point). (This can be fixed by introducing a thread scheduler but it is unnecessary for our use cases)

> Once we implement the thread pool, then we can implement parallel-for-each like this:

`TaskGroup` encapsulates the common code that `parallel_sort`, `parallel_for_each` and `parallel_for_each_n` need: 1) spawns a task 2) synchronization point.
If we had only `parallel_for_each` but not the other `parallel_*` variants, I would agree that inlining `TaskGroup` would be good.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61115/new/

https://reviews.llvm.org/D61115





More information about the llvm-commits mailing list