[Lldb-commits] [lldb] e57361c - [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 22 09:17:56 PDT 2020
Author: Jonas Devlieghere
Date: 2020-04-22T09:17:49-07:00
New Revision: e57361c055d7617ee25cdac8167625000d098ef5
URL: https://github.com/llvm/llvm-project/commit/e57361c055d7617ee25cdac8167625000d098ef5
DIFF: https://github.com/llvm/llvm-project/commit/e57361c055d7617ee25cdac8167625000d098ef5.diff
LOG: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool
Remove LLDB's TaskPool and replace its uses with LLVM's ThreadPool.
Differential revision: https://reviews.llvm.org/D78337
Added:
Modified:
lldb/include/lldb/module.modulemap
lldb/source/Host/CMakeLists.txt
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/unittests/Host/CMakeLists.txt
Removed:
lldb/include/lldb/Host/TaskPool.h
lldb/source/Host/common/TaskPool.cpp
lldb/unittests/Host/TaskPoolTest.cpp
################################################################################
diff --git a/lldb/include/lldb/Host/TaskPool.h b/lldb/include/lldb/Host/TaskPool.h
deleted file mode 100644
index bb91d807ed2c..000000000000
--- a/lldb/include/lldb/Host/TaskPool.h
+++ /dev/null
@@ -1,92 +0,0 @@
-//===--------------------- TaskPool.h ---------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_HOST_TASKPOOL_H
-#define LLDB_HOST_TASKPOOL_H
-
-#include "llvm/ADT/STLExtras.h"
-#include <functional>
-#include <future>
-#include <list>
-#include <memory>
-#include <mutex>
-#include <type_traits>
-
-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 other task on the task pool as they may run on the same
-// thread sequentally.
-class TaskPool {
-public:
- // Add a new task to the task pool and return a std::future belonging to the
- // newly created task. The caller of this function has to wait on the future
- // for this task to complete.
- template <typename F, typename... Args>
- static std::future<typename std::result_of<F(Args...)>::type>
- AddTask(F &&f, Args &&... args);
-
- // Run all of the specified tasks on the task pool and wait until all of them
- // are finished before returning. This method is intended to be used for
- // small number tasks where listing them as function arguments is acceptable.
- // For running large number of tasks you should use AddTask for each task and
- // then call wait() on each returned future.
- template <typename... T> static void RunTasks(T &&... tasks);
-
-private:
- TaskPool() = delete;
-
- template <typename... T> struct RunTaskImpl;
-
- static void AddTaskImpl(std::function<void()> &&task_fn);
-};
-
-template <typename F, typename... Args>
-std::future<typename std::result_of<F(Args...)>::type>
-TaskPool::AddTask(F &&f, Args &&... args) {
- auto task_sp = std::make_shared<
- std::packaged_task<typename std::result_of<F(Args...)>::type()>>(
- std::bind(std::forward<F>(f), std::forward<Args>(args)...));
-
- AddTaskImpl([task_sp]() { (*task_sp)(); });
-
- return task_sp->get_future();
-}
-
-template <typename... T> void TaskPool::RunTasks(T &&... tasks) {
- RunTaskImpl<T...>::Run(std::forward<T>(tasks)...);
-}
-
-template <typename Head, typename... Tail>
-struct TaskPool::RunTaskImpl<Head, Tail...> {
- static void Run(Head &&h, Tail &&... t) {
- auto f = AddTask(std::forward<Head>(h));
- RunTaskImpl<Tail...>::Run(std::forward<Tail>(t)...);
- f.wait();
- }
-};
-
-template <> struct TaskPool::RunTaskImpl<> {
- static void Run() {}
-};
-
-// 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 // LLDB_HOST_TASKPOOL_H
diff --git a/lldb/include/lldb/module.modulemap b/lldb/include/lldb/module.modulemap
index e668abe1c6ae..7feea8ee99c3 100644
--- a/lldb/include/lldb/module.modulemap
+++ b/lldb/include/lldb/module.modulemap
@@ -49,7 +49,6 @@ module lldb_Host {
module SocketAddress { header "Host/SocketAddress.h" export * }
module Socket { header "Host/Socket.h" export * }
module StringConvert { textual header "Host/StringConvert.h" export * }
- module TaskPool { header "Host/TaskPool.h" export * }
module Terminal { header "Host/Terminal.h" export * }
module ThreadLauncher { header "Host/ThreadLauncher.h" export * }
module Time { header "Host/Time.h" export * }
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index 357140432010..2837c0c5e3a1 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -30,7 +30,6 @@ add_host_subdirectory(common
common/SocketAddress.cpp
common/Socket.cpp
common/StringConvert.cpp
- common/TaskPool.cpp
common/TCPSocket.cpp
common/Terminal.cpp
common/ThreadLauncher.cpp
diff --git a/lldb/source/Host/common/TaskPool.cpp b/lldb/source/Host/common/TaskPool.cpp
deleted file mode 100644
index 994ca375499f..000000000000
--- a/lldb/source/Host/common/TaskPool.cpp
+++ /dev/null
@@ -1,126 +0,0 @@
-//===-- TaskPool.cpp ------------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Host/TaskPool.h"
-#include "lldb/Host/ThreadLauncher.h"
-#include "lldb/Utility/Log.h"
-
-#include <cstdint>
-#include <queue>
-#include <thread>
-
-namespace lldb_private {
-
-namespace {
-class TaskPoolImpl {
-public:
- static TaskPoolImpl &GetInstance();
-
- void AddTask(std::function<void()> &&task_fn);
-
-private:
- TaskPoolImpl();
-
- static lldb::thread_result_t WorkerPtr(void *pool);
-
- static void Worker(TaskPoolImpl *pool);
-
- std::queue<std::function<void()>> m_tasks;
- std::mutex m_tasks_mutex;
- uint32_t m_thread_count;
-};
-
-} // end of anonymous namespace
-
-TaskPoolImpl &TaskPoolImpl::GetInstance() {
- static TaskPoolImpl g_task_pool_impl;
- return g_task_pool_impl;
-}
-
-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) {
- 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 < 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).
- llvm::Expected<HostThread> host_thread =
- lldb_private::ThreadLauncher::LaunchThread(
- "task-pool.worker", WorkerPtr, this, min_stack_size);
- if (host_thread) {
- host_thread->Release();
- } else {
- LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST),
- "failed to launch host thread: {}",
- llvm::toString(host_thread.takeError()));
- }
- }
-}
-
-lldb::thread_result_t TaskPoolImpl::WorkerPtr(void *pool) {
- Worker((TaskPoolImpl *)pool);
- return {};
-}
-
-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 = 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};
-
- 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
-
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index f405a2189429..7bf4b52bc783 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -13,10 +13,10 @@
#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
#include "Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h"
#include "lldb/Core/Module.h"
-#include "lldb/Host/TaskPool.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/Timer.h"
+#include "llvm/Support/ThreadPool.h"
using namespace lldb_private;
using namespace lldb;
@@ -71,20 +71,27 @@ void ManualDWARFIndex::Index() {
clear_cu_dies[cu_idx] = units_to_index[cu_idx]->ExtractDIEsScoped();
};
+ // Share one thread pool across operations to avoid the overhead of
+ // recreating the threads.
+ llvm::ThreadPool pool;
+
// Create a task runner that extracts dies for each DWARF unit in a
- // separate thread
+ // separate thread.
// First figure out which units didn't have their DIEs already
// parsed and remember this. If no DIEs were parsed prior to this index
// function call, we are going to want to clear the CU dies after we are
// done indexing to make sure we don't pull in all DWARF dies, but we need
// to wait until all units have been indexed in case a DIE in one
// unit refers to another and the indexes accesses those DIEs.
- TaskMapOverInt(0, units_to_index.size(), extract_fn);
+ for (size_t i = 0; i < units_to_index.size(); ++i)
+ pool.async(extract_fn, i);
+ pool.wait();
// Now create a task runner that can index each DWARF unit in a
// separate thread so we can index quickly.
-
- TaskMapOverInt(0, units_to_index.size(), parser_fn);
+ for (size_t i = 0; i < units_to_index.size(); ++i)
+ pool.async(parser_fn, i);
+ pool.wait();
auto finalize_fn = [this, &sets](NameToDIE(IndexSet::*index)) {
NameToDIE &result = m_set.*index;
@@ -93,14 +100,15 @@ void ManualDWARFIndex::Index() {
result.Finalize();
};
- TaskPool::RunTasks([&]() { finalize_fn(&IndexSet::function_basenames); },
- [&]() { finalize_fn(&IndexSet::function_fullnames); },
- [&]() { finalize_fn(&IndexSet::function_methods); },
- [&]() { finalize_fn(&IndexSet::function_selectors); },
- [&]() { finalize_fn(&IndexSet::objc_class_selectors); },
- [&]() { finalize_fn(&IndexSet::globals); },
- [&]() { finalize_fn(&IndexSet::types); },
- [&]() { finalize_fn(&IndexSet::namespaces); });
+ pool.async(finalize_fn, &IndexSet::function_basenames);
+ pool.async(finalize_fn, &IndexSet::function_fullnames);
+ pool.async(finalize_fn, &IndexSet::function_methods);
+ pool.async(finalize_fn, &IndexSet::function_selectors);
+ pool.async(finalize_fn, &IndexSet::objc_class_selectors);
+ pool.async(finalize_fn, &IndexSet::globals);
+ pool.async(finalize_fn, &IndexSet::types);
+ pool.async(finalize_fn, &IndexSet::namespaces);
+ pool.wait();
}
void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, SymbolFileDWARFDwo *dwp,
diff --git a/lldb/unittests/Host/CMakeLists.txt b/lldb/unittests/Host/CMakeLists.txt
index b88aa97a9005..663645c986f0 100644
--- a/lldb/unittests/Host/CMakeLists.txt
+++ b/lldb/unittests/Host/CMakeLists.txt
@@ -11,7 +11,6 @@ set (FILES
SocketAddressTest.cpp
SocketTest.cpp
SocketTestUtilities.cpp
- TaskPoolTest.cpp
)
if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
diff --git a/lldb/unittests/Host/TaskPoolTest.cpp b/lldb/unittests/Host/TaskPoolTest.cpp
deleted file mode 100644
index fea544267957..000000000000
--- a/lldb/unittests/Host/TaskPoolTest.cpp
+++ /dev/null
@@ -1,45 +0,0 @@
-#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);
-
- ASSERT_EQ(10, f3.get());
- ASSERT_EQ(2, f1.get());
- ASSERT_EQ(17, f4.get());
- ASSERT_EQ(5, f2.get());
-}
-
-TEST(TaskPoolTest, RunTasks) {
- std::vector<int> r(4);
-
- auto fn = [](int x, int &y) { y = x * x + 1; };
-
- TaskPool::RunTasks([fn, &r]() { fn(1, r[0]); }, [fn, &r]() { fn(2, r[1]); },
- [fn, &r]() { fn(3, r[2]); }, [fn, &r]() { fn(4, r[3]); });
-
- ASSERT_EQ(2, r[0]);
- ASSERT_EQ(5, r[1]);
- ASSERT_EQ(10, r[2]);
- ASSERT_EQ(17, r[3]);
-}
-
-TEST(TaskPoolTest, TaskMap) {
- int data[4];
- auto fn = [&data](int x) { data[x] = x * x; };
-
- TaskMapOverInt(0, 4, fn);
-
- ASSERT_EQ(data[0], 0);
- ASSERT_EQ(data[1], 1);
- ASSERT_EQ(data[2], 4);
- ASSERT_EQ(data[3], 9);
-}
More information about the lldb-commits
mailing list