Adding SetGcNotification API and related runtime simplification

docs/design/datacontracts/Notifications.md

@@ -0,0 +1,36 @@
+# Contract Notifications
+
+This contract is for debugger notifications.
+
+## APIs of contract
+
+``` csharp
+// Set the GC notification for condemned generations
+// The argument is a bitmask where the i-th bit set represents the i-th generation.
+void SetGcNotification(int condemnedGeneration);
+```
+
+## Version 1
+
+Data descriptors used: none
+
+Global variables used:
+| Global Name | Type | Purpose |
+| --- | --- | --- |
+| `GcNotificationFlags` | TargetPointer | Global flag for storing GC notification data |
+
+Contracts used: none
+
+``` csharp
+void SetGcNotification(int condemnedGeneration)
+{
+    TargetPointer pGcNotificationFlags = _target.ReadGlobalPointer("GcNotificationFlags");
+    uint currentFlags = _target.Read<uint>(pGcNotificationFlags);
+    if (condemnedGeneration == 0)
+        _target.Write<uint>(pGcNotificationFlags, 0);
+    else
+    {
+        _target.Write<uint>(pGcNotificationFlags, (uint)(currentFlags | condemnedGeneration));
+    }
+}
+```

src/coreclr/debug/daccess/daccess.cpp

@@ -3103,7 +3103,6 @@ ClrDataAccess::ClrDataAccess(ICorDebugDataTarget * pTarget, ICLRDataTarget * pLe
     m_logMessageCb = NULL;
     m_enumMemFlags = (CLRDataEnumMemoryFlags)-1;    // invalid
     m_jitNotificationTable = NULL;
-    m_gcNotificationTable  = NULL;
 
 #ifdef FEATURE_MINIMETADATA_IN_TRIAGEDUMPS
     m_streams = NULL;
@@ -5326,31 +5325,8 @@ ClrDataAccess::GetGcNotification(GcEvtArgs* gcEvtArgs)
 
     EX_TRY
     {
-        if (gcEvtArgs->typ >= GC_EVENT_TYPE_MAX)
-        {
-            status = E_INVALIDARG;
-        }
-        else
-        {
-            GcNotifications gn(GetHostGcNotificationTable());
-            if (!gn.IsActive())
-            {
-                status = E_OUTOFMEMORY;
-            }
-            else
-            {
-                GcEvtArgs *res = gn.GetNotification(*gcEvtArgs);
-                if (res != NULL)
-                {
-                    *gcEvtArgs = *res;
-                    status = S_OK;
-                }
-                else
-                {
-                    status = E_FAIL;
-                }
-            }
-        }
+        // XXX Microsoft.
+        status = E_NOTIMPL;
     }
     EX_CATCH
     {
@@ -5380,22 +5356,8 @@ ClrDataAccess::SetGcNotification(IN GcEvtArgs gcEvtArgs)
         }
         else
         {
-            GcNotifications gn(GetHostGcNotificationTable());
-            if (!gn.IsActive())
-            {
-                status = E_OUTOFMEMORY;
-            }
-            else
-            {
-                if (gn.SetNotification(gcEvtArgs) && gn.UpdateOutOfProcTable())
-                {
-                    status = S_OK;
-                }
-                else
-                {
-                    status = E_FAIL;
-                }
-            }
+            GcNotifications::SetNotification(gcEvtArgs);
+            status = S_OK;
         }
     }
     EX_CATCH
@@ -6398,18 +6360,6 @@ ClrDataAccess::GetHostJitNotificationTable()
     return m_jitNotificationTable;
 }
 
