Re: [boost] [gsoc-2013] Boost.Thread/ThreadPool project

I have some questions:
* Why the submit function of all the thread pools doesn't returns the same? * What is the advantage of returning future from submit call? ....
My personal big issue with a Boost.Thread threadpool is why adding one is necessary when forty lines of C++11 code using Boost.ASIO implements you one: /*! \class thread_pool \brief A very simple thread pool based on Boost.ASIO and std::thread */ class thread_pool { class worker { thread_pool *pool; public: explicit worker(thread_pool *p) : pool(p) { } void operator()() { pool->service.run(); } }; friend class worker; std::vector< std::unique_ptr<thread> > workers; boost::asio::io_service service; boost::asio::io_service::work working; public: //! Constructs a thread pool of \em no workers explicit thread_pool(size_t no) : working(service) { workers.reserve(no); for(size_t n=0; n<no; n++) workers.push_back(std::unique_ptr<thread>(new thread(worker(this)))); } ~thread_pool() { service.stop(); for(auto &i : workers) i->join(); } //! Returns the underlying io_service boost::asio::io_service &io_service() { return service; } //! Sends some callable entity to the thread pool for execution template<class F> future<typename std::result_of<F()>::type> enqueue(F f) { typedef typename std::result_of<F()>::type R; // Somewhat annoyingly, io_service.post() needs its parameter to be copy constructible, // and packaged_task is only move constructible, so ... auto task=std::make_shared<boost::packaged_task<R>>(std::move(f)); // NOTE to self later: this ought to go to std::packaged_task<R()> service.post(std::bind([](std::shared_ptr<boost::packaged_task<R>> t) { (*t)(); }, task)); return task->get_future(); } }; ... and you're done. Moreover, this thread pool integrates seamlessly with Boost.ASIO ioservice callback dispatch which is a huge plus. I took that example from some page on the internet, so I don't claim I wrote most of the above. Do bear in mind before you respond that parts of Boost.ASIO are scheduled to enter the standard in TR2, therefore no you won't need Boost.ASIO as a dependency soon. I'm not ruling out the merit of a Boost.Thread threadpool. For me personally though, the hurdle is very significantly raised: you need to strongly prove to me why your proposed thread pool is much superior to a Boost.ASIO based threadpool. Otherwise I will zero score the GSoC proposal, because in the end we need other proposed projects more. By the way, I actually don't object at all if your proposed Boost.Thread threadpool is a thin wrapper of Boost.ASIO. That, actually, is just fine with me, though I'd doubt Google would feel it substantial enough for GSoC funding seeing as it would be finished within a week. Niall

I'm not ruling out the merit of a Boost.Thread threadpool. For me personally though, the hurdle is very significantly raised: you need to strongly prove to me why your proposed thread pool is much superior to a Boost.ASIO based threadpool. Otherwise I will zero score the GSoC proposal, because in the end we need other proposed projects more.
Hello, Superior in what way? Performance-wise, interface-wise? What advanced features would you want from a thread pool to make it superior to the presented implementation? Thank you, Dan

Le 25/04/13 21:42, Niall Douglas a écrit :
I have some questions:
* Why the submit function of all the thread pools doesn't returns the same? * What is the advantage of returning future from submit call? .... My personal big issue with a Boost.Thread threadpool is why adding one is necessary when forty lines of C++11 code using Boost.ASIO implements you one:
/*! \class thread_pool \brief A very simple thread pool based on Boost.ASIO and std::thread */ class thread_pool { class worker { thread_pool *pool; public: explicit worker(thread_pool *p) : pool(p) { } void operator()() { pool->service.run(); } }; friend class worker;
std::vector< std::unique_ptr<thread> > workers; boost::asio::io_service service; boost::asio::io_service::work working; public: //! Constructs a thread pool of \em no workers explicit thread_pool(size_t no) : working(service) { workers.reserve(no); for(size_t n=0; n<no; n++) workers.push_back(std::unique_ptr<thread>(new thread(worker(this)))); } ~thread_pool() { service.stop(); for(auto &i : workers) i->join(); } //! Returns the underlying io_service boost::asio::io_service &io_service() { return service; } //! Sends some callable entity to the thread pool for execution template<class F> future<typename std::result_of<F()>::type> enqueue(F f) { typedef typename std::result_of<F()>::type R; // Somewhat annoyingly, io_service.post() needs its parameter to be copy constructible, // and packaged_task is only move constructible, so ... auto task=std::make_shared<boost::packaged_task<R>>(std::move(f)); // NOTE to self later: this ought to go to std::packaged_task<R()>
service.post(std::bind([](std::shared_ptr<boost::packaged_task<R>> t) { (*t)(); }, task)); return task->get_future(); } };
... and you're done.
Moreover, this thread pool integrates seamlessly with Boost.ASIO ioservice callback dispatch which is a huge plus. I'm not against it, but could you explain me why this is a plus? I took that example from some page on the internet, so I don't claim I wrote most of the above.
Do bear in mind before you respond that parts of Boost.ASIO are scheduled to enter the standard in TR2, therefore no you won't need Boost.ASIO as a dependency soon. well, std::thread and std::async are already there, so I think integrating well with then is yet more important to me than integrating with a future ASIO standard library. I don't dismiss the good design of ASIO, I just want to note that there are a some C++1y proposals that try to introduce solutions for this
I'm not ruling out the merit of a Boost.Thread threadpool. For me personally though, the hurdle is very significantly raised: you need to strongly prove to me why your proposed thread pool is much superior to a Boost.ASIO based threadpool. Otherwise I will zero score the GSoC proposal, because in the end we need other proposed projects more. This is your right, but I don't know how the fact you can write easily
Why do you think it would take more lines than that for a such simple pool if we don't use ASIO? The use of ASIO is an implementation detail and I don't see what would be the benefit for the user. What is important is the service provided to the user, not how it is implemented. What I'm locking for is not only such a simple thread pool, but a collection of thread pool that are able to satisfy the constraints different users can have on different contexts. This can not be done on a week. problem on the standard. the thread pool you need to diminish a generic thread pool working well with futures.
By the way, I actually don't object at all if your proposed Boost.Thread threadpool is a thin wrapper of Boost.ASIO. That, actually, is just fine with me, though I'd doubt Google would feel it substantial enough for GSoC funding seeing as it would be finished within a week.
I would not mentor a Boost.Thread/ThreadPool project that must be implemented using ASIO. If a good Thread pool library would take I week to do I don't understand why we don't have it yet on the standard. Below it is the simple thread pool in the book C++ Concurrency in Action. This is one of the simplest thread pools that can be provided but not the single one (work stealing thread pools, serializers, time based). The idea is to have a collection of thread pools each one with its own advantages. These thread pool must be integrated well with async/futures and provide additional features as cancellation, continuations, serialization, fork/join ... Hopping this helps you to understand the project I'm looking for. Best, Vicente class thread_pool { std::atomic_bool done; thread_safe_queue<std::function<void()> > work_queue; std::vector<std::thread> threads; join_threads joiner; void worker_thread() { while (!done) { std::function < void() > task; if (work_queue.try_pop(task)) { task(); } else { std::this_thread::yield(); } } } public: thread_pool() : done(false), joiner(threads) { unsigned const thread_count = std::thread::hardware_concurrency(); try { for (unsigned i = 0; i < thread_count; ++i) { threads.push_back(std::thread(&thread_pool::worker_thread, this)); } } catch (...) { done = true; throw; } } ~thread_pool() { done = true; } template <typename FunctionType> void submit(FunctionType f) { work_queue.push(std::function<void()>(f)); } };

