From c116c162a2ac92913caaaf7c330b8bb1e43f9b08 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Thu, 2 May 2024 09:52:48 -0700 Subject: [PATCH] Fix Lock spin-waiting on single-proc machines (#101513) * 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. --- .../src/System/Threading/Lock.NativeAot.cs | 4 +- .../src/System/Threading/Lock.NonNativeAot.cs | 2 +- .../src/System/Threading/Lock.cs | 50 +++++++++++++------ 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs index 6e68125f07e32..999d4061543ed 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs @@ -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; /// /// Initializes a new instance of the class. @@ -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 diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.NonNativeAot.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.NonNativeAot.cs index 8a05f1e7fddc7..bd3a0be6902af 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.NonNativeAot.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.NonNativeAot.cs @@ -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(); /// /// Initializes a new instance of the class. diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs index 4dadf49bdfd4a..4572c9c310ece 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -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; @@ -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) @@ -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; } @@ -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) { @@ -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 @@ -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 @@ -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; } } @@ -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) @@ -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 =