Reading an int that's updated by Interlocked on other threads
Asked Answered
P

6

57

(This is a repeat of: How to correctly read an Interlocked.Increment'ed int field? but, after reading the answers and comments, I'm still not sure of the right answer.)

There's some code that I don't own and can't change to use locks that increments an int counter (numberOfUpdates) in several different threads. All calls use:

Interlocked.Increment(ref numberOfUpdates);

I want to read numberOfUpdates in my code. Now since this is an int, I know that it can't tear. But what's the best way to ensure that I get the latest value possible? It seems like my options are:

int localNumberOfUpdates = Interlocked.CompareExchange(ref numberOfUpdates, 0, 0);

Or

int localNumberOfUpdates = Thread.VolatileRead(numberOfUpdates);

Will both work (in the sense of delivering the latest value possible regardless of optimizations, re-orderings, caching, etc.)? Is one preferred over the other? Is there a third option that's better?

Padre answered 17/7, 2014 at 15:56 Comment(5)
This seems to be a common question, see #517363. But I'm still not clear if Thread.VolatileRead or Interlocked.CompareExchange is better or if it doesn't matter.Padre
One question worth considering: what happens next that relies on getting the 'latest' value of numberOfUpdates and how does this interact with whatever is incrementing it? What are the consequences if you read a slightly stale cached value?Nielsen
Nothing. If I get a value that's old, it would be just fine, as long as I see the right value eventually. I'm not too worried about reading the int and getting a value that's 100 ms old. I'm more concerned about something being optimized or cached in a register, etc. so that my read never sees the updated value. I don't see how that could happen. But I just figured that I should read the int in the best, most idiomatic C# way possible. And I wasn't sure if the Interlocked.CompareExchange or Thread.VolatileRead was better or if it made no difference.Padre
The best way is don't do it in the first place. Multiple threads is a bad idea. If you must have multiple threads, shared memory is a bad idea. If you must share memory, lock evasion is a bad idea. Put a lock around every single access to shared memory. If that turns out to be an unacceptable performance burden, and you can't eliminate contention, or the burden exists even with no contention, only then should you consider evaluating a low-lock solution.Demarche
I don't need the performance so there's no goal of low-lock code. Other developers are already modifying this integer using just Interlocked.* outside of any locks, so locks aren't an option for me. I just have to read this int in the best way possible, given that it's updated outside of locks (though always using Interlocked.* as far as I can see). If I was starting this from the beginning, I would just lock everywhere to make it simpler.Padre
P
56

I'm a firm believer in that if you're using interlocked to increment shared data, then you should use interlocked everywhere you access that shared data. Likewise, if you use insert you favorite synchronization primitive here to increment shared data, then you should use insert you favorite synchronization primitive here everywhere you access that shared data.

int localNumberOfUpdates = Interlocked.CompareExchange(ref numberOfUpdates, 0, 0);

Will give you exactly what your looking for. As others have said interlocked operations are atomic. So Interlocked.CompareExchange will always return the most recent value. I use this all the time for accessing simple shared data like counters.

I'm not as familiar with Thread.VolatileRead, but I suspect it will also return the most recent value. I'd stick with interlocked methods, if only for the sake of being consistent.


Additional info:

I'd recommend taking a look at Jon Skeet's answer for why you may want to shy away from Thread.VolatileRead(): Thread.VolatileRead Implementation

Eric Lippert discusses volatility and the guarantees made by the C# memory model in his blog at http://blogs.msdn.com/b/ericlippert/archive/2011/06/16/atomicity-volatility-and-immutability-are-different-part-three.aspx. Straight from the horses mouth: "I don't attempt to write any low-lock code except for the most trivial usages of Interlocked operations. I leave the usage of "volatile" to real experts."

And I agree with Hans's point that the value will always be stale at least by a few ns, but if you have a use case where that is unacceptable, its probably not well suited for a garbage collected language like C# or a non-real-time OS. Joe Duffy has a good article on the timeliness of interlocked methods here: http://joeduffyblog.com/2008/06/13/volatile-reads-and-writes-and-timeliness/

