From 9a9d119bb3c82b16f7575ac9f6e503b6e5bce155 Mon Sep 17 00:00:00 2001 From: Sean Gillespie Date: Wed, 5 Apr 2017 09:57:50 -0700 Subject: [PATCH] [Local GC] Move Weak Reference finalization out of the GC (dotnet/coreclr#10676) * [Local GC] Move Weak Reference finalization out of the GC * Address two issues: 1) Use GetGCSafeMethodTable instead of GetMethodTable, so that the mark bit is correctly masked off the object's method table pointer, 2) Address code review feedback by re-inserting a missed call to GetCanonicalMethodTable and rename the new API to EagerFinalized to better illustrate its broader purpose. * Repair the GC sample Commit migrated from https://github.com/dotnet/coreclr/commit/be8504bd8a63962c84567990f0b84019f299166c --- src/coreclr/src/gc/env/gcenv.ee.h | 1 + src/coreclr/src/gc/gc.cpp | 9 ++------- src/coreclr/src/gc/gc.h | 6 ------ src/coreclr/src/gc/gcenv.ee.standalone.inl | 6 ++++++ src/coreclr/src/gc/gcinterface.ee.h | 9 +++++++++ src/coreclr/src/gc/sample/gcenv.ee.cpp | 6 ++++++ src/coreclr/src/vm/gcenv.ee.cpp | 22 ++++++++++++++++++++++ src/coreclr/src/vm/gcenv.ee.h | 1 + 8 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/coreclr/src/gc/env/gcenv.ee.h b/src/coreclr/src/gc/env/gcenv.ee.h index 8c4dfcc..e7102b0 100644 --- a/src/coreclr/src/gc/env/gcenv.ee.h +++ b/src/coreclr/src/gc/env/gcenv.ee.h @@ -71,6 +71,7 @@ public: static void HandleFatalError(unsigned int exitCode); static bool ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj); + static bool EagerFinalized(Object* obj); }; #endif // __GCENV_EE_H__ diff --git a/src/coreclr/src/gc/gc.cpp b/src/coreclr/src/gc/gc.cpp index ab8e56b..aad6ec8 100644 --- a/src/coreclr/src/gc/gc.cpp +++ b/src/coreclr/src/gc/gc.cpp @@ -36269,16 +36269,11 @@ CFinalize::ScanForFinalization (promote_func* pfn, int gen, BOOL mark_only_p, assert (method_table(obj)->HasFinalizer()); -#ifndef FEATURE_REDHAWK - if (method_table(obj) == pWeakReferenceMT || method_table(obj)->GetCanonicalMethodTable() == pWeakReferenceOfTCanonMT) + if (GCToEEInterface::EagerFinalized(obj)) { - //destruct the handle right there. - FinalizeWeakReference (obj); MoveItem (i, Seg, FreeList); } - else -#endif //!FEATURE_REDHAWK - if ((obj->GetHeader()->GetBits()) & BIT_SBLK_FINALIZER_RUN) + else if ((obj->GetHeader()->GetBits()) & BIT_SBLK_FINALIZER_RUN) { //remove the object because we don't want to //run the finalizer diff --git a/src/coreclr/src/gc/gc.h b/src/coreclr/src/gc/gc.h index 821b21d..8cd92fd 100644 --- a/src/coreclr/src/gc/gc.h +++ b/src/coreclr/src/gc/gc.h @@ -256,12 +256,6 @@ void TouchPages(void * pStart, size_t cb); void updateGCShadow(Object** ptr, Object* val); #endif -// the method table for the WeakReference class -extern MethodTable *pWeakReferenceMT; -// The canonical method table for WeakReference -extern MethodTable *pWeakReferenceOfTCanonMT; -extern void FinalizeWeakReference(Object * obj); - // The single GC heap instance, shared with the VM. extern IGCHeapInternal* g_theGCHeap; diff --git a/src/coreclr/src/gc/gcenv.ee.standalone.inl b/src/coreclr/src/gc/gcenv.ee.standalone.inl index 328acef..13febb5 100644 --- a/src/coreclr/src/gc/gcenv.ee.standalone.inl +++ b/src/coreclr/src/gc/gcenv.ee.standalone.inl @@ -219,6 +219,12 @@ ALWAYS_INLINE bool GCToEEInterface::ShouldFinalizeObjectForUnload(AppDomain* pDo return g_theGCToCLR->ShouldFinalizeObjectForUnload(pDomain, obj); } +ALWAYS_INLINE bool GCToEEInterface::EagerFinalized(Object* obj) +{ + assert(g_theGCToCLR != nullptr); + return g_theGCToCLR->EagerFinalized(obj); +} + #undef ALWAYS_INLINE #endif // __GCTOENV_EE_STANDALONE_INL__ diff --git a/src/coreclr/src/gc/gcinterface.ee.h b/src/coreclr/src/gc/gcinterface.ee.h index 5c595b4..ee1b8ec 100644 --- a/src/coreclr/src/gc/gcinterface.ee.h +++ b/src/coreclr/src/gc/gcinterface.ee.h @@ -142,6 +142,15 @@ public: // an app domain. virtual bool ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj) = 0; + + // Offers the EE the option to finalize the given object eagerly, i.e. + // not on the finalizer thread but on the current thread. The + // EE returns true if it finalized the object eagerly and the GC does not + // need to do so, and false if it chose not to eagerly finalize the object + // and it's up to the GC to finalize it later. + virtual + bool EagerFinalized(Object* obj) = 0; + }; #endif // _GCINTERFACE_EE_H_ diff --git a/src/coreclr/src/gc/sample/gcenv.ee.cpp b/src/coreclr/src/gc/sample/gcenv.ee.cpp index aaca51e..392bfa1 100644 --- a/src/coreclr/src/gc/sample/gcenv.ee.cpp +++ b/src/coreclr/src/gc/sample/gcenv.ee.cpp @@ -275,6 +275,12 @@ bool GCToEEInterface::ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* return true; } +bool GCToEEInterface::EagerFinalized(Object* obj) +{ + // The sample does not finalize anything eagerly. + return false; +} + bool IsGCSpecialThread() { // TODO: Implement for background GC diff --git a/src/coreclr/src/vm/gcenv.ee.cpp b/src/coreclr/src/vm/gcenv.ee.cpp index f3c139e..a652359 100644 --- a/src/coreclr/src/vm/gcenv.ee.cpp +++ b/src/coreclr/src/vm/gcenv.ee.cpp @@ -29,6 +29,15 @@ #include "comcallablewrapper.h" #endif // FEATURE_COMINTEROP +// the method table for the WeakReference class +extern MethodTable* pWeakReferenceMT; + +// The canonical method table for WeakReference +extern MethodTable* pWeakReferenceOfTCanonMT; + +// Finalizes a weak reference directly. +extern void FinalizeWeakReference(Object* obj); + void GCToEEInterface::SuspendEE(SUSPEND_REASON reason) { WRAPPER_NO_CONTRACT; @@ -1359,3 +1368,16 @@ bool GCToEEInterface::ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* // to move them to a new app domain instead of finalizing them here. return true; } + +bool GCToEEInterface::EagerFinalized(Object* obj) +{ + MethodTable* pMT = obj->GetGCSafeMethodTable(); + if (pMT == pWeakReferenceMT || + pMT->GetCanonicalMethodTable() == pWeakReferenceOfTCanonMT) + { + FinalizeWeakReference(obj); + return true; + } + + return false; +} diff --git a/src/coreclr/src/vm/gcenv.ee.h b/src/coreclr/src/vm/gcenv.ee.h index 030eb71..5eb3e3d 100644 --- a/src/coreclr/src/vm/gcenv.ee.h +++ b/src/coreclr/src/vm/gcenv.ee.h @@ -46,6 +46,7 @@ public: void EnableFinalization(bool foundFinalizers); void HandleFatalError(unsigned int exitCode); bool ShouldFinalizeObjectForUnload(AppDomain* pDomain, Object* obj); + bool EagerFinalized(Object* obj); }; #endif // FEATURE_STANDALONE_GC -- 2.7.4