Getting correct value from Interlocked operations?

[Anders Borum]

Hi!

I am implementing a threaded producer / consumer pattern just for fun. I am
using an internal counter to keep track of the produced / consumed items and
am logging that information. I am using the Interlocked class to increment a
set of counter.

Without placing an explicit lock around calls to the Interlocked class, how
do you get the correct value of the counter? As far as I'm concerned (thanks
to the excellent threading information on e.g. Jon Skeets site and the SDK),
the Interlocked.Increment allows for a faster threadsafe incrementation of
shared variables - and reduces the complexity of your application (which is
nice).

int counter = 0;

// in some method
Interlocked.Increment(ref counter);
Log.Write("produced # {0}.", counter);

Let's imagine that there are about 10 consuming threads running at the same
time. This could potentially lead to the situation, where the call to
Log.Write would reflect a value that's just been incremented by another
thread. The following implementation (and the simple need to actually know
what value was actually written during incrementation) seems rather silly
compared to the lightweight call to the Interlocked class.

// in some method (using the lock)
int local = 0;

lock (_lock)
{
// increment before assignning
local = ++counter;
Log.Write("produced # {0}.", local);
}

I may be missing something, but it would be nice if another counter could be
passed, effectively reflecting the incremented value. Like the following:

int counter = 0;

// in some method
int local = 0;
Interlocked.Increment(ref counter, out local);

The tests I've done show, that you have to be really unlucky if the race
condition would actually occur, but locking is all about taking luck out of
the equation (to quote Jon Skeets site) - and I agree.

Thanks in advance! :-)

--
With regards
Anders Borum / SphereWorks
Microsoft Certified Professional (.NET MCP)

[Anders Borum]

It just struck me, that I could use the Interlocked.Add method - sorry for
bothering you :-))

--
With regards
Anders Borum / SphereWorks
Microsoft Certified Professional (.NET MCP)

