Skip to content

Commit

Permalink
Remove need for full GC for unloadable stuff (dotnet#20384)
Browse files Browse the repository at this point in the history
This change removes enforcing of full GC after native LoaderAllocator is
destroyed. It turns out it was not needed. There is no regression in
running coreclr pri 1 tests inside an unloadable AssemblyLoadContext and
unloading the context after a test completes for each test.
  • Loading branch information
janvorli authored and A-And committed Nov 20, 2018
1 parent 32f72e4 commit a296cb7
Show file tree
Hide file tree
Showing 7 changed files with 0 additions and 48 deletions.
1 change: 0 additions & 1 deletion src/gc/env/gcenv.ee.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ class GCToEEInterface

static void HandleFatalError(unsigned int exitCode);
static bool ShouldFinalizeObjectForUnload(void* pDomain, Object* obj);
static bool ForceFullGCToBeBlocking();
static bool EagerFinalized(Object* obj);
static MethodTable* GetFreeObjectMethodTable();
static bool GetBooleanConfigValue(const char* key, bool* value);
Expand Down
16 changes: 0 additions & 16 deletions src/gc/gc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15124,22 +15124,6 @@ int gc_heap::generation_to_condemn (int n_initial,
}
}

if (n == max_generation && GCToEEInterface::ForceFullGCToBeBlocking())
{
#ifdef BACKGROUND_GC
// do not turn stress-induced collections into blocking GCs, unless there
// have already been more full BGCs than full NGCs
#if 0
// This exposes DevDiv 94129, so we'll leave this out for now
if (!settings.stress_induced ||
full_gc_counts[gc_type_blocking] <= full_gc_counts[gc_type_background])
#endif // 0
#endif // BACKGROUND_GC
{
*blocking_collection_p = TRUE;
}
}

return n;
}

Expand Down
6 changes: 0 additions & 6 deletions src/gc/gcenv.ee.standalone.inl
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,6 @@ inline bool GCToEEInterface::ShouldFinalizeObjectForUnload(void* pDomain, Object
return g_theGCToCLR->ShouldFinalizeObjectForUnload(pDomain, obj);
}

inline bool GCToEEInterface::ForceFullGCToBeBlocking()
{
assert(g_theGCToCLR != nullptr);
return g_theGCToCLR->ForceFullGCToBeBlocking();
}

inline bool GCToEEInterface::EagerFinalized(Object* obj)
{
assert(g_theGCToCLR != nullptr);
Expand Down
6 changes: 0 additions & 6 deletions src/gc/gcinterface.ee.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,6 @@ class IGCToCLR {
virtual
bool EagerFinalized(Object* obj) = 0;

// Asks the EE if it wishes for the current GC to be a blocking GC. The GC will
// only invoke this callback when it intends to do a full GC, so at this point
// the EE can opt to elevate that collection to be a blocking GC and not a background one.
virtual
bool ForceFullGCToBeBlocking() = 0;

// Retrieves the method table for the free object, a special kind of object used by the GC
// to keep the heap traversable. Conceptually, the free object is similar to a managed array
// of bytes: it consists of an object header (like all objects) and a "numComponents" field,
Expand Down
5 changes: 0 additions & 5 deletions src/gc/sample/gcenv.ee.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,6 @@ bool GCToEEInterface::ShouldFinalizeObjectForUnload(void* pDomain, Object* obj)
return true;
}

bool GCToEEInterface::ForceFullGCToBeBlocking()
{
return false;
}

bool GCToEEInterface::EagerFinalized(Object* obj)
{
// The sample does not finalize anything eagerly.
Expand Down
13 changes: 0 additions & 13 deletions src/vm/gcenv.ee.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1003,19 +1003,6 @@ bool GCToEEInterface::ShouldFinalizeObjectForUnload(void* pDomain, Object* obj)
return true;
}

bool GCToEEInterface::ForceFullGCToBeBlocking()
{
// In theory, there is nothing fundamental that requires an AppDomain unload to induce
// a blocking GC. In the past, this workaround was done to fix an Stress AV, but the root
// cause of the AV was never discovered and this workaround remains in place.
//
// It would be nice if this were not necessary. However, it's not clear if the aforementioned
// stress bug is still lurking and will return if this workaround is removed. We should
// do some experiments: remove this workaround and see if the stress bug still repros.
// If so, we should find the root cause instead of relying on this.
return !!SystemDomain::System()->RequireAppDomainCleanup();
}

bool GCToEEInterface::EagerFinalized(Object* obj)
{
MethodTable* pMT = obj->GetGCSafeMethodTable();
Expand Down
1 change: 0 additions & 1 deletion src/vm/gcenv.ee.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ class GCToEEInterface : public IGCToCLR {
void EnableFinalization(bool foundFinalizers);
void HandleFatalError(unsigned int exitCode);
bool ShouldFinalizeObjectForUnload(void* pDomain, Object* obj);
bool ForceFullGCToBeBlocking();
bool EagerFinalized(Object* obj);
MethodTable* GetFreeObjectMethodTable();
bool GetBooleanConfigValue(const char* key, bool* value);
Expand Down

0 comments on commit a296cb7

Please sign in to comment.