From e9bb3b18f69a6671431f8a90f5f491cf583cec85 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 8 Jul 2020 13:32:51 -0700 Subject: [PATCH] Delete dead code related to finalization (#38906) --- src/coreclr/src/gc/gc.cpp | 101 ++------------------------------- src/coreclr/src/gc/gcimpl.h | 4 -- src/coreclr/src/gc/gcinterface.h | 11 ---- src/coreclr/src/gc/gcpriv.h | 2 - src/coreclr/src/vm/finalizerthread.cpp | 84 +++++++-------------------- src/coreclr/src/vm/finalizerthread.h | 4 +- src/coreclr/src/vm/hosting.cpp | 36 +----------- src/coreclr/src/vm/spinlock.h | 1 - 8 files changed, 30 insertions(+), 213 deletions(-) diff --git a/src/coreclr/src/gc/gc.cpp b/src/coreclr/src/gc/gc.cpp index f978c4a..2384889 100644 --- a/src/coreclr/src/gc/gc.cpp +++ b/src/coreclr/src/gc/gc.cpp @@ -80,8 +80,6 @@ int compact_ratio = 0; // See comments in reset_memory. BOOL reset_mm_p = TRUE; -bool g_fFinalizerRunOnShutDown = false; - #ifdef FEATURE_SVR_GC bool g_built_with_svr_gc = true; #else @@ -37829,26 +37827,6 @@ size_t GCHeap::GetFinalizablePromotedCount() #endif //MULTIPLE_HEAPS } -bool GCHeap::ShouldRestartFinalizerWatchDog() -{ - // This condition was historically used as part of the condition to detect finalizer thread timeouts - return gc_heap::gc_lock.lock != -1; -} - -void GCHeap::SetFinalizeQueueForShutdown(bool fHasLock) -{ -#ifdef MULTIPLE_HEAPS - for (int hn = 0; hn < gc_heap::n_heaps; hn++) - { - gc_heap* hp = gc_heap::g_heaps [hn]; - hp->finalize_queue->SetSegForShutDown(fHasLock); - } - -#else //MULTIPLE_HEAPS - pGenGCHeap->finalize_queue->SetSegForShutDown(fHasLock); -#endif //MULTIPLE_HEAPS -} - //--------------------------------------------------------------------------- // Finalized class tracking //--------------------------------------------------------------------------- @@ -37981,18 +37959,9 @@ CFinalize::RegisterForFinalization (int gen, Object* obj, size_t size) } CONTRACTL_END; EnterFinalizeLock(); - // Adjust gen - unsigned int dest = 0; - if (g_fFinalizerRunOnShutDown) - { - //put it in the finalizer queue and sort out when - //dequeueing - dest = FinalizerListSeg; - } - - else - dest = gen_segment (gen); + // Adjust gen + unsigned int dest = gen_segment (gen); // Adjust boundary for segments so that GC will keep objects alive. Object*** s_i = &SegQueue (FreeList); @@ -38048,24 +38017,9 @@ CFinalize::GetNextFinalizableObject (BOOL only_non_critical) Object* obj = 0; EnterFinalizeLock(); -retry: if (!IsSegEmpty(FinalizerListSeg)) { - if (g_fFinalizerRunOnShutDown) - { - obj = *(SegQueueLimit (FinalizerListSeg)-1); - if (method_table(obj)->HasCriticalFinalizer()) - { - MoveItem ((SegQueueLimit (FinalizerListSeg)-1), - FinalizerListSeg, CriticalFinalizerListSeg); - goto retry; - } - else - --SegQueueLimit (FinalizerListSeg); - } - else - obj = *(--SegQueueLimit (FinalizerListSeg)); - + obj = *(--SegQueueLimit (FinalizerListSeg)); } else if (!only_non_critical && !IsSegEmpty(CriticalFinalizerListSeg)) { @@ -38082,52 +38036,10 @@ retry: return obj; } -void -CFinalize::SetSegForShutDown(BOOL fHasLock) -{ - int i; - - if (!fHasLock) - EnterFinalizeLock(); - for (i = 0; i <= max_generation; i++) - { - unsigned int seg = gen_segment (i); - Object** startIndex = SegQueueLimit (seg)-1; - Object** stopIndex = SegQueue (seg); - for (Object** po = startIndex; po >= stopIndex; po--) - { - Object* obj = *po; - if (method_table(obj)->HasCriticalFinalizer()) - { - MoveItem (po, seg, CriticalFinalizerListSeg); - } - else - { - MoveItem (po, seg, FinalizerListSeg); - } - } - } - if (!fHasLock) - LeaveFinalizeLock(); -} - -void -CFinalize::DiscardNonCriticalObjects() -{ - //empty the finalization queue - Object** startIndex = SegQueueLimit (FinalizerListSeg)-1; - Object** stopIndex = SegQueue (FinalizerListSeg); - for (Object** po = startIndex; po >= stopIndex; po--) - { - MoveItem (po, FinalizerListSeg, FreeList); - } -} - size_t CFinalize::GetNumberFinalizableObjects() { - return SegQueueLimit (FinalizerListSeg) - - (g_fFinalizerRunOnShutDown ? m_Array : SegQueue(FinalizerListSeg)); + return SegQueueLimit(FinalizerListSeg) - SegQueue(FinalizerListSeg); } void @@ -38841,11 +38753,6 @@ bool GCHeap::IsConcurrentGCEnabled() #endif //BACKGROUND_GC } -void GCHeap::SetFinalizeRunOnShutdown(bool value) -{ - g_fFinalizerRunOnShutDown = value; -} - void PopulateDacVars(GcDacVars *gcDacVars) { #ifndef DACCESS_COMPILE diff --git a/src/coreclr/src/gc/gcimpl.h b/src/coreclr/src/gc/gcimpl.h index 2478970..d1f062e 100644 --- a/src/coreclr/src/gc/gcimpl.h +++ b/src/coreclr/src/gc/gcimpl.h @@ -214,12 +214,8 @@ public: PER_HEAP_ISOLATED size_t GetNumberFinalizableObjects(); PER_HEAP_ISOLATED size_t GetFinalizablePromotedCount(); - void SetFinalizeQueueForShutdown(bool fHasLock); - bool ShouldRestartFinalizerWatchDog(); - void DiagWalkObject (Object* obj, walk_fn fn, void* context); void DiagWalkObject2 (Object* obj, walk_fn2 fn, void* context); - void SetFinalizeRunOnShutdown(bool value); public: // FIX diff --git a/src/coreclr/src/gc/gcinterface.h b/src/coreclr/src/gc/gcinterface.h index 3ecf024..331f8e1 100644 --- a/src/coreclr/src/gc/gcinterface.h +++ b/src/coreclr/src/gc/gcinterface.h @@ -579,23 +579,12 @@ public: =========================================================================== */ - // Finalizes all registered objects for shutdown, even if they are still reachable. - virtual void SetFinalizeQueueForShutdown(bool fHasLock) = 0; - // Gets the number of finalizable objects. virtual size_t GetNumberOfFinalizable() = 0; - // Traditionally used by the finalizer thread on shutdown to determine - // whether or not to time out. Returns true if the GC lock has not been taken. - virtual bool ShouldRestartFinalizerWatchDog() = 0; - // Gets the next finalizable object. virtual Object* GetNextFinalizable() = 0; - // Sets whether or not the GC should report all finalizable objects as - // ready to be finalized, instead of only collectable objects. - virtual void SetFinalizeRunOnShutdown(bool value) = 0; - /* =========================================================================== BCL routines. These are routines that are directly exposed by mscorlib diff --git a/src/coreclr/src/gc/gcpriv.h b/src/coreclr/src/gc/gcpriv.h index 7e2f7bb..62844b7 100644 --- a/src/coreclr/src/gc/gcpriv.h +++ b/src/coreclr/src/gc/gcpriv.h @@ -4400,9 +4400,7 @@ public: size_t GetPromotedCount(); //Methods used by the shutdown code to call every finalizer - void SetSegForShutDown(BOOL fHasLock); size_t GetNumberFinalizableObjects(); - void DiscardNonCriticalObjects(); void CheckFinalizerObjects(); }; diff --git a/src/coreclr/src/vm/finalizerthread.cpp b/src/coreclr/src/vm/finalizerthread.cpp index 5b72191..a46166b 100644 --- a/src/coreclr/src/vm/finalizerthread.cpp +++ b/src/coreclr/src/vm/finalizerthread.cpp @@ -50,21 +50,6 @@ BOOL FinalizerThread::HaveExtraWorkForFinalizer() return GetFinalizerThread()->HaveExtraWorkForFinalizer(); } -// This helper is here to avoid EH goo associated with DefineFullyQualifiedNameForStack being -// invoked when logging is off. -NOINLINE -void LogFinalization(Object* obj) -{ - STATIC_CONTRACT_NOTHROW; - STATIC_CONTRACT_GC_NOTRIGGER; - STATIC_CONTRACT_MODE_ANY; - -#ifdef FEATURE_EVENT_TRACE - ETW::GCLog::SendFinalizeObjectEvent(obj->GetMethodTable(), obj); -#endif // FEATURE_EVENT_TRACE -} - - void CallFinalizer(Object* obj) { STATIC_CONTRACT_THROWS; @@ -76,40 +61,26 @@ void CallFinalizer(Object* obj) LOG((LF_GC, LL_INFO1000, "Finalizing " LOG_OBJECT_CLASS(obj))); _ASSERTE(GetThread()->PreemptiveGCDisabled()); - // if we don't have a class, we can't call the finalizer - // if the object has been marked run as finalizer run don't call either - if (pMT) + + if (!((obj->GetHeader()->GetBits()) & BIT_SBLK_FINALIZER_RUN)) { - if (!((obj->GetHeader()->GetBits()) & BIT_SBLK_FINALIZER_RUN)) - { + _ASSERTE(pMT->HasFinalizer()); - _ASSERTE(obj->GetMethodTable() == pMT); - _ASSERTE(pMT->HasFinalizer()); +#ifdef FEATURE_EVENT_TRACE + ETW::GCLog::SendFinalizeObjectEvent(pMT, obj); +#endif // FEATURE_EVENT_TRACE - LogFinalization(obj); - MethodTable::CallFinalizer(obj); - } - else - { - //reset the bit so the object can be put on the list - //with RegisterForFinalization - obj->GetHeader()->ClrBit (BIT_SBLK_FINALIZER_RUN); - } + MethodTable::CallFinalizer(obj); + } + else + { + //reset the bit so the object can be put on the list + //with RegisterForFinalization + obj->GetHeader()->ClrBit (BIT_SBLK_FINALIZER_RUN); } } -void FinalizerThread::DoOneFinalization(Object* fobj, Thread* pThread) -{ - STATIC_CONTRACT_THROWS; - STATIC_CONTRACT_GC_TRIGGERS; - STATIC_CONTRACT_MODE_COOPERATIVE; - - CallFinalizer(fobj); - - pThread->InternalReset(); -} - -void FinalizerThread::FinalizeAllObjects(int bitToCheck) +void FinalizerThread::FinalizeAllObjects() { STATIC_CONTRACT_THROWS; STATIC_CONTRACT_GC_TRIGGERS; @@ -126,16 +97,12 @@ void FinalizerThread::FinalizeAllObjects(int bitToCheck) // Finalize everyone while (fobj && !fQuitFinalizer) { - if (fobj->GetHeader()->GetBits() & bitToCheck) - { - fobj = GCHeapUtilities::GetGCHeap()->GetNextFinalizable(); - } - else - { - fcount++; - DoOneFinalization(fobj, pThread); - fobj = GCHeapUtilities::GetGCHeap()->GetNextFinalizable(); - } + fcount++; + + CallFinalizer(fobj); + pThread->InternalReset(); + + fobj = GCHeapUtilities::GetGCHeap()->GetNextFinalizable(); } FireEtwGCFinalizersEnd_V1(fcount, GetClrInstanceId()); } @@ -345,7 +312,7 @@ VOID FinalizerThread::FinalizerThreadWorker(void *args) GetFinalizerThread()->EEResetAbort(Thread::TAR_ALL); } - FinalizeAllObjects(0); + FinalizeAllObjects(); // We may still have the finalizer thread for abort. If so the abort request is for previous finalizer method, not for next one. if (GetFinalizerThread()->IsAbortRequested()) @@ -431,14 +398,7 @@ DWORD WINAPI FinalizerThread::FinalizerThreadStart(void *args) // since doing so will cause OLE32 to CoUninitialize. while (1) { - PAL_TRY(void *, unused, NULL) - { - __SwitchToThread(INFINITE, CALLER_LIMITS_SPINNING); - } - PAL_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - } - PAL_ENDTRY + __SwitchToThread(INFINITE, CALLER_LIMITS_SPINNING); } return 0; diff --git a/src/coreclr/src/vm/finalizerthread.h b/src/coreclr/src/vm/finalizerthread.h index 824b3e9..fdfc2ab 100644 --- a/src/coreclr/src/vm/finalizerthread.h +++ b/src/coreclr/src/vm/finalizerthread.h @@ -32,9 +32,7 @@ class FinalizerThread static void WaitForFinalizerEvent (CLREvent *event); - static void DoOneFinalization(Object* fobj, Thread* pThread); - - static void FinalizeAllObjects(int bitToCheck); + static void FinalizeAllObjects(); public: static Thread* GetFinalizerThread() diff --git a/src/coreclr/src/vm/hosting.cpp b/src/coreclr/src/vm/hosting.cpp index 9489879..92ab961 100644 --- a/src/coreclr/src/vm/hosting.cpp +++ b/src/coreclr/src/vm/hosting.cpp @@ -243,7 +243,6 @@ BOOL ClrVirtualProtect(LPVOID lpAddress, SIZE_T dwSize, DWORD flNewProtect, PDWO #define VirtualProtect(lpAddress, dwSize, flNewProtect, lpflOldProtect) Dont_Use_VirtualProtect(lpAddress, dwSize, flNewProtect, lpflOldProtect) #undef SleepEx -#undef Sleep DWORD ClrSleepEx(DWORD dwMilliseconds, BOOL bAlertable) { CONTRACTL @@ -264,26 +263,11 @@ DWORD ClrSleepEx(DWORD dwMilliseconds, BOOL bAlertable) } #define SleepEx(dwMilliseconds,bAlertable) \ Dont_Use_SleepEx(dwMilliseconds,bAlertable) -#define Sleep(a) Dont_Use_Sleep(a) // non-zero return value if this function causes the OS to switch to another thread // See file:spinlock.h#SwitchToThreadSpinning for an explanation of dwSwitchCount BOOL __SwitchToThread (DWORD dwSleepMSec, DWORD dwSwitchCount) { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - } - CONTRACTL_END; - - return __DangerousSwitchToThread(dwSleepMSec, dwSwitchCount, FALSE); -} - -#undef SleepEx -BOOL __DangerousSwitchToThread (DWORD dwSleepMSec, DWORD dwSwitchCount, BOOL goThroughOS) -{ // If you sleep for a long time, the thread should be in Preemptive GC mode. CONTRACTL { @@ -296,14 +280,7 @@ BOOL __DangerousSwitchToThread (DWORD dwSleepMSec, DWORD dwSwitchCount, BOOL goT if (dwSleepMSec > 0) { - // when called with goThroughOS make sure to not call into the host. This function - // may be called from GetRuntimeFunctionCallback() which is called by the OS to determine - // the personality routine when it needs to unwind managed code off the stack. when this - // happens in the context of an SO we want to avoid calling into the host - if (goThroughOS) - ::SleepEx(dwSleepMSec, FALSE); - else - ClrSleepEx(dwSleepMSec,FALSE); + ClrSleepEx(dwSleepMSec,FALSE); return TRUE; } @@ -330,18 +307,11 @@ BOOL __DangerousSwitchToThread (DWORD dwSleepMSec, DWORD dwSwitchCount, BOOL goT _ASSERTE(CALLER_LIMITS_SPINNING < SLEEP_START_THRESHOLD); if (dwSwitchCount >= SLEEP_START_THRESHOLD) { - if (goThroughOS) - ::SleepEx(1, FALSE); - else - ClrSleepEx(1, FALSE); + ClrSleepEx(1, FALSE); } - { - return SwitchToThread(); - } + return SwitchToThread(); } -#define SleepEx(dwMilliseconds,bAlertable) \ - Dont_Use_SleepEx(dwMilliseconds,bAlertable) // Locking routines supplied by the EE to the other DLLs of the CLR. In a _DEBUG // build of the EE, we poison the Crst as a poor man's attempt to do some argument diff --git a/src/coreclr/src/vm/spinlock.h b/src/coreclr/src/vm/spinlock.h index 37aab47..adef3d4 100644 --- a/src/coreclr/src/vm/spinlock.h +++ b/src/coreclr/src/vm/spinlock.h @@ -61,7 +61,6 @@ // non-zero return value if this function causes the OS to switch to another thread BOOL __SwitchToThread (DWORD dwSleepMSec, DWORD dwSwitchCount); -BOOL __DangerousSwitchToThread (DWORD dwSleepMSec, DWORD dwSwitchCount, BOOL goThroughOS); //---------------------------------------------------------------------------- -- 2.7.4