-GcNotification*
-ClrDataAccess::GetHostGcNotificationTable()
-{
-    if (m_gcNotificationTable == NULL)
-    {
-        m_gcNotificationTable =
-            GcNotifications::InitializeNotificationTable(128);
-    }
-
-    return m_gcNotificationTable;
-}
-
 /* static */ bool
 ClrDataAccess::GetMetaDataFileInfoFromPEFile(PEAssembly *pPEAssembly,
                                              DWORD &dwTimeStamp,

src/coreclr/debug/daccess/dacimpl.h

@@ -1356,7 +1356,6 @@ class ClrDataAccess
 
     void ClearDumpStats();
     JITNotification* GetHostJitNotificationTable();
-    GcNotification*  GetHostGcNotificationTable();
 
     void* GetMetaDataFromHost(PEAssembly* pPEAssembly);
 
@@ -1471,7 +1470,6 @@ class ClrDataAccess
     ICLRDataLoggingCallback* m_logMessageCb;
     CLRDataEnumMemoryFlags m_enumMemFlags;
     JITNotification* m_jitNotificationTable;
-    GcNotification*  m_gcNotificationTable;
     TSIZE_T m_cbMemoryReported;
     DumpMemoryReportStatics m_dumpStats;
 

src/coreclr/inc/dacvars.h

@@ -132,7 +132,8 @@ DEFINE_DACVAR(BOOL, CodeVersionManager__s_HasNonDefaultILVersions, CodeVersionMa
 
 DEFINE_DACVAR(PTR_JITNotification, dac__g_pNotificationTable, ::g_pNotificationTable)
 DEFINE_DACVAR(ULONG32, dac__g_dacNotificationFlags, ::g_dacNotificationFlags)
-DEFINE_DACVAR(PTR_GcNotification, dac__g_pGcNotificationTable, ::g_pGcNotificationTable)
+
+DEFINE_DACVAR(DWORD, dac__g_gcNotificationFlags, g_gcNotificationFlags)
 
 DEFINE_DACVAR(PTR_EEConfig, dac__g_pConfig, ::g_pConfig)
 

src/coreclr/vm/datadescriptor/datadescriptor.inc

@@ -1025,6 +1025,7 @@ CDAC_GLOBAL_POINTER(MiniMetaDataBuffMaxSize, &::g_MiniMetaDataBuffMaxSize)
 CDAC_GLOBAL_POINTER(DacNotificationFlags, &::g_dacNotificationFlags)
 CDAC_GLOBAL_POINTER(OffsetOfCurrentThreadInfo, &::g_offsetOfCurrentThreadInfo)
 CDAC_GLOBAL_POINTER(ThinlockThreadIdDispenser, &::g_pThinLockThreadIdDispenser)
+CDAC_GLOBAL_POINTER(GcNotificationFlags, &::g_gcNotificationFlags)
 CDAC_GLOBAL_POINTER(CoreLib, &::g_CoreLib)
 #ifdef TARGET_WINDOWS
 CDAC_GLOBAL_POINTER(TlsIndexBase, &::_tls_index)
@@ -1060,6 +1061,7 @@ CDAC_GLOBAL_CONTRACT(EcmaMetadata, 1)
 CDAC_GLOBAL_CONTRACT(Exception, 1)
 CDAC_GLOBAL_CONTRACT(ExecutionManager, 2)
 CDAC_GLOBAL_CONTRACT(Loader, 1)
+CDAC_GLOBAL_CONTRACT(Notifications, 1)
 CDAC_GLOBAL_CONTRACT(Object, 1)
 CDAC_GLOBAL_CONTRACT(PlatformMetadata, 1)
 CDAC_GLOBAL_CONTRACT(PrecodeStubs, 3)

src/coreclr/vm/gcenv.ee.cpp

@@ -1724,18 +1724,8 @@ bool GCToEEInterface::AnalyzeSurvivorsRequested(int condemnedGeneration)
         analysisTimer.Start();
     }
 
-    // Is the list active?
-    GcNotifications gn(g_pGcNotificationTable);
-    if (gn.IsActive())
-    {
-        GcEvtArgs gea = { GC_MARK_END, { (1<<condemnedGeneration) } };
-        if (gn.GetNotification(gea) != 0)
-        {
-            return true;
-        }
-    }
-
-    return false;
+    GcEvtArgs gea = { GC_MARK_END, { (1<<condemnedGeneration) } };
+    return GcNotifications::GetNotification(gea);
 }
 
 void GCToEEInterface::AnalyzeSurvivorsFinished(size_t gcIndex, int condemnedGeneration, uint64_t promoted_bytes, void (*reportGenerationBounds)())
@@ -1749,15 +1739,10 @@ void GCToEEInterface::AnalyzeSurvivorsFinished(size_t gcIndex, int condemnedGene
         elapsed = analysisTimer.Elapsed100nsTicks();
     }
 
-    // Is the list active?
-    GcNotifications gn(g_pGcNotificationTable);
-    if (gn.IsActive())
+    GcEvtArgs gea = { GC_MARK_END, { (1<<condemnedGeneration) } };
+    if (GcNotifications::GetNotification(gea))
     {
-        GcEvtArgs gea = { GC_MARK_END, { (1<<condemnedGeneration) } };
-        if (gn.GetNotification(gea) != 0)
-        {
-            DACNotify::DoGCNotification(gea);
-        }
+        DACNotify::DoGCNotification(gea);
     }
 
     if (gcGenAnalysisState == GcGenAnalysisState::Enabled)

src/coreclr/vm/util.cpp

@@ -1016,6 +1016,33 @@ BOOL CLRFreeLibrary(HMODULE hModule)
 
 
 #endif // #ifndef DACCESS_COMPILE
