Avoid some memory allocations in the ThreadPool

[• espindola]

Summary

I was surprised to see std::function related allocations showing up it a profile of lld. In the case of lld the allocations are tiny and don't really impact performance.

I am not sure if we should be trying to improve this code as it probably needs a full rewrite for better performance, but I was curious how hard it would be to remove some of the allocations.

Note that this only updated one of the executors so far.

[dblaikie]

If no one else is using the 'add' API - could simplify it to take a function pointer, if the goal is to avoid situations that might cause allocation overhead?

[dblaikie]

lib/Support/Parallel.cpp

94

This should probably take Latch by reference. I know lld has a bit of a different coding convention there - but it's not usually done that way in LLVM (well, it's perhaps a bit all over the shop, to be fair).

lib/Support/Parallel.cpp

 using namespace llvm;
 
 namespace {
 
 /// \brief An abstract class that takes closures and runs them asynchronously.
 class Executor {
 public:
   virtual ~Executor() = default;
-  virtual void add(std::function<void()> func) = 0;
+  virtual void add(std::function<void()> F, parallel::detail::Latch *L) = 0;
 
   static Executor *getDefaultExecutor();
 };
 
 #if !LLVM_ENABLE_THREADS
 class SyncExecutor : public Executor {
 public:
   virtual void add(std::function<void()> F) { F(); }
public:
   ~ThreadPoolExecutor() override {
     std::unique_lock<std::mutex> Lock(Mutex);
     Stop = true;
     Lock.unlock();
     Cond.notify_all();
     // Wait for ~Latch.
   }
 
-  void add(std::function<void()> F) override {
+  void add(std::function<void()> F, parallel::detail::Latch *L) override {
     std::unique_lock<std::mutex> Lock(Mutex);
-    WorkStack.push(F);
+    WorkStack.push({std::move(F), L});
     Lock.unlock();
     Cond.notify_one();
   }
 
 private:
   void work() {
     while (true) {
       std::unique_lock<std::mutex> Lock(Mutex);
       Cond.wait(Lock, [&] { return Stop || !WorkStack.empty(); });
       if (Stop)
         break;
-      auto Task = WorkStack.top();
+      auto Task = std::move(WorkStack.top());
       WorkStack.pop();
       Lock.unlock();
-      Task();
+      Task.first();
+      Task.second->dec();
     }
     Done.dec();
   }
 
   std::atomic<bool> Stop{false};
-  std::stack<std::function<void()>> WorkStack;
+  std::stack<std::pair<std::function<void()>, parallel::detail::Latch *>>
+      WorkStack;
   std::mutex Mutex;
   std::condition_variable Cond;
   parallel::detail::Latch Done;
 };
 
 Executor *Executor::getDefaultExecutor() {
   static ThreadPoolExecutor exec;
   return &exec;
 }
 #endif
 }
 
 #if LLVM_ENABLE_THREADS
 void parallel::detail::TaskGroup::spawn(std::function<void()> F) {
   L.inc();
-  Executor::getDefaultExecutor()->add([&, F] {
-    F();
-    L.dec();
-  });
+  Executor::getDefaultExecutor()->add(std::move(F), &L);
 }
 #endif