CODEWITHSUNDEEP
c#multithreadinglockingthread-safety
Nime Cloud
Nime Cloud

Reputation: 6397

Where is the correct location to use lock

I try to find the best similarity in multithreaded environment.

Is there any better alternative or both versions are same below?

 // float bestSimilarity is shared
 // float _similarity is local

 lock(locker) 
     if (_similarity > bestSimilarity)
         bestSimilarity = _similarity;

vs

 if (_similarity > bestSimilarity)
     lock(locker) 
         bestSimilarity = _similarity;

Upvotes: 3

Views: 167

Answers (5)

Marc Gravell
Marc Gravell

Reputation: 1062770

You could also do it lock-free:

bool retry;
do
{
    retry = false;
    var copy = Interlocked.CompareExchange(ref bestSimilarity, 0, 0);

    if (_similarity > copy)
    {
        retry = Interlocked.CompareExchange(
              ref bestSimilarity, _similarity, copy) != copy;
    }
} while (retry);

This:

  • takes a snapshot of bestSimilarity at the start (which I assume is the accumulator)
  • it then compares our current value (_similarity) to the snapshot (which is stable, as a local)
  • if it is higher, it swaps in the value but only if the accumulator hasn't changed
  • if the accumulator has changed, it does the whole thing again

This is fully thread-safe, and lock-free

Upvotes: 1

Kiril Popov
Kiril Popov

Reputation: 386

The first solutions is thread safe - the second is not. However you can use double-checked lock to reduce the overhead of acquiring a lock

if (_similarity > bestSimilarity)  
{
    lock(locker) 
    {
         if (_similarity > bestSimilarity)
             bestSimilarity = _similarity;
    }
}

Upvotes: 1

vc 74
vc 74

Reputation: 38179

The second is not thread safe, another thread could change _similarity after the if test has been performed.

Upvotes: 1

Ankur
Ankur

Reputation: 33637

As bestSimilarity is shared, you will need to use the first code segment

Upvotes: 2

Polity
Polity

Reputation: 15130

Your first case will work guaranteed. The second case however might break down. You compare, then request a lock while in the meantime another thread already modifies bestSimilarity without you knowing of it making the comparison invalid.

If you want to avoid the lock untill the last minute, you can do a comparison twice. That is, compare, acquire a lock, compare again and only if its still valid, increase the value. Be carefull though with local cache of the value you're comparing against. If you want to do this, you need to have some kind of synchronization there like a MemoryBarrier. This all can get pretty complex so i recommend just lock the whole thing unless you notice performance really is a bottleneck

Upvotes: 7

Related Questions