CODEWITHSUNDEEP
c++multithreadingthread-safetyopenmp
user12005284
user12005284

Reputation: 17

Locking nested function with thread-unsafe static variable

I have a function memoize that reads and writes a static std::map, eg:

int memoize(int i)
{
static std::map<int,int> map;
const auto iterator = memoizer.find(i);
if (iterator == memoizer.end())
  memoizer[i]=i+1;
else
  return iterator->second;
}

memoize is called by other functions which are called by other functions which are called.... by functions in main. Now if in main I have something like

#pragma omp parallel for
for (int i=0; i<1000; i+++)
  f(i); \\function calling memoize

for (int i=0; i<1000; i+++)
  g(i); \\function calling memoize

then I have a problem in the first loop since std::map is not thread safe. I am trying to figure out a way to lock the static map only if openmp is used (thus only for the first loop). I would rather avoid having to rewrite all functions to take an extra omp_lock_t argument.

What's the best way to achieve that? Hopefully with the least amount of macros possible.

Upvotes: 0

Views: 238

Answers (1)

Gilles
Gilles

Reputation: 9519

A very simple solution to your problem would be to protect both the read and the update parts of your code with a critical OpenMP directive. Of course, to reduce the risk of unwanted interaction/collision with some other critical you might already have somewhere else in your code, you should give it a name to identify it clearly. And should your OpenMP implementation permit it (ie the version of the standard being high enough), and also if you have the corresponding knowledge, you can add a hint on how much of contention you expect from this update among the threads for example.

Then, I would suggest you to check whether this simple implementation gives you sufficient performance, meaning whether the impact of the critical directive isn't too much performance-wise. If you're happy with the performance, then just leave it as is. If not, then come back with a more precise question and we'll see what can be done.

For the simple solution, the code could look like this (with here a hint suggesting that high contention is expected, which is only a example for your sake, not a true expectation of mine):

int memoize( int i ) {
    static std::map<int,int> memoizer;
    bool found = false;
    int value;
    #pragma omp critical( memoize ) hint( omp_sync_hint_contended )
    {
        const auto it = memoizer.find( i );
        if ( it != memoizer.end() ) {
           value = it->second;
           found = true;
        }
    }
    if ( !found ) {
        // here, we didn't find i in memoizer, so we compute it
        value = compute_actual_value_for_memoizer( i );
        #pragma omp critical( memoize ) hint( omp_sync_hint_contended )
        memoizer[i] = value;
    }
    return value;
}

Upvotes: 3

Related Questions