Le 25/04/13 21:42, Niall Douglas a écrit :
I have some questions:
* Why the submit function of all the thread pools doesn't returns the same? * What is the advantage of returning future from submit call? .... My personal big issue with a Boost.Thread threadpool is why adding one is necessary when forty lines of C++11 code using Boost.ASIO implements you one:
/*! \class thread_pool \brief A very simple thread pool based on Boost.ASIO and std::thread */ class thread_pool { class worker { thread_pool *pool; public: explicit worker(thread_pool *p) : pool(p) { } void operator()() { pool->service.run(); } }; friend class worker;
std::vector< std::unique_ptr<thread> > workers; boost::asio::io_service service; boost::asio::io_service::work working; public: //! Constructs a thread pool of \em no workers explicit thread_pool(size_t no) : working(service) { workers.reserve(no); for(size_t n=0; n<no; n++) workers.push_back(std::unique_ptr<thread>(new thread(worker(this)))); } ~thread_pool() { service.stop(); for(auto &i : workers) i->join(); } //! Returns the underlying io_service boost::asio::io_service &io_service() { return service; } //! Sends some callable entity to the thread pool for execution template<class F> future<typename std::result_of<F()>::type> enqueue(F f) { typedef typename std::result_of<F()>::type R; // Somewhat annoyingly, io_service.post() needs its parameter to be copy constructible, // and packaged_task is only move constructible, so ... auto task=std::make_shared<boost::packaged_task<R>>(std::move(f)); // NOTE to self later: this ought to go to std::packaged_task<R()>
service.post(std::bind([](std::shared_ptr<boost::packaged_task<R>> t) { (*t)(); }, task)); return task->get_future(); } };
... and you're done.
Moreover, this thread pool integrates seamlessly with Boost.ASIO ioservice callback dispatch which is a huge plus. I'm not against it, but could you explain me why this is a plus? I took that example from some page on the internet, so I don't claim I wrote most of the above.
Do bear in mind before you respond that parts of Boost.ASIO are scheduled to enter the standard in TR2, therefore no you won't need Boost.ASIO as a dependency soon. well, std::thread and std::async are already there, so I think integrating well with then is yet more important to me than integrating with a future ASIO standard library. I don't dismiss the good design of ASIO, I just want to note that there are some C++1y proposals that try to introduce solutions for this
I'm not ruling out the merit of a Boost.Thread threadpool. For me personally though, the hurdle is very significantly raised: you need to strongly prove to me why your proposed thread pool is much superior to a Boost.ASIO based threadpool. Otherwise I will zero score the GSoC proposal, because in the end we need other proposed projects more. This is your right, but I don't know how the fact you can write easily
Why do you think it would take more lines than that for a such simple pool if we don't use ASIO? See a simple implementation below. The use of ASIO is an implementation detail and I don't see what would be the benefit for the user. What is important is the service provided to the user, not how it is implemented. What I'm locking for is not only such a simple thread pool, but a collection of thread pool that are able to satisfy the constraints different users can have on different contexts. This can not be done on a week. problem on the standard using thread pools. the thread pool you need to diminish a generic thread pool working well with futures.
By the way, I actually don't object at all if your proposed Boost.Thread threadpool is a thin wrapper of Boost.ASIO. That, actually, is just fine with me, though I'd doubt Google would feel it substantial enough for GSoC funding seeing as it would be finished within a week.
If a good ThreadPool library would take I week to do I don't undersatand why we don't have it yet on the standard. Best, Vicente P.S. here it is the simple thread pool in the book C++ Concurrency in Action by A. Williams class thread_pool { std::atomic_bool done; thread_safe_queue<std::function<void()> > work_queue; std::vector<std::thread> threads; join_threads joiner; void worker_thread() { while (!done) { std::function < void() > task; if (work_queue.try_pop(task)) { task(); } else { std::this_thread::yield(); } } } public: thread_pool() : done(false), joiner(threads) { unsigned const thread_count = std::thread::hardware_concurrency(); try { for (unsigned i = 0; i < thread_count; ++i) { threads.push_back(std::thread(&thread_pool::worker_thread, this)); } } catch (...) { done = true; throw; } } ~thread_pool() { done = true; } template <typename FunctionType> void submit(FunctionType f) { work_queue.push(std::function<void()>(f)); } };
participants (3)
-
Dan Lincan
-
Niall Douglas
-
Vicente J. Botet Escriba