Bug 1120655 - Suppress zone/compartment collection while iterating. r=terrence, a=1.4+
authorSteve Fink <sfink@mozilla.com>
Thu, 19 Mar 2015 20:50:57 -0700
changeset 188817 0673402a88ac74c7b57a853b82a2355b367adef0
parent 188816 3b992f3167108ed02dcdb3a27a829dcf74ecc8d8
child 188818 80579b9023ea902780120e544f3c3b391115bc61
push id857
push userryanvm@gmail.com
push dateFri, 10 Apr 2015 15:03:45 +0000
treeherdermozilla-b2g30_v1_4@0673402a88ac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, 1.4
bugs1120655
milestone30.0
Bug 1120655 - Suppress zone/compartment collection while iterating. r=terrence, a=1.4+
js/src/gc/Zone.h
js/src/jsgc.cpp
js/src/jsgc.h
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -349,20 +349,21 @@ namespace js {
  */
 enum ZoneSelector {
     WithAtoms,
     SkipAtoms
 };
 
 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();
 
         if (selector == SkipAtoms) {
             JS_ASSERT(rt->isAtomsZone(*it));
             it++;
         }
     }
@@ -423,32 +424,32 @@ struct CompartmentsInZoneIter
 
 /*
  * This iterator iterates over all the compartments in a given set of zones. The
  * set of zones is determined by iterating ZoneIterT.
  */
 template<class ZonesIterT>
 class CompartmentsIterT
 {
-  private:
+    gc::AutoEnterIteration iterMarker;
     ZonesIterT zone;
     mozilla::Maybe<CompartmentsInZoneIter> comp;
 
   public:
     explicit CompartmentsIterT(JSRuntime* rt)
-      : zone(rt)
+      : iterMarker(rt), zone(rt)
     {
         if (zone.done())
             comp.construct();
         else
             comp.construct(zone);
     }
 
     CompartmentsIterT(JSRuntime* rt, ZoneSelector selector)
-      : zone(rt, selector)
+      : iterMarker(rt), zone(rt, selector)
     {
         if (zone.done())
             comp.construct();
         else
             comp.construct(zone);
     }
 
     bool done() const { return zone.done(); }
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2762,17 +2762,17 @@ ReleaseObservedTypes(JSRuntime* rt)
  * compartment. If we know we're deleting the entire zone, then
  * SweepCompartments is allowed to delete all compartments. In this case,
  * |keepAtleastOne| is false. If some objects remain in the zone so that it
  * cannot be deleted, then we set |keepAtleastOne| to true, which prohibits
  * SweepCompartments from deleting every compartment. Instead, it preserves an
  * 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;
 
     JSCompartment** read = zone->compartments.begin();
     JSCompartment** end = zone->compartments.end();
     JSCompartment** write = read;
     bool foundOne = false;
@@ -2780,60 +2780,64 @@ SweepCompartments(FreeOp* fop, Zone* zon
         JSCompartment* comp = *read++;
         JS_ASSERT(!rt->isAtomsCompartment(comp));
 
         /*
          * Don't delete the last compartment if all the ones before it were
          * 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)
                 JS_DropPrincipals(rt, comp->principals);
             js_delete(comp);
         } else {
             *write++ = comp;
             foundOne = true;
         }
     }
     zone->compartments.resize(write - zone->compartments.begin());
     JS_ASSERT_IF(keepAtleastOne, !zone->compartments.empty());
 }
 
 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. */
     Zone** read = rt->zones.begin() + 1;
     Zone** end = rt->zones.end();
     Zone** write = read;
     JS_ASSERT(rt->zones.length() >= 1);
     JS_ASSERT(rt->isAtomsZone(rt->zones[0]));
 
     while (read < end) {
         Zone* zone = *read++;
 
         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;
     }
     rt->zones.resize(write - rt->zones.begin());
 }
 
 static void
 PurgeRuntime(JSRuntime* rt)
@@ -4016,34 +4020,34 @@ EndSweepingZoneGroup(JSRuntime* rt)
     /* Reset the list of arenas marked as being allocated during sweep phase. */
     while (ArenaHeader* arena = rt->gcArenasAllocatedDuringSweep) {
         rt->gcArenasAllocatedDuringSweep = arena->getNextAllocDuringSweep();
         arena->unsetAllocDuringSweep();
     }
 }
 
 static void
