search

progschj / ThreadPool

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

Develop #38

Closed WillBrennan closed 7 years ago

Adrian-Rosoga commented 7 years ago

It is definitely not right to impose an upper limit to the number of threads in ThreadPool.h. That is: if (n_threads == 0 || n_threads > std::thread::hardware_concurrency())...

Processes with more IO would benefit having more threads than std::thread::hardware_concurrency(). The test makes sense only if computations are 100% CPU bound. That is not always the case in real life.

See for example David Schmitt's answer from http://stackoverflow.com/questions/510441/how-many-threads-to-create-and-when ("For all other situations XU / %IO gives an approximation of how many threads are needed to fully use the available XUs").

I would remove the test as it makes using the thread pool quite inefficient for quite a range of applications.

WillBrennan commented 7 years ago

Hey - Sorry for the huge accidental pull request, was merging a change into my own master.

Very good point, you're right, I was only considering 100% CPU bound operations. Removed the test. I'll leave the pull request closed, I can open it again if you'd like to merge it?

Adrian-Rosoga commented 7 years ago

Hi Will, no worry, I am not the owner of the project so cannot approve pull requests, I just set watching it a while back. I was commenting because I saw activity and was curious what new things would happen.