From 7c17201f14c68991bfad38a9f8041fd5b49571bd Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Wed, 13 Jun 2018 21:24:19 +0000 Subject: [PATCH] Revert "Enable ThreadPool to queue tasks that return values." This is failing to compile when LLVM_ENABLE_THREADS is false, and the fix is not immediately obvious, so reverting while I look into it. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@334658 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/ThreadPool.h | 51 ++++++--------------------------------- lib/Support/ThreadPool.cpp | 21 ++++++++++++++-- unittests/Support/ThreadPool.cpp | 19 --------------- 3 files changed, 26 insertions(+), 65 deletions(-) diff --git a/include/llvm/Support/ThreadPool.h b/include/llvm/Support/ThreadPool.h index 26e0ee84b48..4fdbd528b21 100644 --- a/include/llvm/Support/ThreadPool.h +++ b/include/llvm/Support/ThreadPool.h @@ -14,14 +14,12 @@ #ifndef LLVM_SUPPORT_THREAD_POOL_H #define LLVM_SUPPORT_THREAD_POOL_H -#include "llvm/ADT/STLExtras.h" #include "llvm/Config/llvm-config.h" #include "llvm/Support/thread.h" #include #include -#include #include #include #include @@ -37,21 +35,10 @@ namespace llvm { /// The pool keeps a vector of threads alive, waiting on a condition variable /// for some work to become available. class ThreadPool { - struct TaskBase { - virtual ~TaskBase() {} - virtual void execute() = 0; - }; - - template struct TypedTask : public TaskBase { - explicit TypedTask(std::packaged_task Task) - : Task(std::move(Task)) {} - - void execute() override { Task(); } - - std::packaged_task Task; - }; - public: + using TaskTy = std::function; + using PackagedTaskTy = std::packaged_task; + /// Construct a pool with the number of threads found by /// hardware_concurrency(). ThreadPool(); @@ -65,8 +52,7 @@ public: /// Asynchronous submission of a task to the pool. The returned future can be /// used to wait for the task to finish and is *non-blocking* on destruction. template - inline std::shared_future::type> - async(Function &&F, Args &&... ArgList) { + inline std::shared_future async(Function &&F, Args &&... ArgList) { auto Task = std::bind(std::forward(F), std::forward(ArgList)...); return asyncImpl(std::move(Task)); @@ -75,8 +61,7 @@ public: /// Asynchronous submission of a task to the pool. The returned future can be /// used to wait for the task to finish and is *non-blocking* on destruction. template - inline std::shared_future::type> - async(Function &&F) { + inline std::shared_future async(Function &&F) { return asyncImpl(std::forward(F)); } @@ -87,35 +72,13 @@ public: private: /// Asynchronous submission of a task to the pool. The returned future can be /// used to wait for the task to finish and is *non-blocking* on destruction. - template - std::shared_future::type> - asyncImpl(TaskTy &&Task) { - typedef decltype(Task()) ResultTy; - - /// Wrap the Task in a packaged_task to return a future object. - std::packaged_task PackagedTask(std::move(Task)); - auto Future = PackagedTask.get_future(); - std::unique_ptr TB = - llvm::make_unique>(std::move(PackagedTask)); - - { - // Lock the queue and push the new task - std::unique_lock LockGuard(QueueLock); - - // Don't allow enqueueing after disabling the pool - assert(EnableFlag && "Queuing a thread during ThreadPool destruction"); - - Tasks.push(std::move(TB)); - } - QueueCondition.notify_one(); - return Future.share(); - } + std::shared_future asyncImpl(TaskTy F); /// Threads in flight std::vector Threads; /// Tasks waiting for execution in the pool. - std::queue> Tasks; + std::queue Tasks; /// Locking and signaling for accessing the Tasks queue. std::mutex QueueLock; diff --git a/lib/Support/ThreadPool.cpp b/lib/Support/ThreadPool.cpp index fef665ba3d1..d0212ca1346 100644 --- a/lib/Support/ThreadPool.cpp +++ b/lib/Support/ThreadPool.cpp @@ -32,7 +32,7 @@ ThreadPool::ThreadPool(unsigned ThreadCount) for (unsigned ThreadID = 0; ThreadID < ThreadCount; ++ThreadID) { Threads.emplace_back([&] { while (true) { - std::unique_ptr Task; + PackagedTaskTy Task; { std::unique_lock LockGuard(QueueLock); // Wait for tasks to be pushed in the queue @@ -54,7 +54,7 @@ ThreadPool::ThreadPool(unsigned ThreadCount) Tasks.pop(); } // Run the task we just grabbed - Task->execute(); + Task(); { // Adjust `ActiveThreads`, in case someone waits on ThreadPool::wait() @@ -79,6 +79,23 @@ void ThreadPool::wait() { [&] { return !ActiveThreads && Tasks.empty(); }); } +std::shared_future ThreadPool::asyncImpl(TaskTy Task) { + /// Wrap the Task in a packaged_task to return a future object. + PackagedTaskTy PackagedTask(std::move(Task)); + auto Future = PackagedTask.get_future(); + { + // Lock the queue and push the new task + std::unique_lock LockGuard(QueueLock); + + // Don't allow enqueueing after disabling the pool + assert(EnableFlag && "Queuing a thread during ThreadPool destruction"); + + Tasks.push(std::move(PackagedTask)); + } + QueueCondition.notify_one(); + return Future.share(); +} + // The destructor joins all threads, waiting for completion. ThreadPool::~ThreadPool() { { diff --git a/unittests/Support/ThreadPool.cpp b/unittests/Support/ThreadPool.cpp index d2d50c3764e..0da33ad50c0 100644 --- a/unittests/Support/ThreadPool.cpp +++ b/unittests/Support/ThreadPool.cpp @@ -147,25 +147,6 @@ TEST_F(ThreadPoolTest, GetFuture) { ASSERT_EQ(2, i.load()); } -TEST_F(ThreadPoolTest, TaskWithResult) { - CHECK_UNSUPPORTED(); - // By making only 1 thread in the pool the two tasks are serialized with - // respect to each other, which means that the second one must return 2. - ThreadPool Pool{1}; - std::atomic_int i{0}; - Pool.async([this, &i] { - waitForMainThread(); - ++i; - }); - // Force the future using get() - std::shared_future Future = Pool.async([&i] { return ++i; }); - ASSERT_EQ(0, i.load()); - setMainThreadReady(); - int Result = Future.get(); - ASSERT_EQ(2, i.load()); - ASSERT_EQ(2, Result); -} - TEST_F(ThreadPoolTest, PoolDestruction) { CHECK_UNSUPPORTED(); // Test that we are waiting on destruction -- 2.11.0