Release Acquire Semantics to Compute Average

Question

Say there are two functions to update and return the average of some property being measured:

void Class::Update( int delta )
{
    m_accumulatedValue += delta;
    ++ m_count;
}

double Class::GetAverage( )
{
    return m_accumulatedValue/(double)m_count;
}

Now, suppose they need to be changed to run in a multithreaded environment with a thread pool in which any thread can be requested to execute one of them - that is, the thread executing each one of them can be a different one each time:

std::atomic< int > m_accumulatedValue;
std::atomic< int > m_count;

// ...

void Class::Update( int delta )
{
    m_accumulatedValue.fetch_add( delta , std::memory_order_relaxed );
    m_count.fetch_add( 1 , std::memory_order_release );
}

double Class::GetAverage( )
{
    auto count = m_count.load( std::memory_order_acquire );
    auto acc = m_accumulatedValue.load( std::memory_order_relaxed );

    return acc/(double)count;
}

I'm trying to understand the acquire and release memory orderings.

Suppose there's no concurrent calls on the same object for Update(), but may be concurrent calls on the same object for Update() and GetAverage().

For what I've read, the acquire load of m_count in GetAverage() forbids the reordering of the load of m_accumulatedValue before it and at the same time guarantees that any change to m_accumulatedValue performed by Update() is visible by the thread calling GetAverage() once the change to m_count is also seen, for the store performed on m_cout by Update() has a release ordering.

Is what I've just said right?

Does GetAverage() (with the said guarantee of non-concurrency of the calls to Update()) always return the right answer? Or there can be a way of it returning the calculated average with some of the values "more updated" than the other?

Does m_accumulatedValue need to be atomic at all?

Answer 1

Your description of how acquire/release semantics work is correct; they are used to create an inter-thread happens-before relationship between memory operations before the store/release and after the load/acquire... This is based on a run-time relationship and is only defined if the atomic load/acquire sees the value that was set by the store/release.

The first problem with your code is that it fails to meet this run-time requirement. The value of m_count is not checked and therefore the ordering guarantees do not apply; you could therefore as well have used memory_order_relaxed on all operations.

But that alone does not solve the problem; when you read m_accumulatedValue, its value may have changed again by another call to Update() (m_accumulatedValue therefore has to be atomic). Futhermore, as pointed out in the comments section, since there is no atomicity between the atomic operations, GetAverage() may be called before Update() is finished and return an incorrect value.

What you need is strict ordering between Update() and GetAverage() and the best way to do that is with a std::mutex. The atomic variables can then be regular integers (if not used elsewhere).