Thread safety of dictionary indexer vs double check (.net 2.0)

[cst]

i am using .net 2.0. which should have fixed the problem making double
check locking broken...
( http://www.bluebytesoftware.com/blog/PermaLink,guid,543d89ad-8d57-4a51-b7c9-a821e3992bf6.aspx
)
this should make the code thread safe, and would make the code perform
nicely... only taking the lock when needed, but is the indexer safe to
call (2).. and further, will ContainsKey() return a reliable value in
(1)?

private volatile static Dictionary<int, int> typeOrder = new
Dictionary<int, int>();
private static readonly object typeOrderLock = new object();

internal static int GetTypeOrder(int typeId) {
if (!typeOrder.ContainsKey(typeId))
{ // (1)
lock (typeOrderLock) {
if (!typeOrder.ContainsKey(typeId)) {
// code simpified...
typeOrder.Add(typeId, 9);
}
}
}
// is it ok to deliver results from the dictionary while
there may be added items by other threads?
return
typeOrder[typeId]; // (2)
}

 

[Jon Skeet [C# MVP]]

i am using .net 2.0. which should have fixed the problem making double
check locking broken...
(http://www.bluebytesoftware.com/blog/PermaLink,guid,543d89ad-8d57-4a5...
)
this should make the code thread safe, and would make the code perform
nicely... only taking the lock when needed, but is the indexer safe to
call (2).. and further, will ContainsKey() return a reliable value in
(1)?

You seem to be ignoring the summary at the end of Joe's post:

<quote>
Rather, it should be that once you venture even slightly outside of
the
bounds of the few "blessed" lock-free practices mentioned in the
article
mentioned above, you are opening yourself up to the worst kind of race
conditions. Using locks is a simple way to avoid this pain.
</quote>

Have you tried just locking for the whole method? If so, have you
identified this to be a definite bottleneck?

If the answer to either of these questions is "no" you should go back
to the simplest code before trying to work lock-free.

Jon

 

[cst]

You seem to be ignoring the summary at the end of Joe's post:

<quote>
Rather, it should be that once you venture even slightly outside of
the
bounds of the few "blessed" lock-free practices mentioned in the
article
mentioned above, you are opening yourself up to the worst kind of race
conditions. Using locks is a simple way to avoid this pain.
</quote>

Have you tried just locking for the whole method? If so, have you
identified this to be a definite bottleneck?

If the answer to either of these questions is "no" you should go back
to the simplest code before trying to work lock-free.

Jon

Thank you for your answer.
The code is a template for a lot of things we cache in static memory
once the application is running, we have about 30 "instances" of this
kind of code (considering a factory for this...) so it would would be
nice to do it lock-free as the application typically runs on large
multi processor machines.
Though, i must admit, i havent actually found a problem with the
locking code, but if it can be safely done lock-free i would prefer
it...
Any performance gain is welcome

 

[Jon Skeet [C# MVP]]

Thank you for your answer.
The code is a template for a lot of things we cache in static memory
once the application is running, we have about 30 "instances" of this
kind of code (considering a factory for this...) so it would would be
nice to do it lock-free as the application typically runs on large
multi processor machines.
Though, i must admit, i havent actually found a problem with the
locking code, but if it can be safely done lock-free i would prefer
it...
Any performance gain is welcome

Any readability gain is more welcome, IMO. Suppose you manage to get
it lock-free (which I suspect you won't be able to without diving into
the code of Dictionary<,> to see the implementation details). Then a
maintenance engineer looks at the code and needs to make a change.
What are the chances it'll be changed in a correct way, without the
engineer having to do a load more research himself?

How many times a second are you expecting to make this kind of call?

There's often a balance between simplicity (of code and understanding)
and performance: I pretty much *always* pick simplicity until I *know*
there's a performance problem. I've seen far more examples of
complexity being a problem than performance!

Jon

 

[cst]

Any readability gain is more welcome, IMO. Suppose you manage to get
it lock-free (which I suspect you won't be able to without diving into
the code of Dictionary<,> to see the implementation details). Then a
maintenance engineer looks at the code and needs to make a change.
What are the chances it'll be changed in a correct way, without the
engineer having to do a load more research himself?

How many times a second are you expecting to make this kind of call?

There's often a balance between simplicity (of code and understanding)
and performance: I pretty much *always* pick simplicity until I *know*
there's a performance problem. I've seen far more examples of
complexity being a problem than performance!

Jon

I agree with you to a degree, setting up memory barriers or using low-
lock techniques, is not an option in my opinion, to complex and
unreadable. But the code construct above is relatively simple and
readable, so if it works, it would be great.

There are lots of posts and blogs concerning the "right" singleton
implementation, and the thread safety of generic collections, but, no
clear answers... So the question is also interesting from an
theoretically point of view, and the implementation of dictionary<,>,
would be interesting to se! Is it available somewhere?

 

[Jon Skeet [C# MVP]]

I agree with you to a degree, setting up memory barriers or using low-
lock techniques, is not an option in my opinion, to complex and
unreadable. But the code construct above is relatively simple and
readable, so if it works, it would be great.

It's fairly simple and in some senses readable (although not as simple
or readable as without the double-checking, IMO) but the problem is
that it's very hard to reason about, especially without the code for
ContainsKey. The fact that it's not obvious whether or not it's thread-
safe is a big knock to its readability, IMO.

There are lots of posts and blogs concerning the "right" singleton
implementation, and the thread safety of generic collections, but, no
clear answers... So the question is also interesting from an
theoretically point of view,

From an academic point of view it's certainly interesting, although

I'm afraid I have no answers.

and the implementation of dictionary<,>,
would be interesting to se! Is it available somewhere?

Well, there's Reflector, and there may be an implementation in Rotor,
but MS doesn't publish the source code to the .NET framework
libraries, unfortunately.

Jon

 

[Chris Mullins [MVP]]

Dude, you're nuts. Lock free code is very bug prone (!), and often not any
faster than the locking version of the code.

Write your code using locks, until a profiler tells you that it's the
bottleneck. Even then, leave it with a lock most of the time.

If nothing else, your code is doing 3 lookups into a hashtable for every
single add. That can't be a good thing - in the general case, that's
probably going to have more impact than the locking / non-locking code
(contention is generally pretty rare).

You should also look at "TryGetValue(key, out value)" as a method. This
eliminates one of the hash lookups.