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

[Zachary Turner via lldb-commits]

zturner added a comment.

In http://reviews.llvm.org/D13727#268086, @clayborg wrote:

> > Yes.  But an implementation of std::async is either going to use a global thread pool or it's not.  If it does there's no problem, and if it doesn't, it's going to create one thread for each call, and when the work in that thread is done, the thread will exit.  You wouldn't have a situation where you call std::async 100 times, join on all 100 results, and the 100 threads are still sitting around.  After the work is done the threads would go away.  So by dividing the work into no more than `hardware_concurrency` chunks and sending those chunks to separate calls invocations of `std::async`, you're safe.
>
>
> Performance will be worse if there is a thread created for each task as thread creation is very expensive and can often be slower than just doing things serially for small tasks.
>
> > Having a global thread pool is fine, but if that's what we want to do I think it needs to be a much smaller value than `hardware_concurrency`.  Because having that many threads sitting around doing nothing all the time makes debugging LLDB a real chore (not to mention being wasteful).
>
>
> I don't agree. They are parked doing nothing taking up no real resources, or they are doing something useful. When tasks come in, the parked threads are ready to go, no spin up time like when using std::async(). Debugging LLDB already has many threads and on MacOSX there are already some libdispatch threads around and they don't add much noise IMHO. We could actually back this new approach with libdispatch on MacOSX, but I would want to verify that the performance is as good or better than the current approach before doing so. The benefit of libdispatch is it will tear down worker threads if they haven't been used in a while. But we could do the same with our approach.
>
> If you want to optimize windows with std::async() I would say go for it. Maybe unit tests can run using multiple ways (std::async, TaskPool, libdispatch) and then recommend a default for your platform based on real performance tests. Demangling a bunch of names is one example that could be used for performance testing this.


I agree having a few threads around isn't a big deal, but 48-60 is a really high amount of noise when you debug LLDB and get a list of threads.  I'm kind of leaning back towards just making this use `std::async` on Windows because having 60-70 threads in my thread list every time I start up LLDB is going to be a big problem.  At the same time, even if I had only 32 cores, or 16 cores, I don't think I would want LLDB trying to consume 100% of my CPU *ever* because it makes the machine unusable even if only for a short period of time.  If I'm trying to load 2 GB of debug information (not a hypothetical situation) this is going to be a big problem.

We could try heuristics but I feel like this becomes a lot easier and less error prone by just letting the system manage the thread pool wherever possible.

Tamas: I know you mentioned earlier that it's possible to have 2 different implementations, one that delegates to std::async and one that uses this approach.  Looking over the code it looks like only a couple lines of code need to be different.  If it's that straightforward can we just do that?  This would also give Greg and others on FreeBSD or somewhere else to try flipping the switch on their own platform and seeing which gives better results.


http://reviews.llvm.org/D13727