[Lldb-commits] [PATCH] D33246: Remove most of lldb's TaskPool in favor of llvm's parallel functions

[Scott Smith via Phabricator via lldb-commits]

scott.smith added inline comments.


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1995-1996
     //----------------------------------------------------------------------
-    TaskMapOverInt(0, num_compile_units, extract_fn);
+    llvm::parallel::for_each_n(llvm::parallel::par, 0U, num_compile_units,
+                               extract_fn);
 
----------------
scott.smith wrote:
> zturner wrote:
> > What did you decide about the recursive parallelism?  I don't know if that works yet using LLVM's default executor.
> 1. This code doesn't care.
> 2. It looks like it works, since (I think) for_each creates a separate TaskGroup for each call.
> 3. However I got a deadlock when using this for parallelizing the dynamic library loading itself, which used to work.  That could either be due to other code changes, some oversight on my part, or it could be that for_each_n doesn't actually support recursion - which means that I misunderstood for_each_n.  So I have more work to do...
On further inspection, llvm::parallel does not support recursion, since TaskGroup uses a single static Executor, and provides no way to override that (and besides, there's no way to pass parameters from for_each_n to the TaskGroup).  That's fixable though, by making the default executor a thread local variable, so that worker threads can enqueue work to a different executor.


Repository:
  rL LLVM

https://reviews.llvm.org/D33246