Portfire answered 22/7, 2014 at 16:50 Comment(8)
I agree with your sentiment about sticking with one synchronization primitive everywhere. And I've seen a lot of articles by people who really know what they're doing saying that volatile is confusing in terms of what it actually means. So my inclination is to stick with Interlocked for all reads. But I don't like that I have to "fake" it using Interlocked.CompareExchange(ref foo,0,0) instead of an Interlocked.Read(ref foo) that makes it clear from the code that all I want to do is read.Padre
Actually, I just looked at the BCL for Interlocked.cs. And it looks like Interlocked.Read for longs is literally just implemented with Interlocked.CompareExchange(ref location,0,0). So I guess doing Interlocked.CompareExchange(ref foo,0,0) for ints should be just fine. If there was an int Interlocked.Read, that's how it would have been implemented.Padre
@Michael +1 for checking the BCL. I've glanced at Interlocked.cs before, but never made it to the bottom of the page where Interlocked.Read is defined. I always assumed it had its own external implementation like most other members. Learn something new everyday.Portfire
This is good advice. Regarding volatile reads: a volatile read will give you the most up-to-date value available however the C# language makes no guarantees that a single canonical ordering of all volatile reads and writes will be observed by all threads. Remember, there is a big difference between "I have just read the latest value" and "that read of one variable will not be re-ordered with respect to a write of a different variable", but many people implicitly assume they are the same; they are absolutely not!Demarche
Aren't these two sentences contradictory: "As others have said interlocked operations are atomic. So Interlocked.CompareExchange will always return the most recent value."? Atomicty does not guarantee you will get the most recent value, it guarantees the value won't tear. It happens to always work on x86 and AMD64, but on ARM or IA64 you could get an old value from cache if just using Interlocked from diff thread.. Unless I am missing something?Sasaki
@Sasaki Absolutely correct. It all depends on the memory model of CPU architecture. All Interlocked does is put memory fences around operations. On an x86 CPU (which has a strong memory model) this happens to result in the latest value being returned. Try using Interlocked on architectures with weaker memory models (e.g. ARM, PowerPC) and in certain scenarios unexpected things can happen.Veiling
32-bit reads (such as int) are always atomic in .Net. That's why Interlocked.Read(ref long) exists but Interlocked.Read(ref int) doesn't. The 'firm belief' surely doesn't apply here. I'm surprised that although Timothy Shields' answer has already mentioned this, no-one here has. Surely the only question is (as @EricLippert suggests) whether to just do an ordinary read, or to use Thread.VolatileRead, no?Magnetomotive
I think I was mistaken - it looks like it makes good sense to use CompareExchange to force the expected read behaviour, particularly if targeting Mono: stackoverflow.com/a/11159244Magnetomotive
D
6

Thread.VolatileRead(numberOfUpdates) is what you want. numberOfUpdates is an Int32, so you already have atomicity by default, and Thread.VolatileRead will ensure volatility is dealt with.

If numberOfUpdates is defined as volatile int numberOfUpdates; you don't have to do this, as all reads of it will already be volatile reads.


There seems to be confusion about whether Interlocked.CompareExchange is more appropriate. Consider the following two excerpts from the documentation.

From the Thread.VolatileRead documentation:

Reads the value of a field. The value is the latest written by any processor in a computer, regardless of the number of processors or the state of processor cache.

From the Interlocked.CompareExchange documentation:

Compares two 32-bit signed integers for equality and, if they are equal, replaces one of the values.

In terms of the stated behavior of these methods, Thread.VolatileRead is clearly more appropriate. You do not want to compare numberOfUpdates to another value, and you do not want to replace its value. You want to read its value.


Lasse makes a good point in his comment: you might be better off using simple locking. When the other code wants to update numberOfUpdates it does something like the following.

lock (state)
{
    state.numberOfUpdates++;
}

When you want to read it, you do something like the following.

int value;
lock (state)
{
    value = state.numberOfUpdates;
}

This will ensure your requirements of atomicity and volatility without delving into more-obscure, relatively low-level multithreading primitives.

