[Lldb-commits] [PATCH] D13727: Add task pool to LLDB

[Tamas Berghammer via lldb-commits]

tberghammer added a comment.

Zach: I thought about your idea and I think it won't make the code any simpler because your suggestion would only implement the functionality what is currently implemented by TaskPool and TaskPoolImpl what is using only a single condition variable and a single queue (+ a mutex what is needed in all implementation).

To implement the WaitForNextCompletedTask function we need some more logic what wouldn't be handled better by your approach either because the TreadPool is a global object wile WaitForNextCompletedTask should wait only for a task from a given list, so you still have to add at least 1 additional storage with the synchronization for it.

>From efficiency perspective, I don't think using std::async instead of a manually managed thread pool can make a big positive difference. The advantage of your solution would be that we don't have the worker threads running in the background when we don't use them, but on the other hand in some implementation of std::async we might have to start a new thread for each std::async (I expect something like this to happen on Linux) what isn't too cheap even when we know that we limited the number of threads to a low number. If having the worker thread running in the background is an issue then we can add it to my implementation with a smart heuristic about when we want to tear them down (e.g 100 ms after last task is done).


http://reviews.llvm.org/D13727