Simple parallel_for_each in C++ I need to parallelise a for loop that does quite a lot of processing over thousands of items. I came up with this basic loop that seems to work quite effectively on a small test.

template&lt;class T, class C&gt;
void parallel_for_each(std::vector&lt;T&gt;* ts, C callable, uint poolsize = (std::thread::hardware_concurrency()+1)*2) {

    atomic&lt;size_t&gt; index(0);
    std::vector&lt;thread&gt; threads;

    for (uint i = 0; i &lt; poolsize; i++) {
        threads.emplace_back([&amp;]() {    
            while(index &lt; ts-&gt;size()) {
                callable(ts-&gt;at(index++));
            }
        });
    }
    for (auto&amp;&amp; thread : threads) {
        thread.join();
    }
}

//use like:
std::vector&lt;int&gt; is= { 4,5, 6,7,8,3,2,25,44,34,562,356,235,66,3,45,66};
parallel_for_each(&amp;is, print);


Is there any serious problem I've overlooked, or anything else I can improve?
If you are not going to create a long running thread pool (and manage the threads over many jobs), then I would let the system do it for you; the async() function potentially runs stuff in parallel and manages a thread pool for you.

// Not tested.
// You may need to play with it.
template&lt;class T, class C&gt;
void parallel_for_each(std::vector&lt;T&gt;&amp; ts, C callable)
{
    size_t index =;
    std::vector&lt;std::future&gt; results;

    for(auto const&amp; value: ts)
    {
        results.emplace_back(async(callable, value));
    }
    for (auto&amp; result : results)
    {
        result.wait();
    }
}


Other notes

Don't pass pointers around.

One of your parameters is:

std::vector&lt;T&gt;* ts
          //  ^  Pointer


Passing by pointer is very rare in C++ (very common in bad C++ written by old C programmers). The reason for this is there are no ownership semantics associated with the pointer and thus potential for misunderstanding the interface and thus leading to memory leaks.

In C++ we want to very clearly define the ownership of pointers as it is the owner of the pointer (and nobody else) that is responsible for deleting the pointer. The language has introduced smart pointers that allow us to specify ownership so that when the pointer is passed we are explicit in passing ownership with the pointer. In other situations were ownership is not passed we use references.

If I was using your function for the first time I could not tell if I need to dynamically create the vector or not. Is your function going to delete the passed pointer? I can't tell from reading the interface because raw pointers have no ownership semantics.

If you change your interface to pass by reference then it works just as well and there will be no confusion. If you want to pass ownership, you need to use smart pointers to indicate the type of ownership that is being transferred.

Prefer prefix increment.

ts-&gt;at(index++)


It makes no difference for integer types. But when using other types (including iterators) the default implementation for increment usually makes the prefix version more efficient. So when it makes no difference to your code, prefer the prefix version. This allows the types of your objects to be changed without having to update the code and it will still be the most efficient version.

Prefer operator[]() to at()

If you know that your access to an array will always be in bounds then prefer operator[] as it performs no accesses checks and is thus quicker.

Note: throwing an exception that is not caught inside a thread will cause the application to terminate. Throwing an exception inside an async will catch the exception and transfer it to the future object allowing the main thread to detect it.

Don't know why you are using r-value reference here

for (auto&amp;&amp; thread : threads)
         ^^


Don't know what that gives you. Note: an named r-value reference is treated liked an r-value when passed as a parameter (because it is named). You would still need to call std::move() on it to make it bind to the r-value reference function.


  Sorry I don't know the X/R/L/V value naming scheme very well yet so I am using an older terminology. Please have a look at appropriate documentation.


Personally I would just use a reference:

for (auto&amp; thread : threads)