Diluvial answered 17/7, 2014 at 16:7 Comment(13)
But I think that Interlocked.Read() only works on longs? Not for ints.Padre
It seems like there's two issues: preventing tearing and getting the real latest value. Interlocked.Read() will solve both (as I understand it) but only for longs. I don't have the tearing issue (regardless of x86 vs. x64) since I have an int. But I still have the second issue of reading in the latest value.Padre
@MichaelCovelli I'm fairly certain there is no Interlocked.Read(ref int) exists because reading an Int32 is already always atomic at the processor level. Even reading a 64-bit int is atomic at the processor level - but only on 64-bit processors. That's why this method exists. I missed that you were using int not long. What do you mean, precisely, by "the latest value"?Diluvial
I want to make sure that I'm not reading in a cached value that's stale because the counter is being incremented on a different CPU.Padre
@MichaelCovelli Then Thread.VolatileRead is what you want to use. The read of your Int32 will already be atomic by default, and Thread.VolatileRead will ensure volatility. I updated the answer.Diluvial
But should CompareExchange also work? I've read that that will generate a full fence.Padre
The memory barriers created by VolatileRead are the opposite of what many people expect. In particular, if one thread does a VolatileWrite or interlocked write of a variable and another thread does a VolatileRead or interlocked read, then any other variables written by the first thread before the write will be visible to the second after the read.Chaparro
Also note that VolatileRead might incur a complete flushing of all core caches, somewhat hampering the performance of multithreaded code. There may be better alternatives like locking.Wart
@LasseV.Karlsen But does Interlocked.CompareExchange not incur the same complete flushing?Padre
@TimothyShields I agree that locking is a good way to go if I controlled all the code. I don't need the performance so there's no goal of low-lock code. But others are already modifying this integer using just Interlocked.* outside of any locks, so locks aren't an option for me. I just have to read this int in the best way possible, given that it's updated outside of locks (though always using Interlocked.*)Padre
@MichaelCovelli On x86 'Interlocked.CompareExchange' is implemented as 'CMPXCHG' with a full fence to prevent reordering. It will flush the store buffer but NOT the cache. The cache uses MESI to ensure that it doesn't read a dirty value. I'm not sure about other architectures.Portfire
This is the correct answer, many assume Interlocked is all they need but it only grantees atomicity which prevents the value from tearing - it does not guarantee you will get the latest value.Sasaki
According to the documentation, the Thread.VolatileRead is a legacy APIs and has been replaced by the Volatile.Read.Corbet
R
5

Will both work (in the sense of delivering the latest value possible regardless of optimizations, re-orderings, caching, etc.)?

No, the value you get is always stale. How stale the value might be is entirely unpredictable. The vast majority of the time it will be stale by a few nanoseconds, give or take, depending how quickly you act on the value. But there is no reasonable upper-bound:

  • your thread can lose the processor when it context-switches another thread onto the core. Typical delays are around 45 msec with no firm upper-bound. This does not mean that another thread in your process also gets switched-out, it can keep motoring and continue to mutate the value.
  • just like any user-mode code, your code is subjected to page-faults as well. Incurred when the processor needs RAM for another process. On a heavily loaded machine that can and will page-out active code. As sometimes happens to the mouse driver code for example, leaving a frozen mouse cursor.
  • managed threads are subject to near-random garbage collection pauses. Tends to be the lesser problem since it is likely that another thread that's mutating the value will be paused as well.

Whatever you do with the value needs to take this into account. Needless to say perhaps, that's very, very difficult. Practical examples are hard to come by. The .NET Framework is a very large chunk of battle-scarred code. You can see the cross-reference to usage of VolatileRead from the Reference Source. Number of hits: 0.

Ribwort answered 22/7, 2014 at 17:37 Comment(4)
My question was badly phrased. I'm not expecting or requiring a hard upper bound on the staleness in terms of time. Practically, even just reading the int with no Interlocked or Volatile causes no issues when I run in dev. Getting a value that's 100 milliseconds stale isn't so bad for me. I'm more concerned about some weird re-ordering or optimization that makes my read of the int get cached and not update. So I'd just like to pick the best, most idiomatic C# way to read the int, given that it is only updated with Interlocked.Increment and Interlocked.Decrement.Padre
The OP seems clear to me: at the point in time the value is read is the the most up-to-date int that it will reflect all updates made to it before that, i.e. not at the time we use the value - we all know there is no theoretical upper bound to that.Sasaki
@MichaelCovelli: That's a very different question: you just need to control the ordering of this load wrt. later loads/stores, i.e. an acquire load. You don't need to try to stop this thread's load from loading early, before earlier stores have become visible to other threads. CPU caches are coherent, so you can't get a "stale" value in that sense. See also Does hardware memory barrier make visibility of atomic operations faster in addition to providing necessary guarantees?League
Also Difference between Interlocked.Exchange and Volatile.Write?League
M
3

