std::threads constructor argument error

Question

I am not a good C++ programmer, but currently using some features of C++ to clean up dirty parts of my C code. The g++ compiler complains about threads[i] = thread(split, i, sums[i], from, to, f, nThreads);. Please help me find the problem.

// mjArray is just a thin class to use instead of std::vector which is too heavy in my case.

#include <cstdio>
#include <cmath>
#include <ctime>
#include <thread>
using namespace std;

template<typename T>
class mjArray {
private:
    T* _array;
    int _length;

public:
    mjArray(int length) {
        _array = new T[length];
        _length = length;
    }

    mjArray(int length, T val) {
        _array = new T[length];
        _length = length;
        for (int i = 0; i < length; ++i) {
            _array[i] = val;
        }
    }

    ~mjArray() {
        delete[] _array;
    }

    T& operator[](int i) {
        return _array[i];
    }

    int length() {
        return _length;
    }
};

void split(int n, double& sum, int from, int to, double (*f)(double), int nThreads) {
    for (int i = from + n; i <= to; i += nThreads) {
        sum += f(i);
    }
}

double sigma(int from, int to, double (*f)(double), int nThreads) {
    double sum = 0.0;
    mjArray<double> sums(nThreads, 0.0);
    mjArray<thread> threads(nThreads);
    for (int i = 0; i < nThreads; ++i) {
        threads[i] = thread(split, i, sums[i], from, to, f, nThreads);
    }
    for (int i = 0; i < nThreads; ++i) {
        threads[i].join();
        sum += sums[i];
    }
    return sum;
}

double f(double x) {
    return (4 / (8 * x + 1) - 2 / (8 * x + 4) - 1 / (8 * x + 5) - 1 / (8 * x + 6)) / pow(16, x);
}

int main(void) {
    for (int i = 1; i <= 4; ++i) {
        time_t start = clock();
        double pi = sigma(0, 1000000, f, i);
        time_t end = clock();
        printf("pi = %.10f; nThreads = %d; elapsed = %.3fs\n", pi, i, (double)(end - start) / CLOCKS_PER_SEC);
    }
    return 0;
}

Note that it is a Bad Idea to have adjacent values in an array be updated by different threads. Although it is semantically correct to do so, you create artificial contention due to the need to synchronize cache lines, i.e., you get less concurrency than you'd be hoping for. You'd be much better off to either pad the array entries out to be on different cache lines or to create separate values, e.g., by allocating them (and later, of course, releasing them) as they are less likely to be on the same cache line. — Dietmar Kühl, Dec 17 '14 at 19:05
duplicate of http://stackoverflow.com/q/21048906/981959 and http://stackoverflow.com/q/8299545/981959 — Jonathan Wakely, Dec 17 '14 at 19:07

Piotr Skotnicki · Accepted Answer · 2014-12-17T19:20:27.623

#include <functional>

threads[i] = thread(split, i, std::ref(sums[i]), from, to, f, nThreads);
//                            ~~~~~~~~^

Rationale:

std::thread stores decayed copies of the arguments passed in to its constructor, which are then std::moved to initialize the parameters of the functor object running in that new thread. In case of references this fails, since you can't initialize a non-const lvalue reference (that double& your split function expects) with an xvalue (not to mention that it's a completely different double instance than the one you passed to thread's constructor).

The solution is to use std::reference_wrapper<T> returned from a helper function std::ref that wraps up your reference in a copyable object, which successfully transfers your reference to the newly created thread.

Your solution works exactly as expected! could you explain me a little how it works? — , Dec 17 '14 at 19:00

std::threads constructor argument error

1 Answers1