From a87ac12a0c604e56a93928b94a922d701561a319 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 7 Dec 2018 21:14:44 -0500 Subject: [PATCH] Remove some unnecessary spinning (dotnet/coreclr#21437) Most of the use of SpinWait in CoreLib involves waiting for some short-lived operation to complete on another thread, in which case the spinning thread should backoff as it's unable to make forward progress until the other operation completes. In a few cases, however, SpinWait is being used just around CompareExchange operations, such that at least one thread running this code path is guaranteed to make forward progress, and the backoff in the spinning doesn't actually help (in theory it could help to reduce contention if lots of threads were all trying to CompareExchange concurrently, but in such cases you'd actually want more randomized backoff, as otherwise it's likely all the threads would re-attempt at around the same time and similarly re-encounter contention). Commit migrated from https://github.com/dotnet/coreclr/commit/1cba6f8c32a74a7e7e20fad004be6796a7282c94 --- .../src/System/Threading/Tasks/Task.cs | 29 +++++++++---------- .../Concurrent/ConcurrentQueueSegment.cs | 8 ++--- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs index 4b9ccc6300008e..b9b60dd7a28df9 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs @@ -721,31 +721,31 @@ internal bool AtomicStateUpdate(int newBits, int illegalBits) private bool AtomicStateUpdateSlow(int newBits, int illegalBits) { - var sw = new SpinWait(); + int flags = m_stateFlags; do { - int oldFlags = m_stateFlags; - if ((oldFlags & illegalBits) != 0) return false; - if (Interlocked.CompareExchange(ref m_stateFlags, oldFlags | newBits, oldFlags) == oldFlags) + if ((flags & illegalBits) != 0) return false; + int oldFlags = Interlocked.CompareExchange(ref m_stateFlags, flags | newBits, flags); + if (oldFlags == flags) { return true; } - sw.SpinOnce(); + flags = oldFlags; } while (true); } internal bool AtomicStateUpdate(int newBits, int illegalBits, ref int oldFlags) { - SpinWait sw = new SpinWait(); + int flags = oldFlags = m_stateFlags; do { - oldFlags = m_stateFlags; - if ((oldFlags & illegalBits) != 0) return false; - if (Interlocked.CompareExchange(ref m_stateFlags, oldFlags | newBits, oldFlags) == oldFlags) + if ((flags & illegalBits) != 0) return false; + oldFlags = Interlocked.CompareExchange(ref m_stateFlags, flags | newBits, flags); + if (oldFlags == flags) { return true; } - sw.SpinOnce(); + flags = oldFlags; } while (true); } @@ -772,13 +772,12 @@ internal void SetNotificationForWaitCompletion(bool enabled) else { // Atomically clear the END_AWAIT_NOTIFICATION bit - SpinWait sw = new SpinWait(); + int flags = m_stateFlags; while (true) { - int oldFlags = m_stateFlags; - int newFlags = oldFlags & (~TASK_STATE_WAIT_COMPLETION_NOTIFICATION); - if (Interlocked.CompareExchange(ref m_stateFlags, newFlags, oldFlags) == oldFlags) break; - sw.SpinOnce(); + int oldFlags = Interlocked.CompareExchange(ref m_stateFlags, flags & (~TASK_STATE_WAIT_COMPLETION_NOTIFICATION), flags); + if (oldFlags == flags) break; + flags = oldFlags; } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs index c706fae02e130b..85224a537a5f3b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs @@ -112,15 +112,15 @@ internal static int RoundUpToPowerOf2(int i) _frozenForEnqueues = true; // Increase the tail by FreezeOffset, spinning until we're successful in doing so. - var spinner = new SpinWait(); + int tail = _headAndTail.Tail; while (true) { - int tail = Volatile.Read(ref _headAndTail.Tail); - if (Interlocked.CompareExchange(ref _headAndTail.Tail, tail + FreezeOffset, tail) == tail) + int oldTail = Interlocked.CompareExchange(ref _headAndTail.Tail, tail + FreezeOffset, tail); + if (oldTail == tail) { break; } - spinner.SpinOnce(); + tail = oldTail; } } }