-BeginSweepPhase(JSRuntime* rt, bool lastGC)
+BeginSweepPhase(JSRuntime* rt, bool destroyingRuntime)
 {
     /*
      * Sweep phase.
      *
      * Finalize as we sweep, outside of rt->gcLock but with rt->isHeapBusy()
      * true so that any attempt to allocate a GC-thing from a finalizer will
      * fail, rather than nest badly and leave the unmarked newborn to be swept.
      */
 
     JS_ASSERT(!rt->gcAbortSweepAfterCurrentGroup);
 
     ComputeNonIncrementalMarkingForValidation(rt);
 
     gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP);
 
 #ifdef JS_THREADSAFE
-    rt->gcSweepOnBackgroundThread = !lastGC && rt->useHelperThreads();
+    rt->gcSweepOnBackgroundThread = !destroyingRuntime && rt->useHelperThreads();
 #endif
 
 #ifdef DEBUG
     for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) {
         JS_ASSERT(!c->gcIncomingGrayPointers);
         for (JSCompartment::WrapperEnum e(c); !e.empty(); e.popFront()) {
             if (e.front().key().kind != CrossCompartmentKey::StringWrapper)
                 AssertNotOnGrayList(&e.front().value().get().toObject());
@@ -4132,22 +4136,22 @@ SweepPhase(JSRuntime* rt, SliceBudget& s
         if (!rt->gcCurrentZoneGroup)
             return true;  /* We're finished. */
         EndMarkingZoneGroup(rt);
         BeginSweepingZoneGroup(rt);
     }
 }
 
 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();
 
     /*
      * Recalculate whether GC was full or not as this may have changed due to
      * newly created zones.  Can only change from full to not full.
      */
@@ -4188,18 +4192,18 @@ EndSweepPhase(JSRuntime* rt, JSGCInvocat
         /* Clear out any small pools that we're hanging on to. */
         if (JSC::ExecutableAllocator* execAlloc = rt->maybeExecAlloc())
             execAlloc->purge();
 
         /*
          * 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) {
             /*
              * Destroy arenas after we finished the sweeping so finalizers can
              * safely use IsAboutToBeFinalized(). This is done on the
              * GCHelperThread if possible. We acquire the lock only because
              * Expire needs to unlock it for other callers.
              */
@@ -4230,18 +4234,18 @@ EndSweepPhase(JSRuntime* rt, JSGCInvocat
     if (!rt->gcSweepOnBackgroundThread) {
         gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_DESTROY);
 
         SweepBackgroundThings(rt, false);
 
         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()) {
         zone->setGCLastBytes(zone->gcBytes, gckind);
         if (zone->isCollecting()) {
             JS_ASSERT(zone->isGCFinished());
             zone->setGCState(Zone::NoGC);
         }
@@ -4565,17 +4569,17 @@ IncrementalCollectSlice(JSRuntime* rt,
                         JS::gcreason::Reason reason,
                         JSGCInvocationKind gckind)
 {
     JS_ASSERT(rt->currentThreadHasExclusiveAccess());
 
     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;
 
     int zeal = 0;
 #ifdef JS_GC_ZEAL
     if (reason == JS::gcreason::DEBUG_GC && budget != SliceBudget::Unlimited) {
         /*
          * Do the incremental collection type specified by zeal mode if the
@@ -4610,17 +4614,17 @@ IncrementalCollectSlice(JSRuntime* rt,
     switch (rt->gcIncrementalState) {
 
       case MARK_ROOTS:
         if (!BeginMarkPhase(rt)) {
             rt->gcIncrementalState = NO_INCREMENTAL;
             return;
         }
 
-        if (!lastGC)
+        if (!destroyingRuntime)
             PushZealSelectedObjects(rt);
 
         rt->gcIncrementalState = MARK;
 
         if (rt->gcIsIncremental && zeal == ZealIncrementalRootsThenFinish)
             break;
 
         /* fall through */
@@ -4652,17 +4656,17 @@ IncrementalCollectSlice(JSRuntime* rt,
         }
 
         rt->gcIncrementalState = SWEEP;
 
         /*
          * 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;
 
         /*
          * Always yield here when running in incremental multi-slice zeal
          * mode, so RunDebugGC can reset the slice buget.
          */
         if (rt->gcIsIncremental && zeal == ZealIncrementalMultipleSlices)
@@ -4671,17 +4675,17 @@ IncrementalCollectSlice(JSRuntime* rt,
         /* fall through */
       }
 
       case SWEEP: {
         bool finished = SweepPhase(rt, sliceBudget);
         if (!finished)
             break;
 
-        EndSweepPhase(rt, gckind, lastGC);
+        EndSweepPhase(rt, gckind, destroyingRuntime);
 
         if (rt->gcSweepOnBackgroundThread)
             rt->gcHelperThread.startBackgroundSweep(gckind == GC_SHRINK);
 
         rt->gcIncrementalState = NO_INCREMENTAL;
         break;
       }
 
@@ -5616,8 +5620,25 @@ JS::AutoAssertNoGC::AutoAssertNoGC(JSRun
 }
 
 JS::AutoAssertNoGC::~AutoAssertNoGC()
 {
     if (runtime)
         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 */
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1401,17 +1401,17 @@ MaybeVerifyBarriers(JSContext* cx, bool 
 {
 }
 
 #endif
 
 /*
  * 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
 {
     int32_t& suppressGC_;
 
   public:
     AutoSuppressGC(ExclusiveContext* cx);
@@ -1437,16 +1437,25 @@ class AutoEnterOOMUnsafeRegion
     ~AutoEnterOOMUnsafeRegion() {
         OOM_maxAllocations = saved_;
     }
 };
 #else
 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
 /* Use this to avoid assertions when manipulating the wrapper map. */
 class AutoDisableProxyCheck
 {
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER;
     uintptr_t& count;
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -188,16 +188,17 @@ JSRuntime::JSRuntime(JSRuntime* parentRu
     gcHighFrequencyHeapGrowthMin(1.5),
     gcLowFrequencyHeapGrowth(1.5),
     gcDynamicHeapGrowth(false),
     gcDynamicMarkSlice(false),
     gcDecommitThreshold(32 * 1024 * 1024),
     gcShouldCleanUpEverything(false),
     gcGrayBitsValid(false),
     gcIsNeeded(0),
+    numActiveZoneIters(0),
     gcStats(thisFromCtor()),
     gcNumber(0),
     gcStartNumber(0),
     gcIsFull(false),
     gcTriggerReason(JS::gcreason::NO_REASON),
     gcStrictCompartmentChecking(false),
 #ifdef DEBUG
     gcDisableStrictProxyCheckingCount(0),
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -1057,16 +1057,19 @@ struct JSRuntime : public JS::shadow::Ru
 
     /*
      * These flags must be kept separate so that a thread requesting a
      * compartment GC doesn't cancel another thread's concurrent request for a
      * full GC.
      */
     volatile uintptr_t  gcIsNeeded;
 
+    mozilla::Atomic<size_t, mozilla::ReleaseAcquire> numActiveZoneIters;
+    friend class js::gc::AutoEnterIteration;
+
     js::gcstats::Statistics gcStats;
 
     /* Incremented on every GC slice. */
     uint64_t            gcNumber;
 
     /* The gcNumber at the time of the most recent GC's first slice. */
     uint64_t            gcStartNumber;