[Jon Skeet [C# MVP]]

Anders Borum <an****@sphereworks.dkwrote:

<snip>

I may be missing something, but it would be nice if another counter could be
passed, effectively reflecting the incremented value. Like the following:

int counter = 0;

// in some method
int local = 0;
Interlocked.Increment(ref counter, out local);

If the counter is declared to be volatile, just reading it should be
enough.

Otherwise, you could call CompareExchange:

int local = Interlocked.CompareExchange (ref counter, 0, 0);

Basically that will replace counter with the value 0 if the value is
currently 0 (i.e. it's a no-op) but it *does* return the original value
:)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Anders Borum]

Hi Jon

If the counter is declared to be volatile, just reading it should be
enough.

Yes, but I was originally after the solution to incrementing and immediately
getting the incremented value without having to issue a lock statement.
Seems like the Add() method provides the benefit of the presented solutions
with a single (and high perf) method call.

Otherwise, you could call CompareExchange:
int local = Interlocked.CompareExchange (ref counter, 0, 0);
Basically that will replace counter with the value 0 if the value is
currently 0 (i.e. it's a no-op) but it *does* return the original value

An interesting approach - however, the "volatile" approach and just reading
the variable seems a bit nicer :)

--
With regards
Anders Borum / SphereWorks
Microsoft Certified Professional (.NET MCP)

[Jon Skeet [C# MVP]]

Anders Borum <an****@sphereworks.dkwrote:

If the counter is declared to be volatile, just reading it should be
enough.

Yes, but I was originally after the solution to incrementing and immediately
getting the incremented value without having to issue a lock statement.

Declaring the variable as volatile doesn't mean you need to use a lock.
(You'll still need to use Interlocked to increment etc though.)

Seems like the Add() method provides the benefit of the presented solutions
with a single (and high perf) method call.

True - assuming you are using 2.0 :)

Otherwise, you could call CompareExchange:
int local = Interlocked.CompareExchange (ref counter, 0, 0);
Basically that will replace counter with the value 0 if the value is
currently 0 (i.e. it's a no-op) but it *does* return the original value

An interesting approach - however, the "volatile" approach and just reading
the variable seems a bit nicer :)

:)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[andyk]

Hi,

Forgive me if I am missing something here but Interlocked.Increment
returns the
incremented value.

int counter = 0;

int local = Interlocked.Increment(ref counter);

Console.WriteLine(local.ToString()); // Prints 1.

Is this not what you want?

Andy.

Anders Borum wrote:

Hi!

I am implementing a threaded producer / consumer pattern just for fun. I am
using an internal counter to keep track of the produced / consumed items and
am logging that information. I am using the Interlocked class to increment a
set of counter.

...........

>
--
With regards
Anders Borum / SphereWorks
Microsoft Certified Professional (.NET MCP)

[Jon Skeet [C# MVP]]

andyk <an*************@googlemail.comwrote:

Forgive me if I am missing something here but Interlocked.Increment
returns the
incremented value.

int counter = 0;

int local = Interlocked.Increment(ref counter);

Console.WriteLine(local.ToString()); // Prints 1.

Is this not what you want?

The point is to be able to get the value *without* changing it. Other
threads will be changing the value, but all the OP wanted was a
volatile read, effectively.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Lucian Wischik]

Jon Skeet [C# MVP] <sk***@pobox.comwrote:

>The point is to be able to get the value *without* changing it. Other
threads will be changing the value, but all the OP wanted was a
volatile read, effectively.

In this case it looked like the OP's request came out of a design
error, and the Interlocked.Add was the correct solution. The very
notion of a "true" value of the counter is meaningless in his case.

My general rule of thumb: if you want to read the value of a volatile
variable without being protected by a mutex (or other locking logic),
then you've usually made a design error.

--
Lucian

[Jon Skeet [C# MVP]]

Lucian Wischik <lu***@wischik.comwrote:

Jon Skeet [C# MVP] <sk***@pobox.comwrote:

The point is to be able to get the value *without* changing it. Other
threads will be changing the value, but all the OP wanted was a
volatile read, effectively.

In this case it looked like the OP's request came out of a design
error, and the Interlocked.Add was the correct solution. The very
notion of a "true" value of the counter is meaningless in his case.

In what way? What does Interlocked.Add (with, I assume, a value of 0)
give you that a simple read doesn't?

My general rule of thumb: if you want to read the value of a volatile
variable without being protected by a mutex (or other locking logic),
then you've usually made a design error.

Why? If you're using a lock of some description means you don't need to
have a volatile variable to start with.

It seems perfectly reasonable to me to need a "recent" value of the
variable (e.g. for reporting statistics, a progress message, whatever)
without needing a lock of any description.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[andyk]

A volatile read would not ensure this. The volatile modifier prevents
the compiler from applying certain optimisations, it would not prevent
the counter from being modified again before Log.Write.

Jon wrote:

andyk <an*************@googlemail.comwrote:

Forgive me if I am missing something here but Interlocked.Increment
returns the
incremented value.

int counter = 0;

int local = Interlocked.Increment(ref counter);

Console.WriteLine(local.ToString()); // Prints 1.

Is this not what you want?

The point is to be able to get the value *without* changing it. Other
threads will be changing the value, but all the OP wanted was a
volatile read, effectively.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Jon Skeet [C# MVP]]

andyk <an*************@googlemail.comwrote:

A volatile read would not ensure this. The volatile modifier prevents
the compiler from applying certain optimisations, it would not prevent
the counter from being modified again before Log.Write.

Yes, the counter could be modified between it being read and being
logged *but* the volatility ensures that it is the most recent value
*at the point of the read*.

Note that "volatile" doesn't mean the same thing in .NET as it does in
C.

See http://www.pobox.com/~skeet/csharp/t...latility.shtml for
more info.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Lucian Wischik]

Jon Skeet [C# MVP] <sk***@pobox.comwrote:

>In what way? What does Interlocked.Add (with, I assume, a value of 0)
give you that a simple read doesn't?

I'm saying he should ATOMICALLY increment the counter and get its
value. Any solution in which the two actions (incrementing/getting)
are non-atomic is wrong for his code.

>It seems perfectly reasonable to me to need a "recent" value of the
variable (e.g. for reporting statistics, a progress message, whatever)
without needing a lock of any description.

These only apply to when you want to display something that's
unrelated to the action you just took.

Look at the original post: he writes
Log.Write("produced # {0}.", counter);
i.e. he wants to refer to the counter that he just produced. That's
why it has to be atomic.

--
Lucian

[William Stacey [C# MVP]]

If that is all he needs, then Interlocked does this already (no need for
volatile).
After an interlocked operation completes, all threads will see the most
current value of that location.

--
William Stacey [C# MVP]

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP***********************@msnews.microsoft.co m...
| andyk <an*************@googlemail.comwrote:
| A volatile read would not ensure this. The volatile modifier prevents
| the compiler from applying certain optimisations, it would not prevent
| the counter from being modified again before Log.Write.
|
| Yes, the counter could be modified between it being read and being
| logged *but* the volatility ensures that it is the most recent value
| *at the point of the read*.
|
| Note that "volatile" doesn't mean the same thing in .NET as it does in
| C.
|
| See http://www.pobox.com/~skeet/csharp/t...latility.shtml for
| more info.
|
| --
| Jon Skeet - <sk***@pobox.com>
| http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
| If replying to the group, please do not mail me too

[Anders Borum]

Hi!

Before this gets out of hand, please let me state that the OP was a mistake;
I read the SDK but was probably hallucinating from a severe case of
pneumonia and rising fever (got to do something while in bed). When I think
about it, it was a big WTF to start programming a threaded producer /
consumer pattern with 40 degrees (c) in fever in the first.

The original question was the requirement to increment and read the
(incremented) value in a single atomic operation. The Interlocked.Add() or
Interlocked.Increment() were the obvious performant solutions.

Impressive to see such activity :-)

--
With regards
Anders Borum / SphereWorks
Microsoft Certified Professional (.NET MCP)

[Jon Skeet [C# MVP]]

William Stacey [C# MVP] <wi************@gmail.comwrote:

If that is all he needs, then Interlocked does this already (no need for
volatile).
After an interlocked operation completes, all threads will see the most
current value of that location.

How does that work then? How would the JIT know that it shouldn't cache
(eg enregister) a value?

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Jon Skeet [C# MVP]]

Lucian Wischik <lu***@wischik.comwrote:

<snip>

It seems perfectly reasonable to me to need a "recent" value of the
variable (e.g. for reporting statistics, a progress message, whatever)
without needing a lock of any description.

These only apply to when you want to display something that's
unrelated to the action you just took.

Indeed. I thought that was the case with the OP. It looks like I
misread the situation.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[William Stacey [C# MVP]]

TMK, the JIT does not come into play here. It is already compiled.
Interlocked ops require and use hw ops for this. When cpu gets one of the
instructions (i.e. CMPXCH) it does the operation atomic and ensures all
other cpus reflect the current state of that location. So reads should
always see most current data. Naturally, this is most useful for single
invariants or counters that don't really need to be a perfect snapshot.
That is my take. If different, that would be very handy info. Wouldn't it
be nice if you could interlock a bool?

--
William Stacey [C# MVP]

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP************************@msnews.microsoft.c om...
| William Stacey [C# MVP] <wi************@gmail.comwrote:
| If that is all he needs, then Interlocked does this already (no need for
| volatile).
| After an interlocked operation completes, all threads will see the most
| current value of that location.
|
| How does that work then? How would the JIT know that it shouldn't cache
| (eg enregister) a value?
|
| --
| Jon Skeet - <sk***@pobox.com>
| http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
| If replying to the group, please do not mail me too

[Jon Skeet [C# MVP]]

William Stacey [C# MVP] <wi************@gmail.comwrote:

TMK, the JIT does not come into play here. It is already compiled.
Interlocked ops require and use hw ops for this. When cpu gets one of the
instructions (i.e. CMPXCH) it does the operation atomic and ensures all
other cpus reflect the current state of that location. So reads should
always see most current data. Naturally, this is most useful for single
invariants or counters that don't really need to be a perfect snapshot.
That is my take. If different, that would be very handy info.

The hardware instruction can make the data available to all CPUs, but
that isn't going to help if a thread has, for instance, loaded the
value of a variable into a register at the start of a method and has no
need (as far as the memory model is concerned) to reread the value from
memory.

I wouldn't be surprised to find it all works fine in the x86
implementation, but I don't think it's strictly necessary based on the
memory model itself.

Wouldn't it be nice if you could interlock a bool?

I rarely need to change the state of a bool based on its current state
- a volatile bool is normally good enough.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Lucian Wischik]

Jon Skeet [C# MVP] <sk***@pobox.comwrote:

>William Stacey [C# MVP] <wi************@gmail.comwrote:

>Wouldn't it be nice if you could interlock a bool?

I rarely need to change the state of a bool based on its current state
- a volatile bool is normally good enough.

That's an interesting point! There exists only a single useful
interlocked operation on booleans, and that operation is "negate".

(i.e. as you say, interlocked is for when you're changing the state of
a thing based on its current state. There are four possible unary
boolean functions:
fun1(bool b) {return true;}
fun2(bool b) {return false;}
fun3(bool b) {return b;}
fun4(bool b) {return !b;}
The first two ignore the current state, and can be accomplished just
with assignment to a volatile variable. The third one doesn't do
anything. So it's only the fourth that needs interlock.)

Anyway, it's beside the point because InterlockedAnd/Or/Xor accomplish
functions 1,2,4.

--
Lucian

[William Stacey [C# MVP]]

| Anyway, it's beside the point because InterlockedAnd/Or/Xor accomplish
| functions 1,2,4.

That would work if those where exposed in the framework. It would be a
simple thing (I would think) for them to wrap that, however, and expose a
typed bool api.

[Lucian Wischik]

"William Stacey [C# MVP]" <wi************@gmail.comwrote:

>That would work if those where exposed in the framework. It would be a
simple thing (I would think) for them to wrap that, however, and expose a
typed bool api.

Or, for Interlocked.Xor, you could use Interlocked.Increment and
examine only the lowest bit...

--
Lucian

[William Stacey [C# MVP]]

| The hardware instruction can make the data available to all CPUs, but
| that isn't going to help if a thread has, for instance, loaded the
| value of a variable into a register at the start of a method and has no
| need (as far as the memory model is concerned) to reread the value from
| memory.

But now we talking about a different issue - no?

--
William Stacey [C# MVP]

[William Stacey [C# MVP]]

I would think in most cases you would still want an atomic test and set. As
many times you want to know the value before setting it - atomically.
Otherwise you will need a read outside of the interlocked, which could
result in a race we are trying to avoid. So I think you still need two
methods to round the api out:

Interlocked.Exchange<bool>(ref value, T value) // unconditional set.
Interlocked.CompareExchange<bool>(ref T value, T comp, T value); //
conditional set.

So you could:

private bool started = true;
public void Stop()
{
// Set started to false if true. Set exactly once.
if ( Interlocked.CompareExchange<bool>(ref this.started, false,
true) )
return;
throw new Exception("Already stopped");
}

--
William Stacey [C# MVP]

"Lucian Wischik" <lu***@wischik.comwrote in message
news:1c********************************@4ax.com...
| "William Stacey [C# MVP]" <wi************@gmail.comwrote:
| >That would work if those where exposed in the framework. It would be a
| >simple thing (I would think) for them to wrap that, however, and expose a
| >typed bool api.
|
| Or, for Interlocked.Xor, you could use Interlocked.Increment and
| examine only the lowest bit...
|
| --
| Lucian

[Jon Skeet [C# MVP]]

William Stacey [C# MVP] <wi************@gmail.comwrote:

| The hardware instruction can make the data available to all CPUs, but
| that isn't going to help if a thread has, for instance, loaded the
| value of a variable into a register at the start of a method and has no
| need (as far as the memory model is concerned) to reread the value from
| memory.

But now we talking about a different issue - no?

Not that I'm aware of. I was talking about making sure that when you
read a value, it was correct at that point in time. If the read
occurred a long time ago and then was just enregistered, that isn't the
case. With a volatile variable, however, the read will always get the
"latest" value.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too