[PATCH] D123225: [ThreadPool] add ability to group tasks into separate groups

[Luboš Luňák via Phabricator via llvm-commits]

llunak marked 10 inline comments as done.
llunak added inline comments.


================
Comment at: llvm/lib/Support/ThreadPool.cpp:155
+  // possible deadlock.
+  processTasks(&Group);
 }
----------------
aganea wrote:
> One more thing perhaps: tasks `.wait()`-ing will be "suspended" by re-entering `processTasks`. Any remaining tasks in the queue will be scheduled randomly over the waiting task(s), and this could easily create stack-overflows, since the scheduling is non-deterministic (so we could have several tasks waiting, piled on the top of each other). Depending on the stack size per platform, and how much stack each task eats up, this could potentially cause random crashes in production.
> 
> Probably the less intrusive approach would be to use fibers for suspended tasks. I suppose we could do that as a secondary step after this patch.
Yes, but this is going to cost some resource no matter what, so it's just a choice of what resource it will be. And note that the recursion will be limited to the number of groups, since waiting loops are not allowed.



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

https://reviews.llvm.org/D123225