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

Subject: Re: [boost] [gsoc-2013] Boost.Thread/ThreadPool project
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-04-27 02:35:49

Le 25/04/13 21:42, Niall Douglas a écrit :
>>> [1] http://danlincan.3owl.com/gsoc/Proposal.pdf
>>>
>> 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.

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.
> 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 
problem on the standard using thread pools.
> 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 
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));
   }
};