Effective placement of lock_guard - from paragraph 16 of "Effective Modern C ++"

Question

In clause 16: “Make streams of constant member functions” has the following code:

class Widget {
public:    
  int magicValue() const
  {
    std::lock_guard<std::mutex> guard(m);  // lock m    
    if (cacheValid) return cachedValue;
    else {
      auto val1 = expensiveComputation1();
      auto val2 = expensiveComputation2();
      cachedValue = val1 + val2;
      cacheValid = true;
      return cachedValue;
    }
  }                                        // unlock m    
private:
  mutable std::mutex m;
  mutable int cachedValue;                 // no longer atomic
  mutable bool cacheValid{ false };        // no longer atomic
};

I wonder why std :: lock_guard should always be executed on every call to magicValue (), wouldn't it do the job as expected ?:

class Widget {
public:

  int magicValue() const
  {

    if (cacheValid) return cachedValue;
    else {
      std::lock_guard<std::mutex> guard(m);  // lock m
      if (cacheValid) return cachedValue;          
      auto val1 = expensiveComputation1();
      auto val2 = expensiveComputation2();
      cachedValue = val1 + val2;
      cacheValid = true;
      return cachedValue;
    }
  }                                        // unlock m

private:
  mutable std::atomic<bool>  cacheValid{false};
  mutable std::mutex m;
  mutable int cachedValue;                 // no longer atomic
};

Thus, fewer mutex locks will be required, which will make the code more efficient. I guess atomics are always faster than mutexes.

[edit]

For completeness, I measured the effectiveness of both a priori, and the second looks only 6% faster: http://coliru.stacked-crooked.com/a/e8ce9c3cfd3a4019

Answer 1

Double Checked Locking Pattern (DCLP) () , Meyers, mutex cachedValue.

, .

, , cacheValid atomic, cachedValue. , cachedValue ( mutex) , magicValue(). cacheValid "bool", cacheValid, cachedValue ( undefined ++ 11).

cacheValid , /. , atomic :

int Widget::magicValue() const
{

  if (cacheValid.load(std::memory_order_acquire)) return cachedValue;
  else {
    std::lock_guard<std::mutex> guard(m);  // lock m
    if (cacheValid.load(std::memory_order_relaxed)) return cachedValue;
    auto val1 = expensiveComputation1();
    auto val2 = expensiveComputation2();
    cachedValue = val1 + val2;
    cacheValid.store(true, std::memory_order_release);
    return cachedValue;
  }
}

, , atomic ( , ).

, ; cacheValid . .

Answer 2

, , , : , cacheValid , , .

mutex cachedValue. cachedValue . , , . , , cacheValid . cacheValid , ; cacheValid ( , , cacheValid , ).

, - , : Widget::invalidateCache(). , cacheValid false. , invalidateCache magicValue , . , ( , ), , , . :

• Thread 1 magicValue cacheValid. . , .
• Thread 2 invalidateCache, magicValue. magicValue , , , cacheValid.
• 1, cacheValid.

, , int 32 , , , 32- . , "" cachedValue. , , (, 64 ), . , magicValue , , , - , .

, , . , , , , , .