[Local GC] Remove static fields from GC interface (dotnet/coreclr#10566)
authorSean Gillespie <sean@swgillespie.me>
Sat, 1 Apr 2017 21:25:07 +0000 (14:25 -0700)
committerGitHub <noreply@github.com>
Sat, 1 Apr 2017 21:25:07 +0000 (14:25 -0700)
* Remove max_generation from GC interface

* [Local GC] Clean up the GC interface by removing all static fields
from it:
  1) gcHeapType is replaced by g_heap_type (EE side) and g_gc_heap_type
     (GC side), each of which is set on initialization. g_heap_type is
     read by the DAC to determine what kind of heap is being used.
  2) maxGeneration is not necessary due to the GC DAC changes.

* Rebase against master

* Comments and cleanup

* Use a heap allocation instead of alloca

* Code review feedback: remove a redundant cast and allocate generation count table once, on first use

* Address code review feedback: cache some calls to GetMaxGeneration

Commit migrated from https://github.com/dotnet/coreclr/commit/ed57233f4d26114d45b1293ced80a09f879f72cf

17 files changed:
src/coreclr/src/debug/daccess/enummem.cpp
src/coreclr/src/debug/daccess/request.cpp
src/coreclr/src/debug/daccess/request_svr.cpp
src/coreclr/src/gc/gc.cpp
src/coreclr/src/gc/gc.h
src/coreclr/src/gc/gccommon.cpp
src/coreclr/src/gc/gcinterface.dac.h
src/coreclr/src/gc/gcinterface.dacvars.def
src/coreclr/src/gc/gcinterface.h
src/coreclr/src/inc/dacvars.h
src/coreclr/src/vm/ceemain.cpp
src/coreclr/src/vm/crossgencompile.cpp
src/coreclr/src/vm/gcenv.ee.cpp
src/coreclr/src/vm/gcheaputilities.cpp
src/coreclr/src/vm/gcheaputilities.h
src/coreclr/src/vm/threads.cpp
src/coreclr/src/vm/threads.h

index 0dc7a42..027fe59 100644 (file)
@@ -317,12 +317,10 @@ HRESULT ClrDataAccess::EnumMemCLRStatic(IN CLRDataEnumMemoryFlags flags)
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pFinalizerThread.EnumMem(); )
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED( g_pSuspensionThread.EnumMem(); )
     
-#ifdef FEATURE_SVR_GC
     CATCH_ALL_EXCEPT_RETHROW_COR_E_OPERATIONCANCELLED
     (
-        IGCHeap::gcHeapType.EnumMem();
+        g_heap_type.EnumMem();
     );
-#endif // FEATURE_SVR_GC
 
     m_dumpStats.m_cbClrStatics = m_cbMemoryReported - cbMemoryReported;
 
index c52b6cd..78ac831 100644 (file)
@@ -2929,31 +2929,33 @@ ClrDataAccess::GetGCHeapData(struct DacpGcHeapData *gcheapData)
 
     SOSDacEnter();
 
-    // for server GC-capable builds only, we need to check and see if IGCHeap::gcHeapType
+    // we need to check and see if g_heap_type
     // is GC_HEAP_INVALID, in which case we fail.
-    // IGCHeap::gcHeapType doesn't exist on non-server-GC capable builds.
-#ifdef FEATURE_SVR_GC
-    ULONG32 gcHeapValue = *g_gcDacGlobals->gc_heap_type;
+    ULONG32 gcHeapValue = g_heap_type;
 
     // GC_HEAP_TYPE has three possible values:
     //       GC_HEAP_INVALID = 0,
     //       GC_HEAP_WKS     = 1,
     //       GC_HEAP_SVR     = 2
     // If we get something other than that, we probably read the wrong location.
-    _ASSERTE(gcHeapValue >= IGCHeap::GC_HEAP_INVALID && gcHeapValue <= IGCHeap::GC_HEAP_SVR);
+    _ASSERTE(gcHeapValue >= GC_HEAP_INVALID && gcHeapValue <= GC_HEAP_SVR);
 
