Implementing a parallel_for_each function

Question

My current attempt doesn't compile due to, I think, a problem with std::bind not being able to deduce the return type. The actual error message is

1>Source.cpp(24): error C2783: 'enable_if::value,std::_BindRx(_fastcall _Farg0::* )(_Ftypes...) volatile const,_Rx,_Farg0,_Ftypes...>,_Types...>>::type std::bind(Rx (_fastcall _Farg0::* const )(_Ftypes...) volatile const,_Types &&...)' : could not deduce template argument for '_Ret'

Also, should I be passing the function to std::bind by value or reference? (via std::ref).

template<class InputIt, class Function>
void parallel_for_each(InputIt first, const size_t elements, Function &function)
{
    unsigned int max_threads = std::max(1u, std::min(static_cast<unsigned int>(elements), std::thread::hardware_concurrency()));
    std::vector<std::thread> threads;
    threads.reserve(max_threads);
    size_t inc = elements / max_threads;
    size_t rem = elements % max_threads;
    std::cout << "inc = " << inc << '\n';
    auto last = first + elements;
    for (; first != last; first += rem > 0 ? inc + 1, --rem : inc)
    {
        std::cout << "rem = " << rem << '\n';
        std::cout << "first = " << *first << '\n';
        threads.emplace_back(std::bind(std::for_each, first, first + inc, function));
    }
    for (auto &t: threads) 
        t.join();
}

Calling it with:

std::vector<int> numbers(678, 42);
parallel_for_each(begin(numbers), numbers.size(), [](int &x){ ++x; });
for (auto &n : numbers)
    assert(n == 43);
std::cout << "Assertion tests passed\n";

Edit: I replaced the bugged for loop with:

while (first != last)
{
    auto it = first;
    first += rem > 0 ? --rem, inc + 1 : inc;
    threads.emplace_back(std::bind(std::for_each<InputIt, Function>, it, first, function));
}

Answer 1

You can point manually, what template you want to use

threads.emplace_back
(
   std::bind(std::for_each<InputIt, Function>, first, first + inc, function)
);

or, you can simply use lambda, without std::bind.

Answer 2

Yes, you are correct, std::for_each is a function template so you need to explicitly pick a specialization. Also, you don't actually need std::bind, the thread constructor(emplace) supports the same syntax.

Concerning your second question, you should not pass the values by reference as you need to explicitly copy the values in case e.g.: the functor would have internal state!

There are some other problems in your implementation.

You should not take the function by reference because that will hinder passing lambdas/functors in place. Also, instead of using plain std::thread you should use std::async which will give you exception safety. If in your implementation a functor would throw an exception, std::terminate would be called.

Another thing which is not directly related to multithreading is that I am getting this warning from clang:

warning: left operand of comma operator has no effect [-Wunused-value] for (; first != last; first += rem > 0 ? inc + 1, --rem : inc)

This might be the reason for the seg-fault I am getting.

In a future version you could add something like load-balancing.