OpenMP - "#pragma omp critical" importance

1
votes

So I started using OpenMP (multithreading) to increase the speed of my matrix multiplication and I witnessed weird things: when I turn off OpenMP Support (in Visual Studio 2019) my nested for-loop completes 2x faster. So I removed "#pragma omp critical" to test if it slows down the proccess significantly and the proccess went 4x faster than before (with OpenMP Support On).

Here's my question: is "#pragma omp critical" important in nested loop? Can't I just skip it?

        #pragma omp parallel for collapse(3)
        for (int i = 0; i < this->I; i++)
        {
            for (int j = 0; j < A.J; j++)
            {
                m.matrix[i][j] = 0;
                for (int k = 0; k < A.I; k++)
                {   
                        #pragma omp critical
                        m.matrix[i][j] += this->matrix[i][k] * A.matrix[k][j];                                            
                }
            }
        }
2
c++multithreadingperformanceparallel-processingopenmp

2 Answers

4
votes

Here's my question: is "#pragma omp critical" important in nested loop? Can't I just skip it?

If the matrices m, this and A are different you do not need any critical region. Instead, you need to ensure that each thread will write to a different position of the matrix m as follows:

       #pragma omp parallel for collapse(2)
        for (int i = 0; i < this->I; i++)
        {
            for (int j = 0; j < A.J; j++)
            {
                m.matrix[i][j] = 0;
                for (int k = 0; k < A.I; k++)
                {   
                    m.matrix[i][j] += this->matrix[i][k] * A.matrix[k][j];                                            
                }
            }
        }

The collapse clause will assign to each thread a different pair (i, j) therefore there will not be multiple threads writing to the same position of the matrix m (i.e., race-condition).

2
votes

#pragma omp critical is necessary here, as there is a (remote) chance that two threads could write to a particular m.matrix[i][j] value. It hurts performance because only one thread at a time can access that protected assignment statement.

This would likely be better without the collapse part (then you can remove the #pragma omp critical). Accumulate the sums to a temporary local variable, then store it in m.matrix[i][j] after the k loop finishes.