+namespace GcNotifications
+{
+    VOID SetNotification(GcEvtArgs ev)
+    {
+        LIMITED_METHOD_CONTRACT;
+        if (ev.typ == GC_MARK_END)
+        {
+            if (ev.condemnedGeneration == 0)
+                g_gcNotificationFlags = 0;
+            else
+                g_gcNotificationFlags = (DWORD)(g_gcNotificationFlags | ev.condemnedGeneration);
+        }
+    }
+    BOOL GetNotification(GcEvtArgs ev)
+    {
+        LIMITED_METHOD_CONTRACT;
+        // check to see if we have a notification for this generation
+        if (ev.typ == GC_MARK_END && (ev.condemnedGeneration == 0 || (g_gcNotificationFlags & ev.condemnedGeneration) != 0))
+        {
+            return TRUE;
+        }
+        else
+        {
+            return FALSE;
+        }
+    }
+};
 
 GPTR_IMPL(JITNotification, g_pNotificationTable);
 GVAL_IMPL(ULONG32, g_dacNotificationFlags);
@@ -1320,155 +1347,10 @@ BOOL JITNotifications::UpdateOutOfProcTable()
 }
 #endif // DACCESS_COMPILE
 
-GPTR_IMPL(GcNotification, g_pGcNotificationTable);
-
-GcNotifications::GcNotifications(GcNotification *gcTable)
-{
-    LIMITED_METHOD_CONTRACT;
-    if (gcTable)
-    {
-        // Bookkeeping info is held in the first slot
-        m_gcTable = gcTable + 1;
-    }
-    else
-    {
-        m_gcTable = NULL;
-    }
-}
-
-BOOL GcNotifications::FindItem(GcEvtArgs ev_, UINT *indexOut)
-{
-    LIMITED_METHOD_CONTRACT;
-    if (m_gcTable == NULL)
-    {
-        return FALSE;
-    }
-
-    if (indexOut == NULL)
-    {
-        return FALSE;
-    }
-
-    UINT length = Length();
-    for (UINT i = 0; i < length; i++)
-    {
-        if (m_gcTable[i].IsMatch(ev_))
-        {
-            *indexOut = i;
-            return TRUE;
-        }
-    }
-
-    return FALSE;
-}
-
-
-BOOL GcNotifications::SetNotification(GcEvtArgs ev)
-{
-    if (!IsActive())
-    {
-        return FALSE;
-    }
-
-    if (ev.typ < 0 || ev.typ >= GC_EVENT_TYPE_MAX)
-    {
-        return FALSE;
-    }
-
-    // build the "match" event
-    GcEvtArgs evStar = { ev.typ };
-    switch (ev.typ)
-    {
-        case GC_MARK_END:
-            // specify mark event matching all generations
-            evStar.condemnedGeneration = -1;
-            break;
-        default:
-            break;
-    }
-
-    // look for the entry that matches the evStar argument
-    UINT idx;
-    if (!FindItem(evStar, &idx))
-    {
-        // Find first free item
-        UINT iFirstFree = Length();
-        for (UINT i = 0; i < iFirstFree; i++)
-        {
-            GcNotification *pCurrent = m_gcTable + i;
-            if (pCurrent->IsFree())
-            {
-                iFirstFree = i;
-                break;
-            }
-        }
-
-        if (iFirstFree == Length() &&
-            iFirstFree == GetTableSize())
-    {
-            // No more room
-        return FALSE;
-    }
-
-        // guarantee the free cell is zeroed out
-        m_gcTable[iFirstFree].SetFree();
-        idx = iFirstFree;
-    }
-
-    // Now update the state
-    m_gcTable[idx].ev.typ = ev.typ;
-    switch (ev.typ)
-    {
-        case GC_MARK_END:
-            if (ev.condemnedGeneration == 0)
-            {
-                m_gcTable[idx].SetFree();
-            }
-            else
-            {
-                m_gcTable[idx].ev.condemnedGeneration |= ev.condemnedGeneration;
-            }
-            break;
-        default:
-            break;
-    }
-
-    // and if needed, update the array's length
-    if (idx == Length())
-    {
-        IncrementLength();
-    }
-
-    return TRUE;
-}
 
 GARY_IMPL(size_t, g_clrNotificationArguments, MAX_CLR_NOTIFICATION_ARGS);
 
-#ifdef DACCESS_COMPILE
-
-GcNotification *GcNotifications::InitializeNotificationTable(UINT TableSize)
-{
-    // We use the first entry in the table for recordkeeping info.
-
-    GcNotification *retTable = new (nothrow) GcNotification[TableSize+1];
-    if (retTable)
-    {
-        // Set the length
-        UINT *pUint = (UINT *) &(retTable[0].ev.typ);
-        *pUint = 0;
-        // Set the table size
-        ++pUint;
-        *pUint = TableSize;
-    }
-    return retTable;
-}
-
-BOOL GcNotifications::UpdateOutOfProcTable()
-{
-    return ::UpdateOutOfProcTable<GcNotification>(g_pGcNotificationTable, m_gcTable - 1, GetTableSize() + 1);
-}
-
-#else // DACCESS_COMPILE
+#ifndef DACCESS_COMPILE
 
 static CrstStatic g_clrNotificationCrst;