From 5fc571cd495d3c90f82bdad82be293e84dee96aa Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 31 Jan 2017 14:50:44 -0500 Subject: [PATCH 1/2] Make SpinLock.TryEnter(ref bool) fail faster One uses SpinLock.TryEnter(ref bool) when they want to do a quick test to either acquire the lock or know it's held by someone else. We can make the latter path much faster so that it fails faster. --- .../src/System/Threading/SpinLock.cs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs b/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs index 091d48af59a..0293b6d5bc9 100644 --- a/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs +++ b/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs @@ -189,7 +189,22 @@ public void Enter(ref bool lockTaken) /// public void TryEnter(ref bool lockTaken) { - TryEnter(0, ref lockTaken); + int observedOwner = m_owner; + if (((observedOwner & LOCK_ID_DISABLE_MASK) == 0) | lockTaken) + { + // Thread tracking enabled or invalid arg. Take slow path. + ContinueTryEnter(0, ref lockTaken); + } + else if ((observedOwner & LOCK_ANONYMOUS_OWNED) != 0) + { + // Lock already held by someone + lockTaken = false; + } + else + { + // Lock wasn't held; try to acquire it. + CompareExchange(ref m_owner, observedOwner | LOCK_ANONYMOUS_OWNED, observedOwner, ref lockTaken); + } } /// @@ -527,10 +542,10 @@ public void Exit(bool useMemoryBarrier) { // This is the fast path for the thread tracking is diabled and not to use memory barrier, otherwise go to the slow path // The reason not to add else statement if the usememorybarrier is that it will add more barnching in the code and will prevent - // method inlining, so this is optimized for useMemoryBarrier=false and Exit() overload optimized for useMemoryBarrier=true - if ((m_owner & LOCK_ID_DISABLE_MASK) != 0 && !useMemoryBarrier) + // method inlining, so this is optimized for useMemoryBarrier=false and Exit() overload optimized for useMemoryBarrier=true. + int tmpOwner = m_owner; + if ((tmpOwner & LOCK_ID_DISABLE_MASK) != 0 & !useMemoryBarrier) { - int tmpOwner = m_owner; m_owner = tmpOwner & (~LOCK_ANONYMOUS_OWNED); } else From c161443d8c33abf35dc0a8ffc69f0d172f10b650 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 31 Jan 2017 20:40:43 -0800 Subject: [PATCH 2/2] Port cosmetic SpinLock changes from CoreCLR --- .../src/System/Threading/SpinLock.cs | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs b/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs index 0293b6d5bc9..b5b8268e9b6 100644 --- a/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs +++ b/src/System.Private.CoreLib/src/System/Threading/SpinLock.cs @@ -5,17 +5,15 @@ // =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ // -// SpinLock.cs // A spin lock is a mutual exclusion lock primitive where a thread trying to acquire the lock waits in a loop ("spins") // repeatedly checking until the lock becomes available. As the thread remains active performing a non-useful task, // the use of such a lock is a kind of busy waiting and consumes CPU resources without performing real work. -// - // // =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- using System.Diagnostics; using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; using Internal.Threading.Tracing; @@ -104,7 +102,7 @@ public struct SpinLock // The waiters count is calculated by m_owner & WAITERS_MASK 01111....110 private static int MAXIMUM_WAITERS = WAITERS_MASK; - + [MethodImpl(MethodImplOptions.AggressiveInlining)] private int CompareExchange(ref int location, int value, int comparand, ref bool success) { int result = Interlocked.CompareExchange(ref location, value, comparand); @@ -131,7 +129,6 @@ public SpinLock(bool enableThreadOwnerTracking) } } - /// /// Initializes a new instance of the /// structure with the option to track thread IDs to improve debugging. @@ -301,7 +298,6 @@ private void ContinueTryEnter(int millisecondsTimeout, ref bool lockTaken) nameof(millisecondsTimeout), millisecondsTimeout, SR.SpinLock_TryEnter_ArgumentOutOfRange); } - uint startTime = 0; if (millisecondsTimeout != Timeout.Infinite && millisecondsTimeout != 0) { @@ -338,10 +334,15 @@ private void ContinueTryEnter(int millisecondsTimeout, ref bool lockTaken) observedOwner = m_owner; if ((observedOwner & LOCK_ANONYMOUS_OWNED) == LOCK_UNOWNED) { - if (CompareExchange(ref m_owner, observedOwner | 1, observedOwner, ref lockTaken) == observedOwner - || millisecondsTimeout == 0) + if (CompareExchange(ref m_owner, observedOwner | 1, observedOwner, ref lockTaken) == observedOwner) + { + // Aquired lock + return; + } + + if (millisecondsTimeout == 0) { - // Aquired lock, or did not aquire lock as owned but timeout is 0 so fail fast + // Did not aquire lock in CompareExchange and timeout is 0 so fail fast return; } } @@ -511,7 +512,6 @@ private void ContinueTryEnterWithThreadTracking(int millisecondsTimeout, uint st /// /// Thread ownership tracking is enabled, and the current thread is not the owner of this lock. /// - //[ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] public void Exit() { //This is the fast path for the thread tracking is disabled, otherwise go to the slow path @@ -537,7 +537,6 @@ public void Exit() /// /// Thread ownership tracking is enabled, and the current thread is not the owner of this lock. /// - //[ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] public void Exit(bool useMemoryBarrier) { // This is the fast path for the thread tracking is diabled and not to use memory barrier, otherwise go to the slow path @@ -592,7 +591,6 @@ private void ExitSlowPath(bool useMemoryBarrier) /// public bool IsHeld { - //[ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] get { if (IsThreadOwnerTrackingEnabled) @@ -618,7 +616,6 @@ public bool IsHeld /// public bool IsHeldByCurrentThread { - //[ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] get { if (!IsThreadOwnerTrackingEnabled) @@ -632,7 +629,6 @@ public bool IsHeldByCurrentThread /// Gets whether thread ownership tracking is enabled for this instance. public bool IsThreadOwnerTrackingEnabled { - //[ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] get { return (m_owner & LOCK_ID_DISABLE_MASK) == 0; } }