progschj / ThreadPool

A simple C++11 Thread Pool implementation
zlib License
7.64k stars 2.21k forks source link

Small changes. #19

Closed wilx closed 9 years ago

wilx commented 9 years ago

I have made small changes to your ThreadPool code. I intend to use this in log4cplus 2.0+ to implement asynchronous logging.

progschj commented 9 years ago

I'll have to get around to this :). It was kinda busy recently. I agree with most of them. I'm still not the biggest fan of the default size. Mostly because of the mentioned reasons. I could imagine using max(1, hardware_concurrency()). But then I'm not so sure if a default value is even appropriate here. At least in the generic implementation. I feel the worker amount should always be a conscious decision of the user. One reason why I wrote this was that I felt std::async was too opaque and I didn't really trust it to magically do the right thing. Providing a default again goes into that direction.

I'll probably also skip the wait_until_empty() thing. Not because I don't think it is useful. The main idea of having this repo was to have a base implementation that lends itself well to adaptation. Adding features like this opens it to a whole can of worms though: why don't we have a way to insert barriers then? dependencies? priorities?... :). If there is an argument here why this specific feature is "more fundamental" I'm open to adding it. Othewise I'm just happy keeping it in one of the many specialized forks though.

Cheers

wilx commented 9 years ago

I agree that not all of my additions are suitable for everyone. Feel free to cherry pick any or none of the change sets. I just wanted to offer you something back in return for you excellent starting point. :)

wilx commented 9 years ago

I am not sure how to limit the Github pull request only to some of the changes, so close the issue, if that is what it takes, and just merge stuff using CLI.

progschj commented 9 years ago

I added most of the changes. During rebasing I had to resolve some trivial conflicts which apparently changed the commit author to me. I hope that is ok?

wilx commented 9 years ago

No problem.