Fredou
Fredou

Reputation: 20100

trying to replace a lock() with a SpinWait.SpinUntil() but it doesnt work

let start with the code;

checkedUnlock is an HashSet<ulong>

_hashsetLock is an object

lock (_hashsetLock)
    newMap = checkedUnlock.Add(uniqueId);

vs

fun in an int

SpinWait.SpinUntil(() => Interlocked.CompareExchange(ref fun, 1, 0) == 1);
newMap = checkedUnlock.Add(uniqueId);
fun = 0;

my understanding is the SpinWait in this scenario should work like the lock() but there is more items added in the HashSet, sometime it match lock, sometime there is 1 to 5 more items in it, which make it obvious that it doesnt work

is my understanding flawed?

edit

I tried this and it seem to work, my test show the same number as lock() so far

SpinWait spin = new SpinWait();
while (Interlocked.CompareExchange(ref fun, 1, 0) == 1)
   spin.SpinOnce();

so why would it work with this but not SpinWait.SpinUntil() ?

edit #2

small full application to see

in this code, the SpinWait.SpinUntil will sometime blow up (the add will throw an exception) but when it work, the count will be different so my expected behavior for this one is wrong

using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            var list = new List<int>();
            var rnd = new Random(42);
            for (var i = 0; i < 1000000; ++i)
                list.Add(rnd.Next(500000));



            object _lock1 = new object();
            var hashset1 = new HashSet<int>();

            int _lock2 = 0;
            var hashset2 = new HashSet<int>();

            int _lock3 = 0;
            var hashset3 = new HashSet<int>();

            Parallel.ForEach(list, item =>

            {
                /******************/
                lock (_lock1)
                    hashset1.Add(item);
                /******************/

                /******************/
                SpinWait.SpinUntil(() => Interlocked.CompareExchange(ref _lock2, 1, 0) == 1);

                hashset2.Add(item);

                _lock2 = 0;
                /******************/

                /******************/
                SpinWait spin = new SpinWait();
                while (Interlocked.CompareExchange(ref _lock3, 1, 0) == 1)
                    spin.SpinOnce();

                hashset3.Add(item);

                _lock3 = 0;
                /******************/

            });


            Console.WriteLine("Lock: {0}", hashset1.Count);
            Console.WriteLine("SpinWaitUntil: {0}", hashset2.Count);
            Console.WriteLine("SpinWait: {0}", hashset3.Count);

            Console.ReadKey();
        }

    }
}

Upvotes: -1

Views: 1082

Answers (1)

Adam Simon
Adam Simon

Reputation: 2970

The condition used in SpinWait.SpinUntil is wrong.

  1. Interlocked.CompareExchange returns the original value of the variable.
  2. MSDN docs of SpinWait.SpinUntil says, condition is

A delegate to be executed over and over until it returns true.

You want to spin until a 0 -> 1 transition occurs, so the condition should be

Interlocked.CompareExchange(ref fun, 1, 0) == 0

Subsequent calls to CompareExchange on other threads results in 1, so they will wait until the fun flag is restored to 0 by the "winner" thread.

Some further remarks:

  • fun = 0; should work on x86 architecture, but I'm not sure it's correct everywhere. If you use Interlocked to access a field, it's a best practice to use Interlocked for all access to that field. So I suggest Interlocked.Exchange(ref fun, 0) instead.
  • SpinWait is rarely a good solution regarding performance as it prevents the OS putting the spinning thread into an idle state. It should be used for very short waits only. (An example of a proper usage). Simple locks (aka Monitor.Enter/Exit) or SemaphoreSlim will do in general or you can consider ReaderWriterLockSlim if # of reads >> # of writes.

Upvotes: -1

Related Questions