[PATCH] D115078: Split the locking of the queue and the threads vector in the ThreadPool implementation

[Benoit Jacob via Phabricator via llvm-commits]

Benoit accepted this revision.
Benoit added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/include/llvm/Support/ThreadPool.h:45
+      : Strategy(S), MaxThreadCount(S.compute_thread_count()) {
+    Threads.reserve(MaxThreadCount);
+  }
----------------
is the performance benefit of avoiding some small mallocs worth the line of code?


================
Comment at: llvm/lib/Support/ThreadPool.cpp:27
     return; // Already hit the max thread pool size.
-  if (ActiveThreads + Tasks.size() <= Threads.size())
-    return; // We have enough threads for now.
-  int ThreadID = Threads.size();
-  Threads.emplace_back([this, ThreadID] {
-    Strategy.apply_thread_strategy(ThreadID);
-    while (true) {
-      std::function<void()> Task;
-      {
-        std::unique_lock<std::mutex> LockGuard(QueueLock);
-        // Wait for tasks to be pushed in the queue
-        QueueCondition.wait(LockGuard,
-                            [&] { return !EnableFlag || !Tasks.empty(); });
-        // Exit condition
-        if (!EnableFlag && Tasks.empty())
-          return;
-        // Yeah, we have a task, grab it and release the lock on the queue
+  requested = std::min<int>(requested, MaxThreadCount);
+  while (requested > static_cast<int>(Threads.size())) {
----------------
how about using a new variable instead of mutating `requested`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115078/new/

https://reviews.llvm.org/D115078