SemaphoreSlim: reduce the number of false-wakeups of waiting threads

[ikopylov]

This is the issue for 1 year old PR dotnet/coreclr#137. Hopefully, it will help to move forward.

SemaphoreSlim uses PulseAll to notify all waiting threads that the Release happened. But, in fact, only few of the waiting threads will exit the lock (according to the value of semaphore currentCount). All other threads will waste the CPU resources just to check the condition and return to the Wait state. That becomes a bottleneck when the number of waiting threads is larger than the number of CPU cores.

The resolution will be to call Pulse according to the number of releaseCount and waitCount (Math.Min(releaseCount, waitCount)).

Performance comparison can be found here: dotnet/coreclr#137 (comment)
and here: dotnet/coreclr#137 (comment)

[benaadams]

I can't get the sample code to compile on coreclr as Thread.SpinWait isn't an available api. If I change it to SpinWait and SpinOnce it never completes 😿

[ikopylov]

@benaadams Originally I ran performance tests on .Net Framework 4.5.1.
It is very unfortunate that neither Thread.SpinWait nor Thread.Yield is available for CoreCLR. The rough equivalent of Thread.SpinWait would look like this:

private static void ThreadSpinWait(int iteration)
{
    SpinWait sw = new SpinWait();
    while (iteration > 0)
    {
        sw.SpinOnce();
        sw.Reset();
        iteration--;
    }
}

Also, the values for elemCount, addSpin and takeSpin can be changed so that the test completed in a reasonable amount of time.

[ikopylov]

@benaadams I dug a little deeper and found out that Thread.SpinWait uses rep nop under the hood. So, unfortunately, my implementation of ThreadSpinWait is not equivalent to the original behaviour.

The better alternative is:

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Nop() { }

private static void ThreadSpinWait(int iteration)
{
    for (int i = 0; i < iteration; i++)
    {
        Nop(); Nop(); Nop();
    }
}

[ikopylov]

Closing as the associated PR was merged