Delete interop standby lists, dedup Debugger.DefaultCategory (#115446)

* Delete interop standby lists

Global free lists on top of regular malloc/free used to be a fine optimization years ago on single-proc machines. They are typically a de-optimization these days, breaking thread locality and trashing caches.

* Dedup Debugger.DefaultCategory

Author: jkotas

src/coreclr/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs

@@ -65,16 +65,6 @@ static void NotifyOfCrossThreadDependencySlow()
         [SuppressGCTransition]
         private static partial int IsManagedDebuggerAttached();
 
-        // Constants representing the importance level of messages to be logged.
-        //
-        // An attached debugger can enable or disable which messages will
-        // actually be reported to the user through the debugger
-        // services API.  This info is communicated to the runtime so only
-        // desired events are actually reported to the debugger.
-        //
-        // Constant representing the default category
-        public static readonly string? DefaultCategory;
-
         // Posts a message for the attached debugger.  If there is no
         // debugger attached, has no effect.  The debugger may or may not
         // report the message depending on its settings.

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs

@@ -41,17 +41,6 @@ public static void NotifyOfCrossThreadDependency()
             // nothing to do...yet
         }
 
-        /// <summary>
-        /// Constants representing the importance level of messages to be logged.
-        ///
-        /// An attached debugger can enable or disable which messages will
-        /// actually be reported to the user through the CLR debugger
-        /// services API.  This info is communicated to the runtime so only
-        /// desired events are actually reported to the debugger.
-        /// Constant representing the default category
-        /// </summary>
-        public static readonly string DefaultCategory;
-
         /// <summary>
         /// Posts a message for the attached debugger.  If there is no
         /// debugger attached, has no effect.  The debugger may or may not

src/coreclr/vm/interoputil.cpp

@@ -3662,7 +3662,6 @@ void InitializeComInterop()
     }
     CONTRACTL_END;
 
-    InitializeSListHead(&RCW::s_RCWStandbyList);
     ComCall::Init();
     CtxEntryCache::Init();
     ComCallWrapperTemplate::Init();

src/coreclr/vm/runtimecallablewrapper.cpp

@@ -41,9 +41,6 @@ class Object;
 #include "finalizerthread.h"
 #include "dynamicinterfacecastable.h"
 
-// static
-SLIST_HEADER RCW::s_RCWStandbyList;
-
 #ifdef FEATURE_COMINTEROP_APARTMENT_SUPPORT
 #include "olecontexthelpers.h"
 #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT
@@ -1292,19 +1289,6 @@ const int RCW::s_rGCPressureTable[GCPressureSize_COUNT] =
     GC_PRESSURE_REMOTE,          // GCPressureSize_Remote
 };
 
-// Deletes all items in code:s_RCWStandbyList.
-void RCW::FlushStandbyList()
-{
-    LIMITED_METHOD_CONTRACT;
-
-    PSLIST_ENTRY pEntry = InterlockedFlushSList(&RCW::s_RCWStandbyList);
-    while (pEntry)
-    {
-        PSLIST_ENTRY pNextEntry = pEntry->Next;
-        delete (RCW *)pEntry;
-        pEntry = pNextEntry;
-    }
-}
 //--------------------------------------------------------------------------------
 // The IUnknown passed in is AddRef'ed if we succeed in creating the wrapper.
 RCW* RCW::CreateRCW(IUnknown *pUnk, DWORD dwSyncBlockIndex, DWORD flags, MethodTable *pClassMT)
