1

Let's say I have the following code snippet.

// Some function decleration
void generateOutput(const MyObj1& in, MyObj2& out);

void doTask(const std::vector<MyObj1>& input, std::vector<MyObj2>& output) {

    output.resize(input.size());

    // Use OpenMP to run in parallel

#pragma omp parallel for
    for (size_t i = 0; i < input.size(); ++i) {
        generateOutput(input[i], output[i]);
    }

}

Is the above threasafe? I am mainly concerned about writing to output[i]. Do I need some sort of locking? Or is it unnecessary? ex:


// Some function prototype
void generateOutput(const MyObj1& in, MyObj2& out);

void doTask(const std::vector<MyObj1>& input, std::vector<MyObj2>& output) {

    output.resize(input.size());

    // Use OpenMP to run in parallel

#pragma omp parallel for
    for (size_t i = 0; i < input.size(); ++i) {
        MyObj2 tmpOutput;
        generateOutput(input[i], tmpOutput);
#pragma omp critical
        output[i] = std::move(tmpOutput);
    }

}

I am not worried about the reading portion. As mention in this answer, it looks like reading input[i] is threadsafe.

cyrusbehr
  • 1,100
  • 1
  • 12
  • 32

3 Answers3

3

output[i] does not write to output. This is just a call to std::vector<MyObj2>::operator[]. It returns an unnamed MyObj2&, which is then used to call generateOutput. The latter is where the write happens.

I'll assume that generateOutput is threadsafe itself, and MyObj2 too, since we don't have code for that. So the write to MyObj2& inside generateOutput is also threadsafe.

As a result, all parts are threadsafe.

MSalters
  • 173,980
  • 10
  • 155
  • 350
1

To not do any assumption on the implementation of std::vector you can modify your code as below to make it threadsafe (pointer addresses will by definition point on different zones in memory and hence be thread safe)

// Some function decleration
void generateOutput(const MyObj1& in, MyObj2 *out); // use raw data pointer for output

void doTask(const std::vector<MyObj1>& input, std::vector<MyObj2>& output) {

    output.resize(input.size());

    // Use OpenMP to run in parallel


    auto data = output.data() ;// pointer on vector underlying data outside of OMP threading

    #pragma omp parallel for
    for (size_t i = 0; i < input.size(); ++i) {
        generateOutput(input[i], &data[i]); // access to distinct data elements  ie addresses (indexed by i only in from each omp thred)
    }

}
Jean-Marc Volle
  • 3,113
  • 1
  • 16
  • 20
  • How are pointers in this situation any different than references? You don't need to make an assumption about `std::vector` implementation. The safe access to different elements in containers originates right from the standard (26.2.2.2, C++17) – Zulan Apr 29 '20 at 09:38
  • Hello. A pointer directly references a address in memory so if the pointers differ you known there is no risk in concurrent access to the same content in memory. Since the only usage of vector methods is done outside of the multithreaded loop you know that it is safe whatever the way std::vector[i] is implemented. I agree however that the standard says it is safe so reference or pointer usage are equivalent provided compiler/standard implementation have no bug – Jean-Marc Volle Apr 29 '20 at 11:16
  • 1) Just because the pointer values are different, does **not** imply thread safe access (aliasing, OpenMP memory model). 2) There is no difference between a pointer and a reference that has any impact on this particular situation (see https://stackoverflow.com/a/57492/620382) – Zulan Apr 29 '20 at 12:03
  • 1
    There is no pointer aliasing possible in the provided example. I am just answering on the example. I'm not speaking about reference vs pointer I"m speaking about usage or not of a vector method from within an openmp thread. If no std::vector method is called from the loop then the access to the std::vector data is safe in the context of the provided example. – Jean-Marc Volle Apr 29 '20 at 12:37
  • Ok, I get the point now, thanks for clarifying. P.S. you need to swap `#pragma omp parallel for` and `data = ..` and add `;` for the code to be correct. – Zulan Apr 29 '20 at 17:09
  • @Zulan. Thanks for seeing the typos I corrected them and added explaination as code comments – Jean-Marc Volle Apr 30 '20 at 09:55
1

As long as it is guaranteed that the threads operate on completely separate items (i.e., no item is accessed by different different threads without some kind of synchronization) this is safe.

Since you are using a simple parallel for loop, in which each item is accessed exactly once, this is safe.

mpoeter
  • 2,574
  • 1
  • 5
  • 12