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

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.


================
Comment at: include/lldb/Utility/TaskPool.h:18-20
+  std::function<void()> cbs[sizeof...(T)]{tasks...};
+  llvm::parallel::for_each_n(llvm::parallel::par, static_cast<size_t>(0),
+                             sizeof...(T), [&cbs](size_t idx) { cbs[idx](); });
----------------
I'm not sure this is the most efficient implementation.  `std::function` has pretty poor performance, and there might be no need to even convert everything to `std::function` to begin with.  You could make this a bit better by using `llvm::function_ref<void()>` instead.

That said, I wonder if it's worth adding a function like this to `llvm::TaskGroup`?  And you could just enqueue all the tasks, rather than `for_each_n`.  Not sure if there would be a different in practice, what do you think?


================
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);
 
----------------
What did you decide about the recursive parallelism?  I don't know if that works yet using LLVM's default executor.


Repository:
  rL LLVM

https://reviews.llvm.org/D33246