CODEWITHSUNDEEP
c++multithreadingc++11lambda
Yue Wang
Yue Wang

Reputation: 1750

What's the difference between functor and lambda when used for constructing std::thread

I'm testing a code from the book C++ Concurrency in Action. Basically, it uses multi-thread to implement the same function as std::accumulate. As the following shows, I tried to use lambda in place of functor, but my code gave wrong results. The variable is_from_book can be toggled to test the two ways. 

#include <thread>
#include <algorithm>
#include <vector>
#include <iostream>

namespace para {

template<typename Iter, typename Value>
struct AccumulateBlock
{
    void operator ()(Iter first, Iter last, Value& result)
    {
        result = std::accumulate(first, last, result);
    }
};

template<typename Iter, typename Value>
Value parallel_accumulate(Iter first, Iter last, Value init_val)
{
    using std::size_t;
    size_t length = std::distance(first, last);
    if(length == 0) return init_val;    // trivial case

    size_t min_per_thread = 25;
    size_t max_threads = (length + min_per_thread - 1) / min_per_thread;
    size_t hardware_threads = std::thread::hardware_concurrency();
    size_t num_threads = std::min((hardware_threads!=0 ? hardware_threads : 2), max_threads);
    size_t block_size = length/num_threads;
    std::vector<Value> results(num_threads);
    std::vector<std::thread> threads{num_threads - 1};

    Iter block_start = first;
    for(unsigned long idx=0; idx!=(num_threads-1);  ++idx )
    {
        Iter block_end = block_start;
        std::advance(block_end, block_size);

        if(bool is_from_book = false)   //code from the book that uses functor
        {
            threads[idx] = std::thread{
                    para::AccumulateBlock<Iter, Value>{},
                    block_start,
                    block_end,
                    std::ref(results[idx])
            };
        }
        else    //my code that tries to use lambda instead of functor
        {
            threads[idx] = std::thread{
                [&]{
                    results[idx] = std::accumulate(block_start, block_end, results[idx]);
                }
            };
        }

        block_start = block_end;
    }

    para::AccumulateBlock<Iter, Value>{}(block_start, last, results[num_threads-1]);

    for(auto& t : threads)  t.join();
    return std::accumulate(results.begin(), results.end(), init_val);
}
}//namespace

int main()
{
    std::vector<int> v(10000,1);
    auto sum = para::parallel_accumulate(v.begin(), v.end(), 0);
    std::cout << "sum = " << sum << std::endl;

    return 0;
}

My question is what's the problem? Am I doing right? Any difference between the two ways? How to fix it?Thx.

Upvotes: 0

Views: 182

Answers (1)

Mike Seymour
Mike Seymour

Reputation: 254471

You're capturing everything by reference, so idx, block_start and block_end are shifting under the thread's feet, causing all manner of undefined behaviour.

Capture results by reference - or, safer still, just the array element the thread needs - and the others by value:

Value & result = results[idx];
threads[idx] = std::thread{
    [&result,block_start,block_end]{   // or [=,&result] if you like brevity
        result = std::accumulate(block_start, block_end, result);
    }
};

Upvotes: 5

Related Questions