[Lldb-commits] [PATCH] D32757: Add TaskMap for iterating a function over a set of integers

[Scott Smith via Phabricator via lldb-commits]

scott.smith added a comment.

In https://reviews.llvm.org/D32757#743657, @clayborg wrote:

> In https://reviews.llvm.org/D32757#743567, @scott.smith wrote:
>
> > IMO, this is a simpler interface than TaskRunner.  Also, after this, there are no users of TaskRunner left.  Should I just delete them?
>
>
> I think you might not have understood TaskRunner's real benefits. It is smart in the way it runs things: it lets you run N things and get each item **as soon as it completes**. The TaskMap will serialize everything. So if you have 100 items to do and item 0 takes 200 seconds to complete, and items 1 - 99 take 1ms to complete, you will need to wait for task 0 to complete before you can start parsing the data. This will slow down the DWARF parser if you switch over to this. TaskRunner should not be deleted as it has a very specific purpose. Your TaskMap works fine for one case, but not in the other.


That may provide a benefit on machines with a few cores, but doesn't really help when you have 40+ cores.  Granted, the average laptop has 4 cores/8 hyperthreads.

>> I did this change to help parallel symbol demangling (to come in a separate patch).  Rather than have the symbol demangler use batching, etc, I thought it should be a higher level function.
> 
> Make sure this is a win before switching demangling over to using threads. I tried to improve performance on demangling before and I got worse performance on MacOS when we tried it because all of the malloc contention and threading overhead.

It is on my machine, but maybe not on other machines.  That would be unfortunate.  I'm guessing I shouldn't add go ahead and add jemalloc/tcmalloc to work around poor a MacOS malloc?  haha.


Repository:
  rL LLVM

https://reviews.llvm.org/D32757