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

Question

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();
        }

    }
}

Why don't you use a concurrent hash set-like data type, see https://stackoverflow.com/a/18923091/7565574? These are optimized for concurrent access often in such a way that they don't even need locks. — ckuri, Aug 08 '18 at 16:01
What's wrong with a `lock`? Have you looked at the `SemaphoreSlim` class? This is a good alternative and much easier to reason about. — tigerswithguitars, Aug 08 '18 at 16:02
@ckuri in my code, this is a add only scenario and concurrent dictionary is optimized for read, i have no gain to use it — Fredou, Aug 08 '18 at 16:04
@tigerswithguitars looking at the code of that https://github.com/Microsoft/referencesource/blob/master/mscorlib/system/threading/SemaphoreSlim.cs it use a mix of spinwait & lock & moniter, a bit heavy — Fredou, Aug 08 '18 at 16:11
For a bit of clarity, would be helpful to have a bit more detail about why a `lock` is not applicable and `SemaphoreSlim` is too heavy. Seems the use case is important and this is an interesting question. — tigerswithguitars, Aug 08 '18 at 16:13
@tigerswithguitars i'm trying to gain as much as optimization as possible — Fredou, Aug 08 '18 at 16:26

score -1 · Accepted Answer · answered Aug 08 '18 at 17:34

-1

The condition used in SpinWait.SpinUntil is wrong.

Interlocked.CompareExchange returns the original value of the variable.
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.

answered Aug 08 '18 at 17:34

Adam Simon

2,762
16
22

the way I'm using the CompareExchange is the proper way to simulate a lock, other online example simply do a != original value, I do a == new value instead, if i was going to go with == 0, it would / is blow up in a fraction of a seconds since it would be like there was no lock in place, doing the Exchange directly would also not do what I am expecting which is stop if there is another thread working on that part of the code. for the use of spinwait, it is clearly identified that it should be used for **VERY SHORT** period of time, the operation, add to hashset, should fall into that category – Fredou Aug 08 '18 at 19:15
CompareExchange returns the **original value**, the value before change, if any. CompareExchange returning 0 means that the flag was 0 and has been changed to 1 during the current call. In any other cases, CompareExchange returns 1, which means "the lock is taken". – Adam Simon Aug 08 '18 at 20:05
@Fredou Try [this code](https://gist.github.com/adams85/06dc084c4b368624a935aac92fb7c77f). I hope this will convince you. – Adam Simon Aug 08 '18 at 20:29
this code example doesn't do real multi thread, just put a loop inside the task surrounding the whole code, one task will run X amount of time then the second one will start for X amount of time, it wont switch between thread. – Fredou Aug 08 '18 at 23:52
but your right, for the SpinUntil, the compareExchange is supose to be the reverse of the instance of a SpintWait logic which caused my confusion – Fredou Aug 09 '18 at 00:14
@Fredou The code example is just for demonstrating that the lock construction works as expected. – Adam Simon Aug 09 '18 at 09:08

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

1 Answers1