Well, any value you read will always be somewhat stale as Hans Passant said. You can only control a guarantee that other shared values are consistent with the one you've just read using memory fences in the middle of code reading several shared values without locks (ie: are at the same degree of "staleness")

Fences also have the effect of defeating some compiler optimizations and reordering thus preventing unexpected behavior in release mode on different platforms.

Thread.VolatileRead will cause a full memory fence to be emitted so that no reads or writes can be reordered around your read of the int (in the method that's reading it). Obviously if you're only reading a single shared value (and you're not reading something else shared and the order and consistency of them both is important), then it may not seem necessary...

But I think that you will need it anyway to defeat some optimizations by the compiler or CPU so that you don't get the read more "stale" than necessary.

A dummy Interlocked.CompareExchange will do the same thing as Thread.VolatileRead (full fence and optimization defeating behavior).

There is a pattern followed in the framework used by CancellationTokenSource http://referencesource.microsoft.com/#mscorlib/system/threading/CancellationTokenSource.cs#64

//m_state uses the pattern "volatile int32 reads, with cmpxch writes" which is safe for updates and cannot suffer torn reads. 
private volatile int m_state;

public bool IsCancellationRequested
{
    get { return m_state >= NOTIFYING; } 
}

// ....
if (Interlocked.CompareExchange(ref m_state, NOTIFYING, NOT_CANCELED) == NOT_CANCELED) {
}
// ....

The volatile keyword has the effect of emitting a "half" fence. (ie: it blocks reads/writes from being moved before the read to it, and blocks reads/writes from being moved after the write to it).

Majoriemajority answered 22/7, 2014 at 18:22 Comment(0)
C
1

It seems like my options are:

int localNumberOfUpdates = Interlocked.CompareExchange(ref numberOfUpdates, 0, 0);

Or

int localNumberOfUpdates = Thread.VolatileRead(numberOfUpdates);

Starting from the .NET Framework 4.5, there is a third option:

int localNumberOfUpdates = Volatile.Read(ref numberOfUpdates);

The Interlocked methods impose full fences, while the Volatile methods impose half fences¹. So using the static methods of the Volatile class is a potentially more economic way of reading atomically the latest value of an int variable or field.

Alternatively, if the numberOfUpdates is a field of a class, you could declare it as volatile. Reading a volatile field is equivalent with reading it with the Volatile.Read method.

I should mention one more option, which is to simply read the numberOfUpdates directly, without the help of either the Interlocked or the Volatile. We are not supposed to do this, but demonstrating an actual problem caused by doing so might be impossible. The reason is that the memory models of the most commonly used CPUs are stronger than the C# memory model. So if your machine has such a CPU (for example x86 or x64), you won't be able to write a program that fails as a result of reading directly the field. Nevertheless personally I never use this option, because I am not an expert in CPU architectures and memory protocols, nor I have the desire to become one. So I prefer to use either the Volatile class or the volatile keyword, whatever is more convenient in each case.

¹ With some exceptions, like reading/writing an Int64 or double on a 32-bit machine.

Corbet answered 5/11, 2022 at 13:24 Comment(2)
Related: Difference between Interlocked.Exchange and Volatile.Write? - AFAICT, Volatile.Write / Volatile.Read are release / acquire operations, which can create synchronization between threads without needing slow full memory barriers. (After an acquire load that sees a value from a release-store, you can safely access all other variables the other thread assigned before the release-store. e.g. access an array after seeing a data_ready flag). preshing.com/20120913/acquire-and-release-semanticsLeague
And yes it should be fine to Volatile.Read a value that also gets modified with Interlocked.Add or whatever. Just like in C++, it's safe to do foo.load(std::memory_order_acquire) as well as foo.compare_exchange_weak(). C# is weird in using different families of functions for different kinds of barriers and atomic ops, but the currently-accepted answer suggesting that one should use Interlocked (atomic RMW) for all accesses to something that's ever used with Interlocked sounds insane, or like superstition.League
V
0

Not sure why nobody mentioned Interlocked.Add(ref localNumberOfUpdates, 0), but seems the simplest way to me...

Velour answered 2/3, 2022 at 14:53 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.