@@ -1344,16 +1328,7 @@ RCW* RCW::CreateRCWInternal(IUnknown *pUnk, DWORD dwSyncBlockIndex, DWORD flags,
     CONTRACT_END;
 
     // now allocate the wrapper
-    RCW *pWrap = (RCW *)InterlockedPopEntrySList(&RCW::s_RCWStandbyList);
-    if (pWrap != NULL)
-    {
-        // cache hit - reinitialize the data structure
-        new (pWrap) RCW();
-    }
-    else
-    {
-        pWrap = new RCW();
-    }
+    RCW *pWrap = new RCW();
 
     AppDomain * pAppDomain = GetAppDomain();
     ULONG cbRef = SafeAddRefPreemp(pUnk);
@@ -1764,7 +1739,7 @@ void RCW::Cleanup()
 #endif
 
     // If there's no thread currently working with the RCW, this call will release helper fields on IUnkEntry
-    // and recycle the entire RCW structure, i.e. insert it in the standby list to be reused or free the memory.
+    // and delete the entire RCW structure.
     // If a thread still keeps a ref-count on the RCW, it will release it when it's done. Keeping the structure
     // and the helper fields alive reduces the chances of memory corruption in race scenarios.
     DecrementUseCount();

src/coreclr/vm/runtimecallablewrapper.h

@@ -99,9 +99,6 @@ struct RCW
 
     static CreationFlags CreationFlagsFromObjForComIPFlags(ObjFromComIP::flags flags);
 
-    // List of RCW instances that have been freed since the last RCW cleanup.
-    static SLIST_HEADER s_RCWStandbyList;
-
     // Simple read-only iterator for all cached interface pointers.
     class CachedInterfaceEntryIterator
     {
@@ -161,9 +158,6 @@ struct RCW
         ZeroMemory(this, sizeof(*this));
     }
 
-    // Deletes all items in code:s_RCWStandbyList.
-    static void FlushStandbyList();
-
     // Create a new wrapper for given IUnk, IDispatch
     static RCW* CreateRCW(IUnknown *pUnk, DWORD dwSyncBlockIndex, DWORD flags, MethodTable *pClassMT);
 
@@ -489,20 +483,11 @@ struct RCW
 
         if (InterlockedDecrement(&m_cbUseCount) == 0)
         {
-            // this was the final decrement, go ahead and delete/recycle the RCW
-            {
-                GCX_PREEMP();
-                m_UnkEntry.Free();
-            }
+            // this was the final decrement, go ahead and delete the RCW
+            GCX_PREEMP();
+            m_UnkEntry.Free();
 
-            if (g_fEEShutDown)
-            {
-                delete this;
-            }
-            else
-            {
-                InterlockedPushEntrySList(&RCW::s_RCWStandbyList, (PSLIST_ENTRY)this);
-            }
+            delete this;
         }
     }
 

src/coreclr/vm/syncblk.cpp

@@ -52,11 +52,6 @@ SPTR_IMPL (SyncBlockCache, SyncBlockCache, s_pSyncBlockCache);
 
 #ifndef DACCESS_COMPILE
 
-#ifndef TARGET_UNIX
-// static
-SLIST_HEADER InteropSyncBlockInfo::s_InteropInfoStandbyList;
-#endif // !TARGET_UNIX
-
 InteropSyncBlockInfo::~InteropSyncBlockInfo()
 {
     CONTRACTL
@@ -71,30 +66,6 @@ InteropSyncBlockInfo::~InteropSyncBlockInfo()
     FreeUMEntryThunk();
 }
 
-#ifndef TARGET_UNIX
-// Deletes all items in code:s_InteropInfoStandbyList.
-void InteropSyncBlockInfo::FlushStandbyList()
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_TRIGGERS;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-
-    PSLIST_ENTRY pEntry = InterlockedFlushSList(&InteropSyncBlockInfo::s_InteropInfoStandbyList);
-    while (pEntry)
-    {
-        PSLIST_ENTRY pNextEntry = pEntry->Next;
-
-        // make sure to use the global delete since the destructor has already run
-        ::delete (void *)pEntry;
-        pEntry = pNextEntry;
-    }
-}
-#endif // !TARGET_UNIX
-
 void InteropSyncBlockInfo::FreeUMEntryThunk()
 {
     CONTRACTL
@@ -656,10 +627,6 @@ void SyncBlockCache::Start()
     g_SyncBlockCacheInstance.Init();
 
     SyncBlockCache::GetSyncBlockCache()->m_EphemeralBitmap = bm;
-
-#ifndef TARGET_UNIX
-    InitializeSListHead(&InteropSyncBlockInfo::s_InteropInfoStandbyList);
-#endif // !TARGET_UNIX
 }
 
 
@@ -976,19 +943,7 @@ void SyncBlockCache::DeleteSyncBlock(SyncBlock *psb)
         CleanupSyncBlockComData(psb->m_pInteropInfo);
 #endif // FEATURE_COMINTEROP
 
