Skip to content

Commit

Permalink
Fix Lock spin-waiting on single-proc machines (#101513)
Browse files Browse the repository at this point in the history
* Fix Lock spin-waiting on single-proc machines

Small fix to check for a single proc during initialization. Also renamed things referring to "minSpinCount" to clarify it a bit.
  • Loading branch information
kouvel committed May 2, 2024
1 parent 71e9c46 commit 2d2a5a2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public sealed partial class Lock
private static int s_staticsInitializationStage;
private static bool s_isSingleProcessor;
private static short s_maxSpinCount;
private static short s_minSpinCount;
private static short s_minSpinCountForAdaptiveSpin;

/// <summary>
/// Initializes a new instance of the <see cref="Lock"/> class.
Expand Down Expand Up @@ -202,7 +202,7 @@ private static bool TryInitializeStatics()
// here. Initialize s_isSingleProcessor first, as it may be used by other initialization afterwards.
s_isSingleProcessor = RuntimeImports.RhGetProcessCpuCount() == 1;
s_maxSpinCount = DetermineMaxSpinCount();
s_minSpinCount = DetermineMinSpinCount();
s_minSpinCountForAdaptiveSpin = DetermineMinSpinCountForAdaptiveSpin();
}

// Also initialize some types that are used later to prevent potential class construction cycles. If
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace System.Threading
public sealed partial class Lock
{
private static readonly short s_maxSpinCount = DetermineMaxSpinCount();
private static readonly short s_minSpinCount = DetermineMinSpinCount();
private static readonly short s_minSpinCountForAdaptiveSpin = DetermineMinSpinCountForAdaptiveSpin();

/// <summary>
/// Initializes a new instance of the <see cref="Lock"/> class.
Expand Down
50 changes: 34 additions & 16 deletions src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,15 @@ public sealed partial class Lock

private uint _state; // see State for layout
private uint _recursionCount;

// This field serves a few purposes currently:
// - When positive, it indicates the number of spin-wait iterations that most threads would do upon contention
// - When zero, it indicates that spin-waiting is to be attempted by a thread to test if it is successful
// - When negative, it serves as a rough counter for contentions that would increment it towards zero
//
// See references to this field and "AdaptiveSpin" in TryEnterSlow for more information.
private short _spinCount;

private ushort _waiterStartTimeMs;
private AutoResetEvent? _waitEvent;

Expand Down Expand Up @@ -297,7 +305,7 @@ private void ExitImpl()
}
}

private static bool IsAdaptiveSpinEnabled(short minSpinCount) => minSpinCount <= 0;
private static bool IsAdaptiveSpinEnabled(short minSpinCountForAdaptiveSpin) => minSpinCountForAdaptiveSpin <= 0;

[MethodImpl(MethodImplOptions.NoInlining)]
private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)
Expand Down Expand Up @@ -350,20 +358,19 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)
goto Locked;
}

bool isSingleProcessor = IsSingleProcessor;
short maxSpinCount = s_maxSpinCount;
if (maxSpinCount == 0)
{
goto Wait;
}

short minSpinCount = s_minSpinCount;
short minSpinCountForAdaptiveSpin = s_minSpinCountForAdaptiveSpin;
short spinCount = _spinCount;
if (spinCount < 0)
{
// When negative, the spin count serves as a counter for contentions such that a spin-wait can be attempted
// periodically to see if it would be beneficial. Increment the spin count and skip spin-waiting.
Debug.Assert(IsAdaptiveSpinEnabled(minSpinCount));
Debug.Assert(IsAdaptiveSpinEnabled(minSpinCountForAdaptiveSpin));
_spinCount = (short)(spinCount + 1);
goto Wait;
}
Expand All @@ -388,7 +395,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)

for (short spinIndex = 0; ;)
{
LowLevelSpinWaiter.Wait(spinIndex, SpinSleep0Threshold, isSingleProcessor);
LowLevelSpinWaiter.Wait(spinIndex, SpinSleep0Threshold, isSingleProcessor: false);

if (++spinIndex >= spinCount)
{
Expand All @@ -405,7 +412,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)

if (tryLockResult == TryLockResult.Locked)
{
if (isFirstSpinner && IsAdaptiveSpinEnabled(minSpinCount))
if (isFirstSpinner && IsAdaptiveSpinEnabled(minSpinCountForAdaptiveSpin))
{
// Since the first spinner does a full-length spin-wait, and to keep upward and downward changes to the
// spin count more balanced, only the first spinner adjusts the spin count
Expand All @@ -426,7 +433,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)

// Unregister the spinner and try to acquire the lock
tryLockResult = State.TryLockAfterSpinLoop(this);
if (isFirstSpinner && IsAdaptiveSpinEnabled(minSpinCount))
if (isFirstSpinner && IsAdaptiveSpinEnabled(minSpinCountForAdaptiveSpin))
{
// Since the first spinner does a full-length spin-wait, and to keep upward and downward changes to the
// spin count more balanced, only the first spinner adjusts the spin count
Expand All @@ -444,7 +451,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)
// number of contentions, the first spinner will attempt a spin-wait again to see if it is effective.
Debug.Assert(tryLockResult == TryLockResult.Wait);
spinCount = _spinCount;
_spinCount = spinCount > 0 ? (short)(spinCount - 1) : minSpinCount;
_spinCount = spinCount > 0 ? (short)(spinCount - 1) : minSpinCountForAdaptiveSpin;
}
}

Expand Down Expand Up @@ -517,7 +524,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)
break;
}

LowLevelSpinWaiter.Wait(spinIndex, SpinSleep0Threshold, isSingleProcessor);
LowLevelSpinWaiter.Wait(spinIndex, SpinSleep0Threshold, isSingleProcessor: false);
}

if (acquiredLock)
Expand Down Expand Up @@ -661,14 +668,25 @@ internal nint LockIdForEvents

internal ulong OwningThreadId => _owningThreadId;

private static short DetermineMaxSpinCount() =>
AppContextConfigHelper.GetInt16Config(
"System.Threading.Lock.SpinCount",
"DOTNET_Lock_SpinCount",
DefaultMaxSpinCount,
allowNegative: false);
private static short DetermineMaxSpinCount()
{
if (IsSingleProcessor)
{
return 0;
}

return
AppContextConfigHelper.GetInt16Config(
"System.Threading.Lock.SpinCount",
"DOTNET_Lock_SpinCount",
DefaultMaxSpinCount,
allowNegative: false);
}

private static short DetermineMinSpinCount()
// When the returned value is zero or negative, indicates the lowest value that the _spinCount field will have when
// adaptive spin chooses to pause spin-waiting, see the comment on the _spinCount field for more information. When the
// returned value is positive, adaptive spin is disabled.
private static short DetermineMinSpinCountForAdaptiveSpin()
{
// The config var can be set to -1 to disable adaptive spin
short adaptiveSpinPeriod =
Expand Down

0 comments on commit 2d2a5a2

Please sign in to comment.