[NativeAOT] Thin locks (dotnet#79519)

* switch to managed thread ID in Lock * fattening the lock * __declspec(selectany) * few tweaks * fairness * more room for thread ids * remove CurrentNativeThreadId * couple fixes * fix win-arm64 build * win-arm64 build , another try * Apply suggestions from code review Co-authored-by: Jan Kotas <[email protected]> * fix after renaming * do not report successful spin if thread has waited * keep extern and undo mangling of tls_CurrentThread in asm * use SyncTable indexer in less perf-sensitive places. * GetNewHashCode just delegate to shared random * Apply suggestions from code review Co-authored-by: Jan Kotas <[email protected]> * unchecked const conversion * some refactoring comments and typos * min number of spins in the backoff * moved CurrentManagedThreadIdUnchecked to ManagedThreadId * Use `-1` to report success and allow using element #1 in the SyncTable * use threadstatic for managed thread ID * check before calling RhGetProcessCpuCount * use 0 as default thread ID Co-authored-by: Jan Kotas <[email protected]>
bartonjs · Dec 15, 2022 · d7924cc · d7924cc
1 parent ca873ea
commit d7924cc
Show file tree

Hide file tree

Showing 16 changed files with 744 additions and 306 deletions.
diff --git a/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp b/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp
@@ -51,11 +51,11 @@ EXTERN_C NATIVEAOT_API void __cdecl RhSpinWait(int32_t iterations)
     ASSERT(iterations > 0);
 
     // limit the spin count in coop mode.
-    ASSERT_MSG(iterations <= 10000 || !ThreadStore::GetCurrentThread()->IsCurrentThreadInCooperativeMode(),
+    ASSERT_MSG(iterations <= 1024 || !ThreadStore::GetCurrentThread()->IsCurrentThreadInCooperativeMode(),
         "This is too long wait for coop mode. You must p/invoke with GC transition.");
 
     YieldProcessorNormalizationInfo normalizationInfo;
-    YieldProcessorNormalizedForPreSkylakeCount(normalizationInfo, iterations);
+    YieldProcessorNormalized(normalizationInfo, iterations);
 }
 
 // Yield the cpu to another thread ready to process, if one is available.

diff --git a/src/coreclr/nativeaot/Runtime/thread.cpp b/src/coreclr/nativeaot/Runtime/thread.cpp
@@ -260,7 +260,10 @@ void Thread::Construct()
     // alloc_context ever needs different initialization, a matching change to the tls_CurrentThread
     // static initialization will need to be made.
 
-    m_uPalThreadIdForLogging = PalGetCurrentThreadIdForLogging();
+    m_pTransitionFrame = TOP_OF_STACK_MARKER;
+    m_pDeferredTransitionFrame = TOP_OF_STACK_MARKER;
+    m_hPalThread = INVALID_HANDLE_VALUE;
+
     m_threadId.SetToCurrentThread();
 
     HANDLE curProcessPseudo = PalGetCurrentProcess();
@@ -328,7 +331,7 @@ bool Thread::CatchAtSafePoint()
 
 uint64_t Thread::GetPalThreadIdForLogging()
 {
-    return m_uPalThreadIdForLogging;
+    return *(uint64_t*)&m_threadId;
 }
 
 bool Thread::IsCurrentThread()

diff --git a/src/coreclr/nativeaot/Runtime/thread.h b/src/coreclr/nativeaot/Runtime/thread.h
@@ -95,8 +95,7 @@ struct ThreadBuffer
     PTR_VOID                m_pStackLow;
     PTR_VOID                m_pStackHigh;
     PTR_UInt8               m_pTEB;                                 // Pointer to OS TEB structure for this thread
-    uint64_t                m_uPalThreadIdForLogging;               // @TODO: likely debug-only
-    EEThreadId              m_threadId;
+    EEThreadId              m_threadId;                             // OS thread ID
     PTR_VOID                m_pThreadStressLog;                     // pointer to head of thread's StressLogChunks
     NATIVE_CONTEXT*         m_interruptedContext;                   // context for an asynchronously interrupted thread.
 #ifdef FEATURE_SUSPEND_REDIRECTION
@@ -192,7 +191,7 @@ class Thread : private ThreadBuffer
     gc_alloc_context *  GetAllocContext();  // @TODO: I would prefer to not expose this in this way
 
 #ifndef DACCESS_COMPILE
-    uint64_t              GetPalThreadIdForLogging();
+    uint64_t            GetPalThreadIdForLogging();
     bool                IsCurrentThread();
 
     void                GcScanRoots(void * pfnEnumCallback, void * pvCallbackData);

diff --git a/src/coreclr/nativeaot/Runtime/threadstore.cpp b/src/coreclr/nativeaot/Runtime/threadstore.cpp
@@ -415,20 +415,9 @@ COOP_PINVOKE_HELPER(void, RhpCancelThreadAbort, (void* thread))
 
 C_ASSERT(sizeof(Thread) == sizeof(ThreadBuffer));
 
-EXTERN_C DECLSPEC_THREAD ThreadBuffer tls_CurrentThread;
-DECLSPEC_THREAD ThreadBuffer tls_CurrentThread =
-{
-    { 0 },                              // m_rgbAllocContextBuffer
-    Thread::TSF_Unknown,                // m_ThreadStateFlags
-    TOP_OF_STACK_MARKER,                // m_pTransitionFrame
-    TOP_OF_STACK_MARKER,                // m_pDeferredTransitionFrame
-    0,                                  // m_pCachedTransitionFrame
-    0,                                  // m_pNext
-    INVALID_HANDLE_VALUE,               // m_hPalThread
-    0,                                  // m_ppvHijackedReturnAddressLocation
-    0,                                  // m_pvHijackedReturnAddress
-    0,                                  // all other fields are initialized by zeroes
-};
+#ifndef _MSC_VER
+__thread ThreadBuffer tls_CurrentThread;
+#endif
 
 EXTERN_C ThreadBuffer* RhpGetThread()
 {

diff --git a/src/coreclr/nativeaot/Runtime/threadstore.inl b/src/coreclr/nativeaot/Runtime/threadstore.inl
@@ -1,7 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-EXTERN_C DECLSPEC_THREAD ThreadBuffer tls_CurrentThread;
+#ifdef _MSC_VER
+// a workaround to prevent tls_CurrentThread from becoming dynamically checked/initialized.
+EXTERN_C __declspec(selectany) __declspec(thread) ThreadBuffer tls_CurrentThread;
+#else
+EXTERN_C __thread ThreadBuffer tls_CurrentThread;
+#endif
 
 // static
 inline Thread * ThreadStore::RawGetCurrentThread()

diff --git a/.../System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SynchronizedMethodHelpers.cs b/.../System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SynchronizedMethodHelpers.cs
@@ -14,42 +14,68 @@ internal static class SynchronizedMethodHelpers
         private static void MonitorEnter(object obj, ref bool lockTaken)
         {
             // Inlined Monitor.Enter with a few tweaks
-            Lock lck = Monitor.GetLock(obj);
+            int resultOrIndex = ObjectHeader.Acquire(obj);
+            if (resultOrIndex < 0)
+            {
+                lockTaken = true;
+                return;
+            }
+
+            Lock lck = resultOrIndex == 0 ?
+                ObjectHeader.GetLockObject(obj) :
+                SyncTable.GetLockObject(resultOrIndex);
+
             if (lck.TryAcquire(0))
             {
                 lockTaken = true;
                 return;
             }
+
             Monitor.TryAcquireContended(lck, obj, Timeout.Infinite);
             lockTaken = true;
         }
         private static void MonitorExit(object obj, ref bool lockTaken)
         {
             // Inlined Monitor.Exit with a few tweaks
-            if (!lockTaken) return;
-            Monitor.GetLock(obj).Release();
+            if (!lockTaken)
+                return;
+
+            ObjectHeader.Release(obj);
             lockTaken = false;
         }
 
         private static void MonitorEnterStatic(IntPtr pEEType, ref bool lockTaken)
         {
             // Inlined Monitor.Enter with a few tweaks
             object obj = GetStaticLockObject(pEEType);
-            Lock lck = Monitor.GetLock(obj);
+            int resultOrIndex = ObjectHeader.Acquire(obj);
+            if (resultOrIndex < 0)
+            {
+                lockTaken = true;
+                return;
+            }
+
+            Lock lck = resultOrIndex == 0 ?
+                ObjectHeader.GetLockObject(obj) :
+                SyncTable.GetLockObject(resultOrIndex);
+
             if (lck.TryAcquire(0))
             {
                 lockTaken = true;
                 return;
             }
+
             Monitor.TryAcquireContended(lck, obj, Timeout.Infinite);
             lockTaken = true;
         }
         private static void MonitorExitStatic(IntPtr pEEType, ref bool lockTaken)
         {
             // Inlined Monitor.Exit with a few tweaks
-            if (!lockTaken) return;
+            if (!lockTaken)
+                return;
+
             object obj = GetStaticLockObject(pEEType);
-            Monitor.GetLock(obj).Release();
+            ObjectHeader.Release(obj);
             lockTaken = false;
         }
 

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Environment.NativeAot.Unix.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Environment.NativeAot.Unix.cs
@@ -9,8 +9,6 @@ namespace System
 {
     public static partial class Environment
     {
-        internal static int CurrentNativeThreadId => ManagedThreadId.Current;
-
         public static long TickCount64 => (long)RuntimeImports.RhpGetTickCount64();
 
         [DoesNotReturn]

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Environment.NativeAot.Windows.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Environment.NativeAot.Windows.cs
@@ -7,8 +7,6 @@ namespace System
 {
     public static partial class Environment
     {
-        internal static int CurrentNativeThreadId => unchecked((int)Interop.Kernel32.GetCurrentThreadId());
-
         public static long TickCount64 => (long)Interop.Kernel32.GetTickCount64();
 
         [DoesNotReturn]

diff --git a/...ot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs b/...ot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs
@@ -94,17 +94,9 @@ public static object GetObjectValue(object? obj)
             return RuntimeImports.RhCompareObjectContentsAndPadding(o1, o2);
         }
 
-        [ThreadStatic]
-        private static int t_hashSeed;
-
         internal static int GetNewHashCode()
         {
-            int multiplier = Environment.CurrentManagedThreadId * 4 + 5;
-            // Every thread has its own generator for hash codes so that we won't get into a situation
-            // where two threads consistently give out the same hash codes.
-            // Choice of multiplier guarantees period of 2**32 - see Knuth Vol 2 p16 (3.2.1.2 Theorem A).
-            t_hashSeed = t_hashSeed * multiplier + 1;
-            return t_hashSeed;
+            return Random.Shared.Next();
         }
 
         public static unsafe int GetHashCode(object o)
@@ -228,6 +220,9 @@ internal static unsafe ushort GetElementSize(this Array array)
         internal static unsafe MethodTable* GetMethodTable(this object obj)
             => obj.m_pEEType;
 
+        internal static unsafe ref MethodTable* GetMethodTableRef(this object obj)
+            => ref obj.m_pEEType;
+
         internal static unsafe EETypePtr GetEETypePtr(this object obj)
             => new EETypePtr(obj.m_pEEType);
 

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeExceptionHelpers.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeExceptionHelpers.cs
@@ -464,7 +464,7 @@ private static void SerializeExceptionsForDump(Exception currentException, IntPt
             LowLevelList<Exception> exceptions = new LowLevelList<Exception>(curThreadExceptions);
             LowLevelList<Exception> nonThrownInnerExceptions = new LowLevelList<Exception>();
 
-            uint currentThreadId = (uint)Environment.CurrentNativeThreadId;
+            uint currentThreadId = (uint)Environment.CurrentManagedThreadId;
 
             // Reset nesting levels for exceptions on this thread that might not be currently in flight
             foreach (KeyValuePair<Exception, ExceptionData> item in s_exceptionDataTable)