-#ifndef TARGET_UNIX
-        if (g_fEEShutDown)
-        {
-            delete psb->m_pInteropInfo;
-        }
-        else
-        {
-            psb->m_pInteropInfo->~InteropSyncBlockInfo();
-            InterlockedPushEntrySList(&InteropSyncBlockInfo::s_InteropInfoStandbyList, (PSLIST_ENTRY)psb->m_pInteropInfo);
-        }
-#else // !TARGET_UNIX
         delete psb->m_pInteropInfo;
-#endif // !TARGET_UNIX
     }
 
 #ifdef FEATURE_METADATA_UPDATER

src/coreclr/vm/syncblk.h

@@ -615,11 +615,6 @@ class InteropSyncBlockInfo
     friend class ClrDataAccess;
 
 public:
-#ifndef TARGET_UNIX
-    // List of InteropSyncBlockInfo instances that have been freed since the last syncblock cleanup.
-    static SLIST_HEADER s_InteropInfoStandbyList;
-#endif // !TARGET_UNIX
-
     InteropSyncBlockInfo()
         : m_pUMEntryThunk{}
 #ifdef FEATURE_COMINTEROP
@@ -640,11 +635,6 @@ class InteropSyncBlockInfo
     ~InteropSyncBlockInfo();
 #endif
 
-#ifndef TARGET_UNIX
-    // Deletes all items in code:s_InteropInfoStandbyList.
-    static void FlushStandbyList();
-#endif // !TARGET_UNIX
-
 #ifdef FEATURE_COMINTEROP
 
     //
@@ -963,20 +953,7 @@ class SyncBlock
 
         if (!m_pInteropInfo)
         {
-            NewHolder<InteropSyncBlockInfo> pInteropInfo;
-#ifndef TARGET_UNIX
-            pInteropInfo = (InteropSyncBlockInfo *)InterlockedPopEntrySList(&InteropSyncBlockInfo::s_InteropInfoStandbyList);
-
-            if (pInteropInfo != NULL)
-            {
-                // cache hit - reinitialize the data structure
-                new (pInteropInfo) InteropSyncBlockInfo();
-            }
-            else
-#endif // !TARGET_UNIX
-            {
-                pInteropInfo = new InteropSyncBlockInfo();
-            }
+            NewHolder<InteropSyncBlockInfo> pInteropInfo = new InteropSyncBlockInfo();
 
             if (SetInteropInfo(pInteropInfo))
                 pInteropInfo.SuppressRelease();

src/coreclr/vm/threads.cpp

@@ -6867,14 +6867,6 @@ void Thread::DoExtraWorkForFinalizer()
 
     if (RequireSyncBlockCleanup())
     {
-#ifndef TARGET_UNIX
-        InteropSyncBlockInfo::FlushStandbyList();
-#endif // !TARGET_UNIX
-
-#ifdef FEATURE_COMINTEROP
-        RCW::FlushStandbyList();
-#endif // FEATURE_COMINTEROP
-
         SyncBlockCache::GetSyncBlockCache()->CleanupSyncBlocks();
     }
     if (SystemDomain::System()->RequireAppDomainCleanup())

src/libraries/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs

@@ -8,6 +8,15 @@ namespace System.Diagnostics
 {
     public static partial class Debugger
     {
+        /// <summary>
+        /// Represents the default category of message with a constant.
+        /// </summary>
+        /// <remarks>
+        /// The value of this default constant is `null`. <see cref="Debugger.DefaultCategory"/>
+        /// is used by <see cref="Debugger.Log"/>.
+        /// </remarks>
+        public static readonly string? DefaultCategory;
+
         /// <summary>
         /// Signals a breakpoint to an attached debugger with the <paramref name="exception"/> details
         /// if a .NET debugger is attached with break on user-unhandled exception enabled and a method

src/mono/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs

@@ -7,8 +7,6 @@ namespace System.Diagnostics
 {
     public static partial class Debugger
     {
-        public static readonly string? DefaultCategory;
-
         public static bool IsAttached => IsAttached_internal();
 
         [MethodImplAttribute(MethodImplOptions.InternalCall)]