[lldb] Minor fixes in TaskPool

[alexander-shaposhnikov]

Summary

1. Move everything into the namespace lldb_private
2. Add missing std::move in TaskPoolImpl::Worker
3. std::thread::hardware_concurrency may return 0, handle this case correctly

[alexander-shaposhnikov]

fix the tests

[labath]

Looks good, thanks.

[fjricci]

source/Host/common/TaskPool.cpp

55 ↗

(On Diff #124661)

Is 1 the best default here when hardware_concurrency() isn't computable? Seems like it could have some big performance implications, and I'm not sure how common it is for people to debug on host machines that only support one thread.

lldb/trunk/include/lldb/Host/TaskPool.h

 #include "llvm/ADT/STLExtras.h"
 #include <functional> // for bind, function
 #include <future>
 #include <list>
 #include <memory>      // for make_shared
 #include <mutex>       // for mutex, unique_lock, condition_variable
 #include <type_traits> // for forward, result_of, move
 
+namespace lldb_private {
+
 // Global TaskPool class for running tasks in parallel on a set of worker thread
 // created the first
 // time the task pool is used. The TaskPool provide no guarantee about the order
 // the task will be run
 // and about what tasks will run in parallel. None of the task added to the task
 // pool should block
 // on something (mutex, future, condition variable) what will be set only by the
 // completion of an

 };
 
 // Run 'func' on every value from begin .. end-1.  Each worker will grab
 // 'batch_size' numbers at a time to work on, so for very fast functions, batch
 // should be large enough to avoid too much cache line contention.
 void TaskMapOverInt(size_t begin, size_t end,
                     const llvm::function_ref<void(size_t)> &func);
 
+unsigned GetHardwareConcurrencyHint();
+
+} // namespace lldb_private
+
 #endif // #ifndef utility_TaskPool_h_

lldb/trunk/source/Host/common/TaskPool.cpp

 //===--------------------- TaskPool.cpp -------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/TaskPool.h"
 #include "lldb/Host/ThreadLauncher.h"
 
 #include <cstdint> // for uint32_t
 #include <queue>   // for queue
 #include <thread>  // for thread
 
+namespace lldb_private {
+
 namespace {
 class TaskPoolImpl {
 public:
   static TaskPoolImpl &GetInstance();
 
   void AddTask(std::function<void()> &&task_fn);
 
 private:

 }
 
 void TaskPool::AddTaskImpl(std::function<void()> &&task_fn) {
   TaskPoolImpl::GetInstance().AddTask(std::move(task_fn));
 }
 
 TaskPoolImpl::TaskPoolImpl() : m_thread_count(0) {}
 
+unsigned GetHardwareConcurrencyHint() {
+  // std::thread::hardware_concurrency may return 0
+  // if the value is not well defined or not computable.
+  static const unsigned g_hardware_concurrency = 
+    std::max(1u, std::thread::hardware_concurrency());
+  return g_hardware_concurrency;
+}
+
 void TaskPoolImpl::AddTask(std::function<void()> &&task_fn) {
-  static const uint32_t max_threads = std::thread::hardware_concurrency();
   const size_t min_stack_size = 8 * 1024 * 1024;
 
   std::unique_lock<std::mutex> lock(m_tasks_mutex);
   m_tasks.emplace(std::move(task_fn));
-  if (m_thread_count < max_threads) {
+  if (m_thread_count < GetHardwareConcurrencyHint()) {
     m_thread_count++;
     // Note that this detach call needs to happen with the m_tasks_mutex held.
     // This prevents the thread
     // from exiting prematurely and triggering a linux libc bug
     // (https://sourceware.org/bugzilla/show_bug.cgi?id=19951).
     lldb_private::ThreadLauncher::LaunchThread("task-pool.worker", WorkerPtr,
                                                this, nullptr, min_stack_size)
         .Release();
   }
 }
 
 lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
   Worker((TaskPoolImpl *)pool);
   return 0;
 }
 
 void TaskPoolImpl::Worker(TaskPoolImpl *pool) {
   while (true) {
     std::unique_lock<std::mutex> lock(pool->m_tasks_mutex);
     if (pool->m_tasks.empty()) {
       pool->m_thread_count--;
       break;
     }
 
-    std::function<void()> f = pool->m_tasks.front();
+    std::function<void()> f = std::move(pool->m_tasks.front());
     pool->m_tasks.pop();
     lock.unlock();
 
     f();
   }
 }
 
 void TaskMapOverInt(size_t begin, size_t end,
                     const llvm::function_ref<void(size_t)> &func) {
+  const size_t num_workers = std::min<size_t>(end, GetHardwareConcurrencyHint());
   std::atomic<size_t> idx{begin};
-  size_t num_workers =
-      std::min<size_t>(end, std::thread::hardware_concurrency());
   
   auto wrapper = [&idx, end, &func]() {
     while (true) {
       size_t i = idx.fetch_add(1);
       if (i >= end)
         break;
       func(i);
     }
   };
 
   std::vector<std::future<void>> futures;
   futures.reserve(num_workers);
   for (size_t i = 0; i < num_workers; i++)
     futures.push_back(TaskPool::AddTask(wrapper));
   for (size_t i = 0; i < num_workers; i++)
     futures[i].wait();
 }
+
+} // namespace lldb_private
+

lldb/trunk/unittests/Host/TaskPoolTest.cpp

 #include "gtest/gtest.h"
 
 #include "lldb/Host/TaskPool.h"
 
+using namespace lldb_private;
+
 TEST(TaskPoolTest, AddTask) {
   auto fn = [](int x) { return x * x + 1; };
 
   auto f1 = TaskPool::AddTask(fn, 1);
   auto f2 = TaskPool::AddTask(fn, 2);
   auto f3 = TaskPool::AddTask(fn, 3);
   auto f4 = TaskPool::AddTask(fn, 4);