From 588b59cfa114e94b3e5b612f3fed65bd7bb0ba5d Mon Sep 17 00:00:00 2001 From: Jakob Progsch Date: Thu, 25 Sep 2014 12:06:19 +0200 Subject: [PATCH 1/7] Check stop condition under lock. Use emplace instead of push. --- ThreadPool.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ThreadPool.h b/ThreadPool.h index 0235c8b..b6d95d7 100644 --- a/ThreadPool.h +++ b/ThreadPool.h @@ -60,10 +60,6 @@ auto ThreadPool::enqueue(F&& f, Args&&... args) -> std::future::type> { typedef typename std::result_of::type return_type; - - // don't allow enqueueing after stopping the pool - if(stop) - throw std::runtime_error("enqueue on stopped ThreadPool"); auto task = std::make_shared< std::packaged_task >( std::bind(std::forward(f), std::forward(args)...) @@ -72,7 +68,12 @@ auto ThreadPool::enqueue(F&& f, Args&&... args) std::future res = task->get_future(); { std::unique_lock lock(queue_mutex); - tasks.push([task](){ (*task)(); }); + + // don't allow enqueueing after stopping the pool + if(stop) + throw std::runtime_error("enqueue on stopped ThreadPool"); + + tasks.emplace([task](){ (*task)(); }); } condition.notify_one(); return res; From 0fb92153980554853b95ece4bfd97a41058ac3a6 Mon Sep 17 00:00:00 2001 From: Vaclav Zeman Date: Thu, 11 Sep 2014 13:03:55 +0200 Subject: [PATCH 2/7] Do not call unlock() manually. --- ThreadPool.h | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/ThreadPool.h b/ThreadPool.h index b6d95d7..66634d8 100644 --- a/ThreadPool.h +++ b/ThreadPool.h @@ -40,14 +40,18 @@ inline ThreadPool::ThreadPool(size_t threads) { for(;;) { - std::unique_lock lock(this->queue_mutex); - while(!this->stop && this->tasks.empty()) - this->condition.wait(lock); - if(this->stop && this->tasks.empty()) - return; - std::function task(this->tasks.front()); - this->tasks.pop(); - lock.unlock(); + std::function task; + + { + std::unique_lock lock(this->queue_mutex); + while(!this->stop && this->tasks.empty()) + this->condition.wait(lock); + if(this->stop && this->tasks.empty()) + return; + task = std::move(this->tasks.front()); + this->tasks.pop(); + } + task(); } } From b168ca0037ea28c145b766f52c4ba3dde15a6ffd Mon Sep 17 00:00:00 2001 From: Vaclav Zeman Date: Thu, 11 Sep 2014 13:07:31 +0200 Subject: [PATCH 3/7] Use C++11 for(a:b) loop. --- ThreadPool.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ThreadPool.h b/ThreadPool.h index 66634d8..04f0291 100644 --- a/ThreadPool.h +++ b/ThreadPool.h @@ -91,8 +91,8 @@ inline ThreadPool::~ThreadPool() stop = true; } condition.notify_all(); - for(size_t i = 0;i Date: Thu, 25 Sep 2014 12:07:58 +0200 Subject: [PATCH 4/7] Use emplace_back() and for(a:b) loop. --- example.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/example.cpp b/example.cpp index 66d6ab7..837277b 100644 --- a/example.cpp +++ b/example.cpp @@ -11,7 +11,7 @@ int main() std::vector< std::future > results; for(int i = 0; i < 8; ++i) { - results.push_back( + results.emplace_back( pool.enqueue([i] { std::cout << "hello " << i << std::endl; std::this_thread::sleep_for(std::chrono::seconds(1)); @@ -19,10 +19,10 @@ int main() return i*i; }) ); - } - - for(size_t i = 0;i Date: Fri, 12 Sep 2014 07:25:17 +0200 Subject: [PATCH 5/7] Use two argument condition_variable::wait() instead of a loop. --- ThreadPool.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ThreadPool.h b/ThreadPool.h index 04f0291..076f9fd 100644 --- a/ThreadPool.h +++ b/ThreadPool.h @@ -44,8 +44,8 @@ inline ThreadPool::ThreadPool(size_t threads) { std::unique_lock lock(this->queue_mutex); - while(!this->stop && this->tasks.empty()) - this->condition.wait(lock); + this->condition.wait(lock, + [this]{ return this->stop || !this->tasks.empty(); }); if(this->stop && this->tasks.empty()) return; task = std::move(this->tasks.front()); From fcc914156f4387ad4d178c4be7f1b317aec4faa4 Mon Sep 17 00:00:00 2001 From: Jakob Progsch Date: Fri, 26 Sep 2014 12:31:30 +0200 Subject: [PATCH 6/7] added attribution --- COPYING | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COPYING b/COPYING index c57ee2d..7d69178 100644 --- a/COPYING +++ b/COPYING @@ -1,4 +1,4 @@ -Copyright (c) 2012 Jakob Progsch +Copyright (c) 2012 Jakob Progsch, Václav Zeman This software is provided 'as-is', without any express or implied warranty. In no event will the authors be held liable for any damages From 9a42ec1329f259a5f4881a291db1dcb8f2ad9040 Mon Sep 17 00:00:00 2001 From: Jakob Progsch Date: Fri, 26 Sep 2014 12:32:41 +0200 Subject: [PATCH 7/7] changed typedef to using --- ThreadPool.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ThreadPool.h b/ThreadPool.h index 076f9fd..4183203 100644 --- a/ThreadPool.h +++ b/ThreadPool.h @@ -63,7 +63,7 @@ template auto ThreadPool::enqueue(F&& f, Args&&... args) -> std::future::type> { - typedef typename std::result_of::type return_type; + using return_type = typename std::result_of::type; auto task = std::make_shared< std::packaged_task >( std::bind(std::forward(f), std::forward(args)...) @@ -91,7 +91,7 @@ inline ThreadPool::~ThreadPool() stop = true; } condition.notify_all(); - for(std::thread & worker: workers) + for(std::thread &worker: workers) worker.join(); }