-    // we have GC_HEAP_INVALID if gcHeapValue == 0, so we're done
-    if (gcHeapValue == IGCHeap::GC_HEAP_INVALID)
+    // we have GC_HEAP_INVALID if gcHeapValue == 0, so we're done - we haven't
+    // initialized the heap yet.
+    if (gcHeapValue == GC_HEAP_INVALID)
     {
         hr = E_FAIL;
         goto cleanup;
     }
-#endif
 
     // Now we can get other important information about the heap
+    // We can use GCHeapUtilities::IsServerHeap here because we have already validated
+    // that the heap is in a valid state. We couldn't use it above, because IsServerHeap
+    // asserts if the heap type is GC_HEAP_INVALID.
     gcheapData->g_max_generation = *g_gcDacGlobals->max_gen;
-    gcheapData->bServerMode = gcHeapValue == IGCHeap::GC_HEAP_SVR;
+    gcheapData->bServerMode = GCHeapUtilities::IsServerHeap();
     gcheapData->bGcStructuresValid = *g_gcDacGlobals->gc_structures_invalid_cnt == 0;
+
     if (GCHeapUtilities::IsServerHeap())
     {
 #if !defined (FEATURE_SVR_GC)
@@ -2968,10 +2970,8 @@ ClrDataAccess::GetGCHeapData(struct DacpGcHeapData *gcheapData)
         gcheapData->HeapCount = 1;
     }
 
-#ifdef FEATURE_SVR_GC
 cleanup:
     ;
-#endif
 
     SOSDacLeave();
     return hr;
