0

I'm creating a matrix class in C++ made of 2-dimensional vectors and I'm trying to use multithreading for the multiplication operation. Here are the two functions in question:

// Global variables for threading
std::vector<std::vector<double>> multiSecond;
std::vector<std::vector<double>> multiResult;
std::vector<std::vector<double>> multiFirst;
int step_i = 0;

void* multi(void*){
    int core = step_i++;

    for(int i = core; i < core + 1; i++){
        for(int j = 0; j < multiSecond[0].size(); j++){
            for(int k = 0; k < multiFirst[0].size(); k++){
                multiResult[i][j] += multiFirst[i][k] * multiSecond[k][j];
            }
        }
    }
}   
class Matrix{
private:
    int rows;
    int columns;
public:
    std::vector<std::vector<double>> matrix;

...

Matrix operator * (const Matrix &obj){
            multiResult = std::vector<std::vector<double>>
                (rows, std::vector<double>(obj.columns));
            multiSecond = obj.matrix;
            multiFirst = matrix;
            step_i = 0;
            pthread_t threads[rows];

            for(int i = 0; i < rows; i++) { 
                int* p; 
                pthread_create(&threads[i], NULL, multi, (void*)(p)); 
            }

            for(int i = 0; i < rows; i++){  
                pthread_join(threads[i], NULL);
            }
            Matrix product(multiResult);
            return product;
        }
...

In a test file I'm multiplying to matricies:

std::vector<std::vector<double>> u {{1, 2, 3}, {3, 2, 1}, {2, 1, 3}, {3, 1, 2}};
std::vector<std::vector<double>> w {{1, 2, 3}, {2, 4, 1}, {1, 3, 2}};
Matrix U(u);
Matrix W(w);
Matrix X = U * W;
X.print(3);

These are some of the results from running this multiplication:

(correct answer)
8.000 19.000 11.000 
8.000 17.000 13.000 
7.000 17.000 13.000 
7.000 16.000 14.000

15.000 27.000 19.000 
8.000 17.000 13.000 
7.000 17.000 13.000 
0.000 0.000 0.000

8.000 36.000 22.000 
8.000 17.000 13.000 
7.000 17.000 13.000 
0.000 0.000 0.000 

11.000 38.000 22.000 
8.000 17.000 13.000 
7.000 17.000 13.000 
0.000 0.000 0.000

It gives the correct answer about 90% of the time.

IDEwhatyoumean
  • 21
  • 3
  • `void* multi(void*)` -- I don't think you actually returned a value from this function, thus the behavior is undefined. If after changing `void* multi(void *)` to `void multi(void *)` things start to magically work, then possibly this question will be closed as a typo. – PaulMcKenzie Jun 16 '20 at 15:43
  • `int core = step_i++;` thats not thread safe and wont give you what you expect. Make `step_i` a `std::atomic` and it should fix this issue. – Mike Vine Jun 16 '20 at 15:47
  • The shown code fails to synchronize multiple threads, in any way. Right out of the starting gate, "`int core = step_i++;`" is not thread-safe, and comprises undefined behavior. Please [get a good C++ textbook](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) that discusses all the complicated parts of writing well-defined, correct, multithreaded code. It's not as simple as calling `pthread_create`, or `std::thread`, and you should really be using `std::thread` in modern C++. The shown code is fundamentally flawed, and won't work. – Sam Varshavchik Jun 16 '20 at 15:47
  • `pthread_t threads[rows];` -- This is also not valid C++, as arrays in C++ must have their sizes denoted by a compile-time expression, not a runtime value. You're using `std::vector` already, so that should be `std::vector threads(rows);` – PaulMcKenzie Jun 16 '20 at 15:49
  • 1
    I agree with SamV. Multithreaded programming is a lot more to it than knowing how to create threads. – PaulMcKenzie Jun 16 '20 at 15:50
  • All your threads write their results to the same global object `multiResult` at the same time. You could synchronize access to it to fix the race condition, but then you would lose any benefit from multithreading. – François Andrieux Jun 16 '20 at 15:53
  • I think some people here already pointed out flaws in your code. I'd like to give you some advise concerning your question: Firstly, take the [tour] and read [ask]. Secondly, questions like this should *always* come with a [mcve]. Actually, extract such an example when you're stuck, it's a way to analyze a problem in and of itself. – Ulrich Eckhardt Jun 16 '20 at 17:28

0 Answers0