From 109ccbbbefcc2f33807a56edd010448e6b7748ff Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 24 Jan 2019 14:26:51 +0100 Subject: [PATCH 1/8] Move portable thread pool implementation to shared partition. --- .../Interop.GetCpuUtilization.cs | 2 +- .../Kernel32}/Interop.GetSystemTimes.cs | 0 .../System.Private.CoreLib.Shared.projitems | 15 +++++++++++++++ ...ClrThreadPool.CpuUtilizationReader.Unix.cs | 0 ...ThreadPool.CpuUtilizationReader.Windows.cs | 0 .../Threading/ClrThreadPool.GateThread.cs | 0 .../ClrThreadPool.HillClimbing.Complex.cs | 0 .../Threading/ClrThreadPool.HillClimbing.cs | 0 .../Threading/ClrThreadPool.ThreadCounts.cs | 0 .../Threading/ClrThreadPool.WaitThread.cs | 0 .../Threading/ClrThreadPool.WorkerThread.cs | 0 .../System/Threading/ClrThreadPool.cs | 0 .../Threading/ClrThreadPoolEventSource.cs | 0 .../System/Threading/ThreadPool.Portable.cs | 0 .../src/System.Private.CoreLib.csproj | 19 ------------------- 15 files changed, 16 insertions(+), 20 deletions(-) rename src/{Common/src/Interop/Unix/System.Private.CoreLib.Native => System.Private.CoreLib/shared/Interop/Unix/System.Native}/Interop.GetCpuUtilization.cs (88%) rename src/{Common/src/Interop/Windows/kernel32 => System.Private.CoreLib/shared/Interop/Windows/Kernel32}/Interop.GetSystemTimes.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.CpuUtilizationReader.Unix.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.CpuUtilizationReader.Windows.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.GateThread.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.HillClimbing.Complex.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.HillClimbing.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.ThreadCounts.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.WaitThread.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.WorkerThread.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPool.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ClrThreadPoolEventSource.cs (100%) rename src/System.Private.CoreLib/{src => shared}/System/Threading/ThreadPool.Portable.cs (100%) diff --git a/src/Common/src/Interop/Unix/System.Private.CoreLib.Native/Interop.GetCpuUtilization.cs b/src/System.Private.CoreLib/shared/Interop/Unix/System.Native/Interop.GetCpuUtilization.cs similarity index 88% rename from src/Common/src/Interop/Unix/System.Private.CoreLib.Native/Interop.GetCpuUtilization.cs rename to src/System.Private.CoreLib/shared/Interop/Unix/System.Native/Interop.GetCpuUtilization.cs index 1f01d97a861..35e77c45419 100644 --- a/src/Common/src/Interop/Unix/System.Private.CoreLib.Native/Interop.GetCpuUtilization.cs +++ b/src/System.Private.CoreLib/shared/Interop/Unix/System.Native/Interop.GetCpuUtilization.cs @@ -18,7 +18,7 @@ internal struct ProcessCpuInformation ulong lastRecordedUserTime; } - [DllImport(Libraries.CoreLibNative, EntryPoint = "CoreLibNative_GetCpuUtilization")] + [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetCpuUtilization")] internal static extern unsafe int GetCpuUtilization(ref ProcessCpuInformation previousCpuInfo); } } diff --git a/src/Common/src/Interop/Windows/kernel32/Interop.GetSystemTimes.cs b/src/System.Private.CoreLib/shared/Interop/Windows/Kernel32/Interop.GetSystemTimes.cs similarity index 100% rename from src/Common/src/Interop/Windows/kernel32/Interop.GetSystemTimes.cs rename to src/System.Private.CoreLib/shared/Interop/Windows/Kernel32/Interop.GetSystemTimes.cs diff --git a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems index bfd79aa3919..1c501e04b87 100644 --- a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems +++ b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems @@ -1110,6 +1110,21 @@ + + + + + + + + + + + + + + + diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.CpuUtilizationReader.Unix.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.CpuUtilizationReader.Unix.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.CpuUtilizationReader.Unix.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.CpuUtilizationReader.Unix.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.CpuUtilizationReader.Windows.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.CpuUtilizationReader.Windows.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.CpuUtilizationReader.Windows.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.CpuUtilizationReader.Windows.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.GateThread.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.GateThread.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.GateThread.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.GateThread.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.HillClimbing.Complex.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.HillClimbing.Complex.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.HillClimbing.Complex.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.HillClimbing.Complex.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.HillClimbing.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.HillClimbing.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.HillClimbing.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.HillClimbing.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.ThreadCounts.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.ThreadCounts.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.ThreadCounts.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.ThreadCounts.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.WaitThread.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WaitThread.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.WaitThread.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WaitThread.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.WorkerThread.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.WorkerThread.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ClrThreadPoolEventSource.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPoolEventSource.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ClrThreadPoolEventSource.cs rename to src/System.Private.CoreLib/shared/System/Threading/ClrThreadPoolEventSource.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs b/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs rename to src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs diff --git a/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj index 64ac952c752..439e33ab733 100644 --- a/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj +++ b/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj @@ -357,9 +357,6 @@ Interop\Windows\kernel32\Interop.ConditionVariable.cs - - Interop\Windows\kernel32\Interop.GetSystemTimes.cs - Interop\Windows\kernel32\Interop.GetModuleFileName.cs @@ -450,19 +447,6 @@ Interop\Windows\mincore\Interop.QueryUnbiasedInterruptTime.cs - - - - - - - - - - - - - @@ -491,9 +475,6 @@ Interop\Unix\System.Private.CoreLib.Native\Interop.Exit.cs - - Interop\Unix\System.Private.CoreLib.Native\Interop.GetCpuUtilization.cs - Interop\Unix\System.Private.CoreLib.Native\Interop.GetEnv.cs From 7d7a80606946aa6fb9eb1deb0537e7173121772b Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 29 Jan 2019 10:09:32 +0100 Subject: [PATCH 2/8] Remove function that was moved to System.Native in CoreFX. --- .../pal_time.cpp | 86 ------------------- 1 file changed, 86 deletions(-) diff --git a/src/Native/System.Private.CoreLib.Native/pal_time.cpp b/src/Native/System.Private.CoreLib.Native/pal_time.cpp index 136cb03bc82..225189200fa 100644 --- a/src/Native/System.Private.CoreLib.Native/pal_time.cpp +++ b/src/Native/System.Private.CoreLib.Native/pal_time.cpp @@ -141,89 +141,3 @@ extern "C" uint64_t CoreLibNative_GetTickCount64() #endif return retval; } - - -struct ProcessCpuInformation -{ - uint64_t lastRecordedCurrentTime; - uint64_t lastRecordedKernelTime; - uint64_t lastRecordedUserTime; -}; - - -/* -Function: -CoreLibNative_GetCpuUtilization - -The main purpose of this function is to compute the overall CPU utilization -for the CLR thread pool to regulate the number of worker threads. -Since there is no consistent API on Unix to get the CPU utilization -from a user process, getrusage and gettimeofday are used to -compute the current process's CPU utilization instead. - -*/ - -static long numProcessors = 0; -extern "C" int32_t CoreLibNative_GetCpuUtilization(ProcessCpuInformation* previousCpuInfo) -{ - if (numProcessors <= 0) - { - numProcessors = sysconf(_SC_NPROCESSORS_CONF); - if (numProcessors <= 0) - { - return 0; - } - } - - uint64_t kernelTime = 0; - uint64_t userTime = 0; - - struct rusage resUsage; - if (getrusage(RUSAGE_SELF, &resUsage) == -1) - { - assert(false); - return 0; - } - else - { - kernelTime = TimeValToNanoseconds(resUsage.ru_stime); - userTime = TimeValToNanoseconds(resUsage.ru_utime); - } - - uint64_t currentTime = CoreLibNative_GetHighPrecisionCount() * NanosecondsPerSecond / CoreLibNative_GetHighPrecisionCounterFrequency(); - - uint64_t lastRecordedCurrentTime = previousCpuInfo->lastRecordedCurrentTime; - uint64_t lastRecordedKernelTime = previousCpuInfo->lastRecordedKernelTime; - uint64_t lastRecordedUserTime = previousCpuInfo->lastRecordedUserTime; - - uint64_t cpuTotalTime = 0; - if (currentTime > lastRecordedCurrentTime) - { - // cpuTotalTime is based on clock time. Since multiple threads can run in parallel, - // we need to scale cpuTotalTime cover the same amount of total CPU time. - // rusage time is already scaled across multiple processors. - cpuTotalTime = (currentTime - lastRecordedCurrentTime); - cpuTotalTime *= numProcessors; - } - - uint64_t cpuBusyTime = 0; - if (userTime >= lastRecordedUserTime && kernelTime >= lastRecordedKernelTime) - { - cpuBusyTime = (userTime - lastRecordedUserTime) + (kernelTime - lastRecordedKernelTime); - } - - int32_t cpuUtilization = 0; - if (cpuTotalTime > 0 && cpuBusyTime > 0) - { - cpuUtilization = static_cast(cpuBusyTime / cpuTotalTime); - } - - assert(cpuUtilization >= 0 && cpuUtilization <= 100); - - previousCpuInfo->lastRecordedCurrentTime = currentTime; - previousCpuInfo->lastRecordedUserTime = userTime; - previousCpuInfo->lastRecordedKernelTime = kernelTime; - - return cpuUtilization; -} - From 2a4710ff463b8805db879ca145b12816b7837a40 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 29 Jan 2019 10:10:41 +0100 Subject: [PATCH 3/8] Use new EventWaitHandle.Set API to remove platform specific code. --- .../System/Threading/ThreadPool.Portable.cs | 29 +++---------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs b/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs index e59d5df2da1..04be0fcaed0 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs @@ -90,9 +90,7 @@ internal void RestartTimeout(int currentTimeMs) /// private SafeWaitHandle UserUnregisterWaitHandle { get; set; } - private IntPtr UserUnregisterWaitHandleValue { get; set; } - - internal bool IsBlocking => UserUnregisterWaitHandleValue == (IntPtr)(-1); + internal bool IsBlocking => UserUnregisterWaitHandle == null || UserUnregisterWaitHandle.IsInvalid; /// /// The this was registered on. @@ -132,7 +130,6 @@ public bool Unregister(WaitHandle waitObject) { GC.SuppressFinalize(this); _callbackLock.Acquire(); - bool needToRollBackRefCountOnException = false; try { if (_unregisterCalled) @@ -141,10 +138,6 @@ public bool Unregister(WaitHandle waitObject) } UserUnregisterWaitHandle = waitObject?.SafeWaitHandle; - UserUnregisterWaitHandle?.DangerousAddRef(); - needToRollBackRefCountOnException = true; - - UserUnregisterWaitHandleValue = UserUnregisterWaitHandle?.DangerousGetHandle() ?? IntPtr.Zero; if (_unregistered) { @@ -175,14 +168,7 @@ public bool Unregister(WaitHandle waitObject) _callbacksComplete = null; } - UserUnregisterWaitHandleValue = IntPtr.Zero; - - if (needToRollBackRefCountOnException) - { - UserUnregisterWaitHandle?.DangerousRelease(); - } - - UserUnregisterWaitHandle = null; + UserUnregisterWaitHandle = null; throw; } finally @@ -201,22 +187,15 @@ private void SignalUserWaitHandle() { _callbackLock.VerifyIsLocked(); SafeWaitHandle handle = UserUnregisterWaitHandle; - IntPtr handleValue = UserUnregisterWaitHandleValue; try { - if (handleValue != IntPtr.Zero && handleValue != (IntPtr)(-1)) + if (handle != null) { - Debug.Assert(handleValue == handle.DangerousGetHandle()); -#if PLATFORM_WINDOWS - Interop.Kernel32.SetEvent(handle); -#else - WaitSubsystem.SetEvent(handleValue); -#endif + EventWaitHandle.Set(handle); } } finally { - handle?.DangerousRelease(); _callbacksComplete?.Set(); _unregistered = true; } From 4db63c7d9859f340af983611d765d960bc8d65a0 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 29 Jan 2019 10:14:43 +0100 Subject: [PATCH 4/8] Move portable Timer implementation to shared CoreLib. --- .../shared/System.Private.CoreLib.Shared.projitems | 3 +++ .../System/Threading/Timer.Portable.cs} | 0 src/System.Private.CoreLib/src/System.Private.CoreLib.csproj | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) rename src/System.Private.CoreLib/{src/System/Threading/Timer.Unix.cs => shared/System/Threading/Timer.Portable.cs} (100%) diff --git a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems index 1c501e04b87..d612a780783 100644 --- a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems +++ b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems @@ -1125,6 +1125,9 @@ + + + diff --git a/src/System.Private.CoreLib/src/System/Threading/Timer.Unix.cs b/src/System.Private.CoreLib/shared/System/Threading/Timer.Portable.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/Timer.Unix.cs rename to src/System.Private.CoreLib/shared/System/Threading/Timer.Portable.cs diff --git a/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj index 439e33ab733..8b98524b791 100644 --- a/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj +++ b/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj @@ -43,6 +43,8 @@ false true + false + true true @@ -460,7 +462,6 @@ - From ae451d5bd56f19f0aedbed97115c0b9a95c59c77 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 29 Jan 2019 10:59:21 +0100 Subject: [PATCH 5/8] Add back guard for invalid handles. --- .../shared/System/Threading/ThreadPool.Portable.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs b/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs index 04be0fcaed0..740d6d443f4 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs @@ -189,7 +189,7 @@ private void SignalUserWaitHandle() SafeWaitHandle handle = UserUnregisterWaitHandle; try { - if (handle != null) + if (handle != null && !handle.IsInvalid) { EventWaitHandle.Set(handle); } From 7eb9487ba7581444f78088eb63ac52f054942339 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 29 Jan 2019 10:59:41 +0100 Subject: [PATCH 6/8] Remove unnecessary reference to Internal.LowLevelLinq. --- .../shared/System/Threading/ClrThreadPool.WorkerThread.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs index fc55b861228..b3fa80e24ef 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System.Globalization; -using Internal.LowLevelLinq; using Internal.Runtime.Augments; namespace System.Threading From 776178c3b9a4a58b716a4c33eaa76cd9a8f645a0 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 29 Jan 2019 11:29:43 +0100 Subject: [PATCH 7/8] Make LowLevelLock in portable thread pool a CoreRT-specific optimization. --- .../Threading/ClrThreadPool.GateThread.cs | 4 +- .../Threading/ClrThreadPool.WaitThread.cs | 51 ++++++----- .../Threading/ClrThreadPool.WorkerThread.cs | 2 +- .../shared/System/Threading/ClrThreadPool.cs | 64 ++++++++----- .../System/Threading/ThreadPool.Portable.cs | 89 ++++++++++--------- 5 files changed, 117 insertions(+), 93 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.GateThread.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.GateThread.cs index 874e8f9b8c3..be1fb64be6d 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.GateThread.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.GateThread.cs @@ -17,9 +17,7 @@ private static class GateThread private static int s_runningState; - private static AutoResetEvent s_runGateThreadEvent = new AutoResetEvent(true); - - private static LowLevelLock s_createdLock = new LowLevelLock(); + private static readonly AutoResetEvent s_runGateThreadEvent = new AutoResetEvent(true); private static readonly CpuUtilizationReader s_cpu = new CpuUtilizationReader(); private const int MaxRuns = 2; diff --git a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WaitThread.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WaitThread.cs index 8a13659737b..b0e1d098918 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WaitThread.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WaitThread.cs @@ -16,7 +16,11 @@ internal partial class ClrThreadPool private WaitThreadNode _waitThreadsHead; private WaitThreadNode _waitThreadsTail; - private LowLevelLock _waitThreadLock = new LowLevelLock(); +#if CORERT + private readonly Lock _waitThreadLock = new Lock(); +#else + private object _waitThreadLock = new object(); +#endif /// /// Register a wait handle on a . @@ -24,8 +28,11 @@ internal partial class ClrThreadPool /// A description of the requested registration. internal void RegisterWaitHandle(RegisteredWaitHandle handle) { - _waitThreadLock.Acquire(); - try +#if CORERT + using (LockHolder.Hold(_waitThreadLock)) +#else + lock (_waitThreadLock) +#endif { if (_waitThreadsHead == null) // Lazily create the first wait thread. { @@ -56,10 +63,6 @@ internal void RegisterWaitHandle(RegisteredWaitHandle handle) prev.Next.Thread.RegisterWaitHandle(handle); return; } - finally - { - _waitThreadLock.Release(); - } } /// @@ -69,8 +72,11 @@ internal void RegisterWaitHandle(RegisteredWaitHandle handle) /// true if the thread was successfully removed; otherwise, false private bool TryRemoveWaitThread(WaitThread thread) { - _waitThreadLock.Acquire(); - try +#if CORERT + using (LockHolder.Hold(_waitThreadLock)) +#else + lock (_waitThreadLock) +#endif { if (thread.AnyUserWaits) { @@ -78,10 +84,6 @@ private bool TryRemoveWaitThread(WaitThread thread) } RemoveWaitThread(thread); } - finally - { - _waitThreadLock.Release(); - } return true; } @@ -267,8 +269,11 @@ private void WaitThreadStart() /// private void ProcessRemovals() { - ThreadPoolInstance._waitThreadLock.Acquire(); - try +#if CORERT + using (LockHolder.Hold(ThreadPoolInstance._waitThreadLock)) +#else + lock (ThreadPoolInstance._waitThreadLock) +#endif { Debug.Assert(_numPendingRemoves >= 0); Debug.Assert(_numPendingRemoves <= _pendingRemoves.Length); @@ -307,10 +312,6 @@ private void ProcessRemovals() Debug.Assert(originalNumUserWaits - originalNumPendingRemoves == _numUserWaits, $"{originalNumUserWaits} - {originalNumPendingRemoves} == {_numUserWaits}"); } - finally - { - ThreadPoolInstance._waitThreadLock.Release(); - } } /// @@ -350,7 +351,6 @@ private void CompleteWait(object state) /// If the handle was successfully registered on this wait thread. public bool RegisterWaitHandle(RegisteredWaitHandle handle) { - ThreadPoolInstance._waitThreadLock.VerifyIsLocked(); if (_numUserWaits == WaitHandle.MaxWaitHandles - 1) { return false; @@ -389,8 +389,11 @@ private void UnregisterWait(RegisteredWaitHandle handle, bool blocking) { bool pendingRemoval = false; // TODO: Optimization: Try to unregister wait directly if it isn't being waited on. - ThreadPoolInstance._waitThreadLock.Acquire(); - try +#if CORERT + using (LockHolder.Hold(ThreadPoolInstance._waitThreadLock)) +#else + lock (ThreadPoolInstance._waitThreadLock) +#endif { // If this handle is not already pending removal and hasn't already been removed if (Array.IndexOf(_registeredWaits, handle) != -1 && Array.IndexOf(_pendingRemoves, handle) == -1) @@ -400,10 +403,6 @@ private void UnregisterWait(RegisteredWaitHandle handle, bool blocking) pendingRemoval = true; } } - finally - { - ThreadPoolInstance._waitThreadLock.Release(); - } if (blocking) { diff --git a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs index b3fa80e24ef..ab3f0a7606d 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.WorkerThread.cs @@ -18,7 +18,7 @@ private static class WorkerThread /// Semaphore for controlling how many threads are currently working. /// private static LowLevelLifoSemaphore s_semaphore = new LowLevelLifoSemaphore(0, MaxPossibleThreadCount); - + private static void WorkerThreadStart() { ClrThreadPoolEventSource.Log.WorkerThreadStart(ThreadCounts.VolatileReadCounts(ref ThreadPoolInstance._separated.counts).numExistingThreads); diff --git a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.cs b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.cs index 4252d93e62e..9fbc16aeaf1 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ClrThreadPool.cs @@ -34,7 +34,11 @@ internal sealed partial class ClrThreadPool private short _minThreads; private short _maxThreads; - private readonly LowLevelLock _maxMinThreadLock = new LowLevelLock(); +#if CORERT + private readonly Lock _maxMinThreadLock = new Lock(); +#else + private readonly object _maxMinThreadLock = new object(); +#endif [StructLayout(LayoutKind.Explicit, Size = CacheLineSize * 5)] private struct CacheLineSeparated @@ -61,7 +65,11 @@ private struct CacheLineSeparated private int _completionCount = 0; private int _threadAdjustmentIntervalMs; - private LowLevelLock _hillClimbingThreadAdjustmentLock = new LowLevelLock(); +#if CORERT + private readonly Lock _hillClimbingThreadAdjustmentLock = new Lock(); +#else + private readonly object _hillClimbingThreadAdjustmentLock = new object(); +#endif private volatile int _numRequestedWorkers = 0; @@ -90,8 +98,11 @@ private ClrThreadPool() public bool SetMinThreads(int minThreads) { - _maxMinThreadLock.Acquire(); - try +#if CORERT + using (LockHolder.Hold(_maxMinThreadLock)) +#else + lock (_maxMinThreadLock) +#endif { if (minThreads < 0 || minThreads > _maxThreads) { @@ -129,18 +140,17 @@ public bool SetMinThreads(int minThreads) return true; } } - finally - { - _maxMinThreadLock.Release(); - } } public int GetMinThreads() => _minThreads; public bool SetMaxThreads(int maxThreads) { - _maxMinThreadLock.Acquire(); - try +#if CORERT + using (LockHolder.Hold(_maxMinThreadLock)) +#else + lock (_maxMinThreadLock) +#endif { if (maxThreads < _minThreads || maxThreads == 0) { @@ -173,10 +183,6 @@ public bool SetMaxThreads(int maxThreads) return true; } } - finally - { - _maxMinThreadLock.Release(); - } } public int GetMaxThreads() => _maxThreads; @@ -197,17 +203,34 @@ internal bool NotifyWorkItemComplete() // TODO: Check perf. Might need to make this thread-local. Interlocked.Increment(ref _completionCount); Volatile.Write(ref _separated.lastDequeueTime, Environment.TickCount); - - if (ShouldAdjustMaxWorkersActive() && _hillClimbingThreadAdjustmentLock.TryAcquire()) + + if (ShouldAdjustMaxWorkersActive()) { - try +#if CORERT + if (_hillClimbingThreadAdjustmentLock.TryAcquire(0)) { - AdjustMaxWorkersActive(); + try + { + AdjustMaxWorkersActive(); + } + finally + { + _hillClimbingThreadAdjustmentLock.Release(); + } } - finally +#else + if (Monitor.TryEnter(_hillClimbingThreadAdjustmentLock)) { - _hillClimbingThreadAdjustmentLock.Release(); + try + { + AdjustMaxWorkersActive(); + } + finally + { + Monitor.Exit(_hillClimbingThreadAdjustmentLock); + } } +#endif } return !WorkerThread.ShouldStopProcessingWorkNow(); @@ -219,7 +242,6 @@ internal bool NotifyWorkItemComplete() // private void AdjustMaxWorkersActive() { - _hillClimbingThreadAdjustmentLock.VerifyIsLocked(); int currentTicks = Environment.TickCount; int totalNumCompletions = Volatile.Read(ref _completionCount); int numCompletions = totalNumCompletions - _separated.priorCompletionCount; diff --git a/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs b/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs index 740d6d443f4..4c8658bab28 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/ThreadPool.Portable.cs @@ -102,7 +102,11 @@ internal void RestartTimeout(int currentTimeMs) /// private int _numRequestedCallbacks; - private LowLevelLock _callbackLock = new LowLevelLock(); +#if CORERT + private readonly Lock _callbackLock = new Lock(); +#else + private readonly object _callbackLock = new object(); +#endif /// /// Notes if we need to signal the user's unregister event after all callbacks complete. @@ -129,51 +133,53 @@ internal void RestartTimeout(int currentTimeMs) public bool Unregister(WaitHandle waitObject) { GC.SuppressFinalize(this); - _callbackLock.Acquire(); - try - { - if (_unregisterCalled) - { - return false; - } - - UserUnregisterWaitHandle = waitObject?.SafeWaitHandle; - - if (_unregistered) - { - SignalUserWaitHandle(); - return true; - } - - if (IsBlocking) - { - _callbacksComplete = RentEvent(); - } - else - { - _removed = RentEvent(); - } - _unregisterCalled = true; - } - catch (Exception) // Rollback state on exception +#if CORERT + using (LockHolder.Hold(_callbackLock)) +#else + lock (_callbackLock) +#endif { - if (_removed != null) + try { - ReturnEvent(_removed); - _removed = null; + if (_unregisterCalled) + { + return false; + } + + UserUnregisterWaitHandle = waitObject?.SafeWaitHandle; + + if (_unregistered) + { + SignalUserWaitHandle(); + return true; + } + + if (IsBlocking) + { + _callbacksComplete = RentEvent(); + } + else + { + _removed = RentEvent(); + } + _unregisterCalled = true; } - else if (_callbacksComplete != null) + catch (Exception) // Rollback state on exception { - ReturnEvent(_callbacksComplete); - _callbacksComplete = null; + if (_removed != null) + { + ReturnEvent(_removed); + _removed = null; + } + else if (_callbacksComplete != null) + { + ReturnEvent(_callbacksComplete); + _callbacksComplete = null; + } + + UserUnregisterWaitHandle = null; + throw; } - - UserUnregisterWaitHandle = null; - throw; - } - finally - { - _callbackLock.Release(); } WaitThread.UnregisterWait(this); @@ -185,7 +191,6 @@ public bool Unregister(WaitHandle waitObject) /// private void SignalUserWaitHandle() { - _callbackLock.VerifyIsLocked(); SafeWaitHandle handle = UserUnregisterWaitHandle; try { From 1a66e331652351022e8846a1ceaddd6557836b56 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 29 Jan 2019 13:50:19 +0100 Subject: [PATCH 8/8] Move some low-level locking primitives to be Unix-only since they are no longer used on Windows. --- .../src/System.Private.CoreLib.csproj | 6 +- ...Waiter.cs => FirstLevelSpinWaiter.Unix.cs} | 0 .../{LowLevelLock.cs => LowLevelLock.Unix.cs} | 0 .../System/Threading/LowLevelMonitor.Unix.cs | 92 ++++++++++++-- .../Threading/LowLevelMonitor.Windows.cs | 67 ---------- .../src/System/Threading/LowLevelMonitor.cs | 116 ------------------ 6 files changed, 85 insertions(+), 196 deletions(-) rename src/System.Private.CoreLib/src/System/Threading/{FirstLevelSpinWaiter.cs => FirstLevelSpinWaiter.Unix.cs} (100%) rename src/System.Private.CoreLib/src/System/Threading/{LowLevelLock.cs => LowLevelLock.Unix.cs} (100%) delete mode 100644 src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Windows.cs delete mode 100644 src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.cs diff --git a/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj index 8b98524b791..db70494d286 100644 --- a/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj +++ b/src/System.Private.CoreLib/src/System.Private.CoreLib.csproj @@ -279,11 +279,8 @@ - - - @@ -421,7 +418,6 @@ Interop\Windows\mincore\Interop.DynamicLoad.cs - @@ -459,6 +455,8 @@ + + diff --git a/src/System.Private.CoreLib/src/System/Threading/FirstLevelSpinWaiter.cs b/src/System.Private.CoreLib/src/System/Threading/FirstLevelSpinWaiter.Unix.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/FirstLevelSpinWaiter.cs rename to src/System.Private.CoreLib/src/System/Threading/FirstLevelSpinWaiter.Unix.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/LowLevelLock.cs b/src/System.Private.CoreLib/src/System/Threading/LowLevelLock.Unix.cs similarity index 100% rename from src/System.Private.CoreLib/src/System/Threading/LowLevelLock.cs rename to src/System.Private.CoreLib/src/System/Threading/LowLevelLock.Unix.cs diff --git a/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs b/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs index f87ba7a91f1..b91ec3367ef 100644 --- a/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs +++ b/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Unix.cs @@ -15,6 +15,9 @@ namespace System.Threading internal sealed partial class LowLevelMonitor : IDisposable { private IntPtr _nativeMonitor; +#if DEBUG + private RuntimeThread _ownerThread = null; +#endif public LowLevelMonitor() { @@ -25,6 +28,18 @@ public LowLevelMonitor() } } + ~LowLevelMonitor() + { + Dispose(); + } + + public void Dispose() + { + VerifyIsNotLockedByAnyThread(); + DisposeCore(); + GC.SuppressFinalize(this); + } + private void DisposeCore() { if (_nativeMonitor == IntPtr.Zero) @@ -36,37 +51,96 @@ private void DisposeCore() _nativeMonitor = IntPtr.Zero; } - private void AcquireCore() +#if DEBUG + public bool IsLocked => _ownerThread == RuntimeThread.CurrentThread; +#endif + + public void VerifyIsLocked() + { +#if DEBUG + Debug.Assert(IsLocked); +#endif + } + + public void VerifyIsNotLocked() + { +#if DEBUG + Debug.Assert(!IsLocked); +#endif + } + + private void VerifyIsNotLockedByAnyThread() { +#if DEBUG + Debug.Assert(_ownerThread == null); +#endif + } + + private void ResetOwnerThread() + { +#if DEBUG + VerifyIsLocked(); + _ownerThread = null; +#endif + } + + private void SetOwnerThreadToCurrent() + { +#if DEBUG + VerifyIsNotLockedByAnyThread(); + _ownerThread = RuntimeThread.CurrentThread; +#endif + } + + public void Acquire() + { + VerifyIsNotLocked(); Interop.Sys.LowLevelMutex_Acquire(_nativeMonitor); + SetOwnerThreadToCurrent(); } - private void ReleaseCore() + public void Release() { + ResetOwnerThread(); Interop.Sys.LowLevelMutex_Release(_nativeMonitor); } - private void WaitCore() + public void Wait() { + ResetOwnerThread(); Interop.Sys.LowLevelMonitor_Wait(_nativeMonitor); + SetOwnerThreadToCurrent(); } - private bool WaitCore(int timeoutMilliseconds) + public bool Wait(int timeoutMilliseconds) { Debug.Assert(timeoutMilliseconds >= -1); + ResetOwnerThread(); + bool waitResult; if (timeoutMilliseconds < 0) { - WaitCore(); - return true; + Interop.Sys.LowLevelMonitor_Wait(_nativeMonitor); + waitResult = true; } - - return Interop.Sys.LowLevelMonitor_TimedWait(_nativeMonitor, timeoutMilliseconds); + else + { + waitResult = Interop.Sys.LowLevelMonitor_TimedWait(_nativeMonitor, timeoutMilliseconds);; + } + SetOwnerThreadToCurrent(); + return waitResult; } - private void Signal_ReleaseCore() + public void Signal_Release() { + ResetOwnerThread(); Interop.Sys.LowLevelMonitor_Signal_Release(_nativeMonitor); } + + /// The following methods typical in a monitor are omitted since they are currently not necessary for the way in which + /// this class is used: + /// - TryAcquire + /// - Signal (use instead) + /// - SignalAll } } diff --git a/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Windows.cs b/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Windows.cs deleted file mode 100644 index a492e3acbd7..00000000000 --- a/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.Windows.cs +++ /dev/null @@ -1,67 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Diagnostics; -using System.Runtime.InteropServices; -using Internal.Runtime.Augments; - -namespace System.Threading -{ - /// - /// Wraps a critical section and condition variable. - /// - internal sealed partial class LowLevelMonitor : IDisposable - { - private Interop.Kernel32.CRITICAL_SECTION _criticalSection; - private Interop.Kernel32.CONDITION_VARIABLE _conditionVariable; - - public LowLevelMonitor() - { - Interop.Kernel32.InitializeCriticalSection(out _criticalSection); - Interop.Kernel32.InitializeConditionVariable(out _conditionVariable); - } - - private void DisposeCore() - { - Interop.Kernel32.DeleteCriticalSection(ref _criticalSection); - } - - private void AcquireCore() - { - Interop.Kernel32.EnterCriticalSection(ref _criticalSection); - } - - private void ReleaseCore() - { - Interop.Kernel32.LeaveCriticalSection(ref _criticalSection); - } - - private void WaitCore() - { - WaitCore(-1); - } - - private bool WaitCore(int timeoutMilliseconds) - { - bool waitResult = Interop.Kernel32.SleepConditionVariableCS(ref _conditionVariable, ref _criticalSection, timeoutMilliseconds); - if (!waitResult) - { - int lastError = Marshal.GetLastWin32Error(); - if (lastError != Interop.Errors.ERROR_TIMEOUT) - { - var exception = new OutOfMemoryException(); - exception.HResult = lastError; - throw exception; - } - } - return waitResult; - } - - private void Signal_ReleaseCore() - { - Interop.Kernel32.WakeConditionVariable(ref _conditionVariable); - Interop.Kernel32.LeaveCriticalSection(ref _criticalSection); - } - } -} diff --git a/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.cs b/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.cs deleted file mode 100644 index 88f1f5725c2..00000000000 --- a/src/System.Private.CoreLib/src/System/Threading/LowLevelMonitor.cs +++ /dev/null @@ -1,116 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Diagnostics; -using Internal.Runtime.Augments; - -namespace System.Threading -{ - /// - /// Wraps a non-recursive mutex and condition. - /// - /// Used by the wait subsystem on Unix, so this class cannot have any dependencies on the wait subsystem. - /// - internal sealed partial class LowLevelMonitor : IDisposable - { -#if DEBUG - private RuntimeThread _ownerThread = null; -#endif - - ~LowLevelMonitor() - { - Dispose(); - } - - public void Dispose() - { - VerifyIsNotLockedByAnyThread(); - DisposeCore(); - GC.SuppressFinalize(this); - } - -#if DEBUG - public bool IsLocked => _ownerThread == RuntimeThread.CurrentThread; -#endif - - public void VerifyIsLocked() - { -#if DEBUG - Debug.Assert(IsLocked); -#endif - } - - public void VerifyIsNotLocked() - { -#if DEBUG - Debug.Assert(!IsLocked); -#endif - } - - private void VerifyIsNotLockedByAnyThread() - { -#if DEBUG - Debug.Assert(_ownerThread == null); -#endif - } - - private void ResetOwnerThread() - { -#if DEBUG - VerifyIsLocked(); - _ownerThread = null; -#endif - } - - private void SetOwnerThreadToCurrent() - { -#if DEBUG - VerifyIsNotLockedByAnyThread(); - _ownerThread = RuntimeThread.CurrentThread; -#endif - } - - public void Acquire() - { - VerifyIsNotLocked(); - AcquireCore(); - SetOwnerThreadToCurrent(); - } - - public void Release() - { - ResetOwnerThread(); - ReleaseCore(); - } - - public void Wait() - { - ResetOwnerThread(); - WaitCore(); - SetOwnerThreadToCurrent(); - } - - public bool Wait(int timeoutMilliseconds) - { - Debug.Assert(timeoutMilliseconds >= -1); - - ResetOwnerThread(); - bool waitResult = WaitCore(timeoutMilliseconds); - SetOwnerThreadToCurrent(); - return waitResult; - } - - public void Signal_Release() - { - ResetOwnerThread(); - Signal_ReleaseCore(); - } - - /// The following methods typical in a monitor are omitted since they are currently not necessary for the way in which - /// this class is used: - /// - TryAcquire - /// - Signal (use instead) - /// - SignalAll - } -}