@@ -3069,7 +3069,7 @@ ClrDataAccess::GetGCInterestingInfoStaticData(struct DacpGCInterestingInfoData *
     SOSDacEnter();
     memset(data, 0, sizeof(DacpGCInterestingInfoData));
 
-    if (*g_gcDacGlobals->gc_heap_type != IGCHeap::GC_HEAP_SVR)
+    if (g_heap_type != GC_HEAP_SVR)
     {
         for (int i = 0; i < NUM_GC_DATA_POINTS; i++)
             data->interestingDataPoints[i] = g_gcDacGlobals->interesting_data_per_heap[i];
index 930a35b..6a1de35 100644 (file)
@@ -240,7 +240,7 @@ ClrDataAccess::EnumSvrGlobalMemoryRegions(CLRDataEnumMemoryFlags flags)
 
 DWORD DacGetNumHeaps()
 {
-    if (g_gcDacGlobals->gc_heap_type == IGCHeap::GC_HEAP_SVR)
+    if (g_heap_type == GC_HEAP_SVR)
         return (DWORD)*g_gcDacGlobals->n_heaps;
 
     // workstation gc
index 515069b..d8a59cd 100644 (file)
@@ -36946,10 +36946,7 @@ void PopulateDacVars(GcDacVars *gcDacVars)
     gcDacVars->build_variant = &g_build_variant;
     gcDacVars->gc_structures_invalid_cnt = const_cast<int32_t*>(&GCScan::m_GcStructuresInvalidCnt);
     gcDacVars->generation_size = sizeof(generation);
-#ifdef FEATURE_SVR_GC
-    gcDacVars->gc_heap_type = &IGCHeap::gcHeapType;
-#endif // FEATURE_SVR_GC
-    gcDacVars->max_gen = &IGCHeap::maxGeneration;
+    gcDacVars->max_gen = &g_max_generation;
 #ifndef MULTIPLE_HEAPS
     gcDacVars->mark_array = &gc_heap::mark_array;
     gcDacVars->ephemeral_heap_segment = reinterpret_cast<dac_heap_segment**>(&gc_heap::ephemeral_heap_segment);
index ecb791f..821b21d 100644 (file)
@@ -87,6 +87,7 @@ class IGCHeapInternal;
 
 /* misc defines */
 #define LARGE_OBJECT_SIZE ((size_t)(85000))
+#define max_generation 2
 
 #ifdef GC_CONFIG_DRIVEN
 #define MAX_GLOBAL_GC_MECHANISMS_COUNT 6
@@ -110,6 +111,8 @@ extern "C" uint32_t* g_gc_card_bundle_table;
 extern "C" uint32_t* g_gc_card_table;
 extern "C" uint8_t* g_gc_lowest_address;
 extern "C" uint8_t* g_gc_highest_address;
+extern "C" GCHeapType g_gc_heap_type;
+extern "C" uint32_t g_max_generation;
 
 ::IGCHandleTable*  CreateGCHandleTable();
 
@@ -219,7 +222,7 @@ public:
 
     unsigned GetMaxGeneration()
     {
-        return IGCHeap::maxGeneration;
+        return max_generation;
     }
 
     bool IsValidSegmentSize(size_t cbSize)
@@ -235,8 +238,6 @@ public:
 
     BOOL IsLargeObject(MethodTable *mt)
     {
-        WRAPPER_NO_CONTRACT;
-
         return mt->GetBaseSize() >= LARGE_OBJECT_SIZE;
     }
 
@@ -270,18 +271,15 @@ extern IGCHandleTable* g_theGCHandleTable;
 #ifndef DACCESS_COMPILE
 inline bool IsGCInProgress(bool bConsiderGCStart = false)
 {
-    WRAPPER_NO_CONTRACT;
-
     return g_theGCHeap != nullptr ? g_theGCHeap->IsGCInProgressHelper(bConsiderGCStart) : false;
 }
 #endif // DACCESS_COMPILE
 
 inline bool IsServerHeap()
 {
-    LIMITED_METHOD_CONTRACT;
 #ifdef FEATURE_SVR_GC
-    _ASSERTE(IGCHeap::gcHeapType != IGCHeap::GC_HEAP_INVALID);
-    return (IGCHeap::gcHeapType == IGCHeap::GC_HEAP_SVR);
+    assert(g_gc_heap_type != GC_HEAP_INVALID);
+    return g_gc_heap_type == GC_HEAP_SVR;
 #else // FEATURE_SVR_GC
     return false;
 #endif // FEATURE_SVR_GC
index 8f6d1ac..1cd3c82 100644 (file)
 #include "gcenv.h"
 #include "gc.h"
 
-#ifdef FEATURE_SVR_GC
-SVAL_IMPL_INIT(uint32_t,IGCHeap,gcHeapType,IGCHeap::GC_HEAP_INVALID);
-#endif // FEATURE_SVR_GC
-
-SVAL_IMPL_INIT(uint32_t,IGCHeap,maxGeneration,2);
-
 IGCHeapInternal* g_theGCHeap;
 IGCHandleTable* g_theGCHandleTable;
 
@@ -47,6 +41,8 @@ uint32_t* g_gc_card_bundle_table;
 
 uint8_t* g_gc_lowest_address  = 0;
 uint8_t* g_gc_highest_address = 0;
+GCHeapType g_gc_heap_type = GC_HEAP_INVALID;
+uint32_t g_max_generation = max_generation;
 
 #ifdef GC_CONFIG_DRIVEN
 void record_global_mechanism (int mech_index)
@@ -122,9 +118,9 @@ void InitializeHeapType(bool bServerHeap)
 {
     LIMITED_METHOD_CONTRACT;
 #ifdef FEATURE_SVR_GC
-    IGCHeap::gcHeapType = bServerHeap ? IGCHeap::GC_HEAP_SVR : IGCHeap::GC_HEAP_WKS;
+    g_gc_heap_type = bServerHeap ? GC_HEAP_SVR : GC_HEAP_WKS;
 #ifdef WRITE_BARRIER_CHECK
-    if (IGCHeap::gcHeapType == IGCHeap::GC_HEAP_SVR)
+    if (g_gc_heap_type == GC_HEAP_SVR)
     {
         g_GCShadow = 0;
         g_GCShadowEnd = 0;
@@ -163,9 +159,9 @@ bool InitializeGarbageCollector(IGCToCLR* clrToGC, IGCHeap** gcHeap, IGCHandleTa
     }
 
 #ifdef FEATURE_SVR_GC
-    assert(IGCHeap::gcHeapType != IGCHeap::GC_HEAP_INVALID);
+    assert(g_gc_heap_type != GC_HEAP_INVALID);
 
-    if (IGCHeap::gcHeapType == IGCHeap::GC_HEAP_SVR)
+    if (g_gc_heap_type == GC_HEAP_SVR)
     {
         heap = SVR::CreateGCHeap();
         SVR::PopulateDacVars(gcDacVars);
index 84c8ffb..647101f 100644 (file)
 #define MAX_GLOBAL_GC_MECHANISMS_COUNT 6
 #define NUMBERGENERATIONS              4
 
-// TODO(segilles) - Implement this scheme for Server GC
-namespace SVR {
-    class heap_segment;
-    class gc_heap;
-}
-
 // Analogue for the GC heap_segment class, containing information regarding a single
 // heap segment.
 class dac_heap_segment {
index f9c2078..b788079 100644 (file)
@@ -38,7 +38,6 @@ GC_DAC_VAR        (uint8_t,               build_variant)
 GC_DAC_VAR        (bool,                  built_with_svr)
 GC_DAC_ARRAY_VAR  (size_t,                gc_global_mechanisms)
 GC_DAC_ARRAY_VAR  (dac_generation,        generation_table)
-GC_DAC_VAR        (uint32_t,              gc_heap_type)
 GC_DAC_VAR        (uint32_t,              max_gen)
 GC_DAC_PTR_VAR    (uint32_t,              mark_array)
 GC_DAC_VAR        (c_gc_state,            current_c_gc_state)
index b5a9e0e..66d8b31 100644 (file)
@@ -163,8 +163,6 @@ struct segment_info
 // one for the object header, and one for the first field in the object.
 #define min_obj_size ((sizeof(uint8_t*) + sizeof(uintptr_t) + sizeof(size_t)))
 
-#define max_generation 2
-
 // The bit shift used to convert a memory address into an index into the
 // Software Write Watch table.
 #define SOFTWARE_WRITE_WATCH_AddressToTableByteIndexShift 0xc
@@ -379,6 +377,13 @@ typedef enum
     HNDTYPE_WEAK_WINRT   = 9
 } HandleType;
 
+typedef enum
+{
+    GC_HEAP_INVALID = 0,
+    GC_HEAP_WKS     = 1,
+    GC_HEAP_SVR     = 2
+} GCHeapType;
+
 typedef bool (* walk_fn)(Object*, void*);
 typedef void (* gen_walk_fn)(void* context, int generation, uint8_t* range_start, uint8_t* range_end, uint8_t* range_reserved);
 typedef void (* record_surv_fn)(uint8_t* begin, uint8_t* end, ptrdiff_t reloc, void* context, bool compacting_p, bool bgc_p);
@@ -729,19 +734,6 @@ public:
 
     IGCHeap() {}
     virtual ~IGCHeap() {}
-
-    typedef enum
-    {
-        GC_HEAP_INVALID = 0,
-        GC_HEAP_WKS     = 1,
-        GC_HEAP_SVR     = 2
-    } GC_HEAP_TYPE;
-
-#ifdef FEATURE_SVR_GC
-    SVAL_DECL(uint32_t, gcHeapType);
-#endif
-
-    SVAL_DECL(uint32_t, maxGeneration);
 };
 
 #ifdef WRITE_BARRIER_CHECK
index ab3bca2..4b7b7b1 100644 (file)
@@ -121,12 +121,8 @@ DEFINE_DACVAR(ULONG, int, dac__HillClimbingLogSize, ::HillClimbingLogSize)
 DEFINE_DACVAR(ULONG, PTR_Thread, dac__g_pFinalizerThread, ::g_pFinalizerThread)
 DEFINE_DACVAR(ULONG, PTR_Thread, dac__g_pSuspensionThread, ::g_pSuspensionThread)
 
-#ifdef FEATURE_SVR_GC
-DEFINE_DACVAR(ULONG, DWORD, IGCHeap__gcHeapType, IGCHeap::gcHeapType)
-#endif // FEATURE_SVR_GC
-
+DEFINE_DACVAR(ULONG, DWORD, dac__g_heap_type, g_heap_type)
 DEFINE_DACVAR(ULONG, PTR_GcDacVars, dac__g_gcDacGlobals, g_gcDacGlobals)
-DEFINE_DACVAR(ULONG, DWORD, IGCHeap__maxGeneration, IGCHeap::maxGeneration)
 
 DEFINE_DACVAR(ULONG, PTR_SystemDomain, SystemDomain__m_pSystemDomain, SystemDomain::m_pSystemDomain)
 DEFINE_DACVAR(ULONG, ArrayListStatic, SystemDomain__m_appDomainIndexList, SystemDomain::m_appDomainIndexList)
index 602ccc9..b8c1387 100644 (file)
@@ -483,6 +483,7 @@ void InitializeStartupFlags()
 
 
     InitializeHeapType((flags & STARTUP_SERVER_GC) != 0);
+    g_heap_type = (flags & STARTUP_SERVER_GC) == 0 ? GC_HEAP_WKS : GC_HEAP_SVR;
 
 #ifdef FEATURE_LOADER_OPTIMIZATION            
     g_dwGlobalSharePolicy = (flags&STARTUP_LOADER_OPTIMIZATION_MASK)>>1;
index c29d3cc..b106ecc 100644 (file)
@@ -133,9 +133,7 @@ BOOL g_fEEComActivatedStartup=FALSE;
 
 GVAL_IMPL_INIT(DWORD, g_fHostConfig, 0);
 
-#ifdef FEATURE_SVR_GC
-SVAL_IMPL_INIT(uint32_t,IGCHeap,gcHeapType,IGCHeap::GC_HEAP_WKS);
-#endif
+GVAL_IMPL_INIT(GCHeapType, g_heap_type, GC_HEAP_WKS);
 
 void UpdateGCSettingFromHost()
 {
index e861995..cf2728d 100644 (file)
@@ -1016,6 +1016,7 @@ void GCProfileWalkHeapWorker(BOOL fProfilerPinned, BOOL fShouldWalkHeapRootsForE
 {
     {
         ProfilingScanContext SC(fProfilerPinned);
+        unsigned max_generation = GCHeapUtilities::GetGCHeap()->GetMaxGeneration();
 
         // **** Scan roots:  Only scan roots if profiling API wants them or ETW wants them.
         if (fProfilerPinned || fShouldWalkHeapRootsForEtw)
index 7307289..91640d3 100644 (file)
@@ -14,6 +14,7 @@
 GPTR_IMPL_INIT(uint32_t, g_card_table,      nullptr);
 GPTR_IMPL_INIT(uint8_t,  g_lowest_address,  nullptr);
 GPTR_IMPL_INIT(uint8_t,  g_highest_address, nullptr);
+GVAL_IMPL_INIT(GCHeapType, g_heap_type,     GC_HEAP_INVALID);
 uint8_t* g_ephemeral_low  = (uint8_t*)1;
 uint8_t* g_ephemeral_high = (uint8_t*)~0;
 
index 064165b..baa2558 100644 (file)
@@ -16,6 +16,7 @@ extern "C" {
 GPTR_DECL(uint8_t,g_lowest_address);
 GPTR_DECL(uint8_t,g_highest_address);
 GPTR_DECL(uint32_t,g_card_table);
+GVAL_DECL(GCHeapType, g_heap_type);
 #ifndef DACCESS_COMPILE
 }
 #endif // !DACCESS_COMPILE
@@ -120,10 +121,11 @@ public:
     inline static bool IsServerHeap()
     {
         LIMITED_METHOD_CONTRACT;
+
 #ifdef FEATURE_SVR_GC
-        _ASSERTE(IGCHeap::gcHeapType != IGCHeap::GC_HEAP_INVALID);
-        return (IGCHeap::gcHeapType == IGCHeap::GC_HEAP_SVR);
-#else // FEATURE_SVR_GC
+        _ASSERTE(g_heap_type != GC_HEAP_INVALID);
+        return g_heap_type == GC_HEAP_SVR;
+#else
         return false;
 #endif // FEATURE_SVR_GC
     }
index da805bd..68a81bf 100644 (file)
@@ -5750,6 +5750,7 @@ void ThreadStore::InitThreadStore()
     }
     s_DeadThreadGCTriggerPeriodMilliseconds =
         CLRConfig::GetConfigValue(CLRConfig::INTERNAL_Thread_DeadThreadGCTriggerPeriodMilliseconds);
+    s_DeadThreadGenerationCounts = nullptr;
 }
 
 // Enter and leave the critical section around the thread store.  Clients should
@@ -5986,6 +5987,7 @@ void ThreadStore::TransferStartedThread(Thread *thread, BOOL bRequiresTSL)
 
 LONG ThreadStore::s_DeadThreadCountThresholdForGCTrigger = 0;
 DWORD ThreadStore::s_DeadThreadGCTriggerPeriodMilliseconds = 0;
+SIZE_T *ThreadStore::s_DeadThreadGenerationCounts = nullptr;
 
 void ThreadStore::IncrementDeadThreadCountForGCTrigger()
 {
@@ -6012,7 +6014,7 @@ void ThreadStore::IncrementDeadThreadCountForGCTrigger()
         return;
     }
 
-    SIZE_T gcLastMilliseconds = gcHeap->GetLastGCStartTime(max_generation);
+    SIZE_T gcLastMilliseconds = gcHeap->GetLastGCStartTime(gcHeap->GetMaxGeneration());
     SIZE_T gcNowMilliseconds = gcHeap->GetNow();
     if (gcNowMilliseconds - gcLastMilliseconds < s_DeadThreadGCTriggerPeriodMilliseconds)
     {
@@ -6088,11 +6090,22 @@ void ThreadStore::TriggerGCForDeadThreadsIfNecessary()
         return;
     }
 
-    int gcGenerationToTrigger = 0;
+    unsigned gcGenerationToTrigger = 0;
     IGCHeap *gcHeap = GCHeapUtilities::GetGCHeap();
     _ASSERTE(gcHeap != nullptr);
     SIZE_T generationCountThreshold = static_cast<SIZE_T>(s_DeadThreadCountThresholdForGCTrigger) / 2;
-    SIZE_T newDeadThreadGenerationCounts[max_generation + 1] = {0};
+    unsigned maxGeneration = gcHeap->GetMaxGeneration();
+    if (!s_DeadThreadGenerationCounts)
+    {
+        // initialize this field on first use with an entry for every table.
+        s_DeadThreadGenerationCounts = new (nothrow) SIZE_T[maxGeneration + 1];
+        if (!s_DeadThreadGenerationCounts)
+        {
+            return;
+        }
+    }
+
+    memset(s_DeadThreadGenerationCounts, 0, sizeof(SIZE_T) * (maxGeneration + 1));
     {
         ThreadStoreLockHolder threadStoreLockHolder;
         GCX_COOP();
@@ -6114,12 +6127,12 @@ void ThreadStore::TriggerGCForDeadThreadsIfNecessary()
                 continue;
             }
 
-            int exposedObjectGeneration = gcHeap->WhichGeneration(exposedObject);
-            SIZE_T newDeadThreadGenerationCount = ++newDeadThreadGenerationCounts[exposedObjectGeneration];
+            unsigned exposedObjectGeneration = gcHeap->WhichGeneration(exposedObject);
+            SIZE_T newDeadThreadGenerationCount = ++s_DeadThreadGenerationCounts[exposedObjectGeneration];
             if (exposedObjectGeneration > gcGenerationToTrigger && newDeadThreadGenerationCount >= generationCountThreshold)
             {
                 gcGenerationToTrigger = exposedObjectGeneration;
-                if (gcGenerationToTrigger >= max_generation)
+                if (gcGenerationToTrigger >= maxGeneration)
                 {
                     break;
                 }
@@ -6153,8 +6166,8 @@ void ThreadStore::TriggerGCForDeadThreadsIfNecessary()
                 continue;
             }
 
-            if (gcGenerationToTrigger < max_generation &&
-                static_cast<int>(gcHeap->WhichGeneration(exposedObject)) > gcGenerationToTrigger)
+            if (gcGenerationToTrigger < maxGeneration &&
+                gcHeap->WhichGeneration(exposedObject) > gcGenerationToTrigger)
             {
                 continue;
             }
index f34066f..697ffea 100644 (file)
@@ -5555,6 +5555,7 @@ private:
 private:
     static LONG s_DeadThreadCountThresholdForGCTrigger;
     static DWORD s_DeadThreadGCTriggerPeriodMilliseconds;
+    static SIZE_T *s_DeadThreadGenerationCounts;
 
 public: