From 272c1ba11fac7a9ceede2f4f737bb27b4bbcad71 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Thu, 19 Mar 2015 20:50:57 -0700 Subject: [PATCH] Bug 1120655 - Suppress zone/compartment collection while iterating. r=terrence, a=bkerensa --- js/src/gc/Zone.h | 9 ++++---- js/src/jsgc.cpp | 57 +++++++++++++++++++++++++++++++++++---------------- js/src/jsgc.h | 11 +++++++++- js/src/vm/Runtime.cpp | 1 + js/src/vm/Runtime.h | 3 +++ 5 files changed, 58 insertions(+), 23 deletions(-) diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h index e7f687a..dd058f0 100644 --- a/js/src/gc/Zone.h +++ b/js/src/gc/Zone.h @@ -353,10 +353,11 @@ enum ZoneSelector { class ZonesIter { private: + gc::AutoEnterIteration iterMarker; JS::Zone **it, **end; public: - ZonesIter(JSRuntime *rt, ZoneSelector selector) { + ZonesIter(JSRuntime *rt, ZoneSelector selector) : iterMarker(rt) { it = rt->zones.begin(); end = rt->zones.end(); @@ -427,13 +428,13 @@ struct CompartmentsInZoneIter template class CompartmentsIterT { - private: + gc::AutoEnterIteration iterMarker; ZonesIterT zone; mozilla::Maybe comp; public: explicit CompartmentsIterT(JSRuntime *rt) - : zone(rt) + : iterMarker(rt), zone(rt) { if (zone.done()) comp.construct(); @@ -442,7 +443,7 @@ class CompartmentsIterT } CompartmentsIterT(JSRuntime *rt, ZoneSelector selector) - : zone(rt, selector) + : iterMarker(rt), zone(rt, selector) { if (zone.done()) comp.construct(); diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 15c86c8..1dfe0ab 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -2525,7 +2525,7 @@ ReleaseObservedTypes(JSRuntime* rt) * arbitrary compartment in the zone. */ static void -SweepCompartments(FreeOp *fop, Zone *zone, bool keepAtleastOne, bool lastGC) +SweepCompartments(FreeOp *fop, Zone *zone, bool keepAtleastOne, bool destroyingRuntime) { JSRuntime *rt = zone->runtimeFromMainThread(); JSDestroyCompartmentCallback callback = rt->destroyCompartmentCallback; @@ -2543,7 +2543,7 @@ SweepCompartments(FreeOp *fop, Zone *zone, bool keepAtleastOne, bool lastGC) * deleted and keepAtleastOne is true. */ bool dontDelete = read == end && !foundOne && keepAtleastOne; - if ((!comp->marked && !dontDelete) || lastGC) { + if ((!comp->marked && !dontDelete) || destroyingRuntime) { if (callback) callback(fop, comp); if (comp->principals) @@ -2559,9 +2559,13 @@ SweepCompartments(FreeOp *fop, Zone *zone, bool keepAtleastOne, bool lastGC) } static void -SweepZones(FreeOp *fop, bool lastGC) +SweepZones(FreeOp *fop, bool destroyingRuntime) { JSRuntime *rt = fop->runtime(); + MOZ_ASSERT_IF(destroyingRuntime, rt->numActiveZoneIters == 0); + if (rt->numActiveZoneIters) + return; + JSZoneCallback callback = rt->destroyZoneCallback; /* Skip the atomsCompartment zone. */ @@ -2576,17 +2580,17 @@ SweepZones(FreeOp* fop, bool lastGC) if (zone->wasGCStarted()) { if ((zone->allocator.arenas.arenaListsAreEmpty() && !zone->hasMarkedCompartments()) || - lastGC) + destroyingRuntime) { zone->allocator.arenas.checkEmptyFreeLists(); if (callback) callback(zone); - SweepCompartments(fop, zone, false, lastGC); + SweepCompartments(fop, zone, false, destroyingRuntime); JS_ASSERT(zone->compartments.empty()); fop->delete_(zone); continue; } - SweepCompartments(fop, zone, true, lastGC); + SweepCompartments(fop, zone, true, destroyingRuntime); } *write++ = zone; } @@ -3787,7 +3791,7 @@ EndSweepingZoneGroup(JSRuntime *rt) } static void -BeginSweepPhase(JSRuntime *rt, bool lastGC) +BeginSweepPhase(JSRuntime *rt, bool destroyingRuntime) { /* * Sweep phase. @@ -3804,7 +3808,7 @@ BeginSweepPhase(JSRuntime *rt, bool lastGC) gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP); #ifdef JS_THREADSAFE - rt->gcSweepOnBackgroundThread = !lastGC && rt->useHelperThreads(); + rt->gcSweepOnBackgroundThread = !destroyingRuntime && rt->useHelperThreads(); #endif #ifdef DEBUG @@ -3903,12 +3907,12 @@ SweepPhase(JSRuntime *rt, SliceBudget &sliceBudget) } static void -EndSweepPhase(JSRuntime *rt, JSGCInvocationKind gckind, bool lastGC) +EndSweepPhase(JSRuntime *rt, JSGCInvocationKind gckind, bool destroyingRuntime) { gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP); FreeOp fop(rt, rt->gcSweepOnBackgroundThread); - JS_ASSERT_IF(lastGC, !rt->gcSweepOnBackgroundThread); + JS_ASSERT_IF(destroyingRuntime, !rt->gcSweepOnBackgroundThread); JS_ASSERT(rt->gcMarker.isDrained()); rt->gcMarker.stop(); @@ -3959,8 +3963,8 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocationKind gckind, bool lastGC) * This removes compartments from rt->compartment, so we do it last to make * sure we don't miss sweeping any compartments. */ - if (!lastGC) - SweepZones(&fop, lastGC); + if (!destroyingRuntime) + SweepZones(&fop, destroyingRuntime); if (!rt->gcSweepOnBackgroundThread) { /* @@ -4001,8 +4005,8 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocationKind gckind, bool lastGC) rt->freeLifoAlloc.freeAll(); /* Ensure the compartments get swept if it's the last GC. */ - if (lastGC) - SweepZones(&fop, lastGC); + if (destroyingRuntime) + SweepZones(&fop, destroyingRuntime); } for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) { @@ -4339,7 +4343,7 @@ IncrementalCollectSlice(JSRuntime *rt, AutoCopyFreeListToArenasForGC copy(rt); AutoGCSlice slice(rt); - bool lastGC = (reason == JS::gcreason::DESTROY_RUNTIME); + bool destroyingRuntime = (reason == JS::gcreason::DESTROY_RUNTIME); gc::State initialState = rt->gcIncrementalState; @@ -4384,7 +4388,7 @@ IncrementalCollectSlice(JSRuntime *rt, return; } - if (!lastGC) + if (!destroyingRuntime) PushZealSelectedObjects(rt); rt->gcIncrementalState = MARK; @@ -4426,7 +4430,7 @@ IncrementalCollectSlice(JSRuntime *rt, * This runs to completion, but we don't continue if the budget is * now exhasted. */ - BeginSweepPhase(rt, lastGC); + BeginSweepPhase(rt, destroyingRuntime); if (sliceBudget.isOverBudget()) break; @@ -4445,7 +4449,7 @@ IncrementalCollectSlice(JSRuntime *rt, if (!finished) break; - EndSweepPhase(rt, gckind, lastGC); + EndSweepPhase(rt, gckind, destroyingRuntime); if (rt->gcSweepOnBackgroundThread) rt->gcHelperThread.startBackgroundSweep(gckind == GC_SHRINK); @@ -5386,3 +5390,20 @@ JS::AutoAssertNoGC::~AutoAssertNoGC() MOZ_ASSERT(gcNumber == runtime->gcNumber, "GC ran inside an AutoAssertNoGC scope."); } #endif + +namespace js { +namespace gc { + +AutoEnterIteration::AutoEnterIteration(JSRuntime *rt_) : rt(rt_) +{ + ++rt->numActiveZoneIters; +} + +AutoEnterIteration::~AutoEnterIteration() +{ + MOZ_ASSERT(rt->numActiveZoneIters); + --rt->numActiveZoneIters; +} + +} /* namespace gc */ +} /* namespace js */ diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 825cff5..ca331c0 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -1077,7 +1077,7 @@ MaybeVerifyBarriers(JSContext* cx, bool always = false) /* * Instances of this class set the |JSRuntime::suppressGC| flag for the duration * that they are live. Use of this class is highly discouraged. Please carefully - * read the comment in jscntxt.h above |suppressGC| and take all appropriate + * read the comment in vm/Runtime.h above |suppressGC| and take all appropriate * precautions before instantiating this class. */ class AutoSuppressGC @@ -1113,6 +1113,15 @@ class AutoEnterOOMUnsafeRegion class AutoEnterOOMUnsafeRegion {}; #endif /* DEBUG */ +/* Prevent compartments and zones from being collected during iteration. */ +class AutoEnterIteration { + JSRuntime *rt; + + public: + AutoEnterIteration(JSRuntime *rt_); + ~AutoEnterIteration(); +}; + } /* namespace gc */ #ifdef DEBUG diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp index bb5c8680..0d8c6cd 100644 --- a/js/src/vm/Runtime.cpp +++ b/js/src/vm/Runtime.cpp @@ -195,6 +195,7 @@ JSRuntime::JSRuntime(JSRuntime *parentRuntime, JSUseHelperThreads useHelperThrea gcShouldCleanUpEverything(false), gcGrayBitsValid(false), gcIsNeeded(0), + numActiveZoneIters(0), gcStats(thisFromCtor()), gcNumber(0), gcStartNumber(0), diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index 5aeb924..ba4180e 100644 --- a/js/src/vm/Runtime.h +++ b/js/src/vm/Runtime.h @@ -1061,6 +1061,9 @@ struct JSRuntime : public JS::shadow::Runtime, */ volatile uintptr_t gcIsNeeded; + mozilla::Atomic numActiveZoneIters; + friend class js::gc::AutoEnterIteration; + js::gcstats::Statistics gcStats; /* Incremented on every GC slice. */ -- 2.2.1