Bug 826673 - GC: Only finish sweeping the current compartment group o…

…n reset r=billm
iaxat · Dec 13, 2012 · 9b12cf2 · 9b12cf2
1 parent 4cbaec2
commit 9b12cf2
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 34 deletions.
diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp
@@ -731,13 +731,9 @@ js::gc::MarkRuntime(JSTracer *trc, bool useSavedRoots)
         if (IS_GC_MARKING_TRACER(trc) && !c->isCollecting())
             continue;
 
-        if (IS_GC_MARKING_TRACER(trc)) {
-            if ((c->activeAnalysis || c->isPreservingCode())) {
-                gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_MARK_TYPES);
-                c->markTypes(trc);
-            } else {
-                c->gcTypesMarked = false;
-            }
+        if (IS_GC_MARKING_TRACER(trc) && (c->activeAnalysis || c->isPreservingCode())) {
+            gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_MARK_TYPES);
+            c->markTypes(trc);
         }
 
         /* During a GC, these are treated as weak pointers. */

diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
@@ -808,6 +808,7 @@ JSRuntime::JSRuntime(JSUseHelperThreads useHelperThreads)
     gcSweepPhase(0),
     gcSweepCompartment(NULL),
     gcSweepKindIndex(0),
+    gcAbortSweepAfterCurrentGroup(false),
     gcArenasAllocatedDuringSweep(NULL),
 #ifdef DEBUG
     gcMarkingValidator(NULL),

diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h
@@ -761,6 +761,7 @@ struct JSRuntime : js::RuntimeFriendFields
     int                 gcSweepPhase;
     JSCompartment       *gcSweepCompartment;
     int                 gcSweepKindIndex;
+    bool                gcAbortSweepAfterCurrentGroup;
 
     /*
      * List head of arenas allocated during the sweep phase.

diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp
@@ -578,8 +578,6 @@ JSCompartment::markTypes(JSTracer *trc)
         MarkTypeObjectRoot(trc, &type, "mark_types_scan");
         JS_ASSERT(type == i.get<types::TypeObject>());
     }
-
-    gcTypesMarked = true;
 }
 
 void

diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h
@@ -387,12 +387,6 @@ struct JSCompartment : private JS::shadow::Compartment, public js::gc::GraphNode
     /* This compartment's gray roots. */
     js::Vector<js::GrayRoot, 0, js::SystemAllocPolicy> gcGrayRoots;
 
-    /*
-     * Whether type objects have been marked by markTypes().  This is used to
-     * determine whether they need to be swept.
-     */
-    bool                         gcTypesMarked;
-
   private:
     /*
      * Malloc counter to measure memory pressure for GC scheduling. It runs from

diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp
@@ -3098,6 +3098,8 @@ FindCompartmentGroups(JSRuntime *rt)
     rt->gcCompartmentGroupIndex = 0;
 }
 
+static void ResetGrayList(JSCompartment* comp);
+
 static void
 GetNextCompartmentGroup(JSRuntime *rt)
 {
@@ -3106,6 +3108,22 @@ GetNextCompartmentGroup(JSRuntime *rt)
 
     if (!rt->gcIsIncremental)
         ComponentFinder<JSCompartment>::mergeCompartmentGroups(rt->gcCurrentCompartmentGroup);
+
+    if (rt->gcAbortSweepAfterCurrentGroup) {
+        JS_ASSERT(!rt->gcIsIncremental);
+        for (GCCompartmentGroupIter c(rt); !c.done(); c.next()) {
+            JS_ASSERT(!c->gcNextGraphComponent);
+            JS_ASSERT(c->isGCMarking());
+            c->setNeedsBarrier(false, JSCompartment::UpdateIon);
+            c->setGCState(JSCompartment::NoGC);
+            ArrayBufferObject::resetArrayBufferList(c);
+            ResetGrayList(c);
+            c->gcGrayRoots.clearAndFree();
+        }
+
+        rt->gcAbortSweepAfterCurrentGroup = false;
+        rt->gcCurrentCompartmentGroup = NULL;
+    }
 }
 
 /*
@@ -3296,6 +3314,15 @@ RemoveFromGrayList(RawObject wrapper)
     return false;
 }
 
+static void
+ResetGrayList(JSCompartment* comp)
+{
+    RawObject src = comp->gcIncomingGrayPointers;
+    while (src)
+        src = NextIncomingCrossCompartmentPointer(src, true);
+    comp->gcIncomingGrayPointers = NULL;
+}
+
 void
 js::NotifyGCNukeWrapper(RawObject o)
 {
@@ -3501,6 +3528,8 @@ BeginSweepPhase(JSRuntime *rt)
      * 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);
@@ -3610,7 +3639,8 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocationKind gckind, bool lastGC)
 
     /*
      * If we found any black->gray edges during marking, we completely clear the
-     * mark bits of all uncollected compartments. This is safe, although it may
+     * mark bits of all uncollected compartments, or if a reset has occured, compartments that
+     * will no longer be collected. This is safe, although it may
      * prevent the cycle collector from collecting some dead objects.
      */
     if (rt->gcFoundBlackGrayEdges) {
@@ -3811,11 +3841,11 @@ ResetIncrementalGC(JSRuntime *rt, const char *reason)
         rt->gcMarker.stop();
 
         for (GCCompartmentsIter c(rt); !c.done(); c.next()) {
-            if (c->isGCMarking()) {
-                c->setNeedsBarrier(false, JSCompartment::UpdateIon);
-                c->setGCState(JSCompartment::NoGC);
-                ArrayBufferObject::resetArrayBufferList(c);
-            }
+            JS_ASSERT(c->isGCMarking());
+            c->setNeedsBarrier(false, JSCompartment::UpdateIon);
+            c->setGCState(JSCompartment::NoGC);
+            ArrayBufferObject::resetArrayBufferList(c);
+            ResetGrayList(c);
         }
 
         rt->gcIncrementalState = NO_INCREMENTAL;
@@ -3826,21 +3856,13 @@ ResetIncrementalGC(JSRuntime *rt, const char *reason)
       }
 
       case SWEEP:
-        for (CompartmentsIter c(rt); !c.done(); c.next()) {
-            c->scheduledForDestruction = false;
+        rt->gcMarker.reset();
 
-            if (c->isGCMarking() && c->activeAnalysis && !c->gcTypesMarked) {
-                AutoCopyFreeListToArenas copy(rt);
-                gcstats::AutoPhase ap1(rt->gcStats, gcstats::PHASE_SWEEP);
-                gcstats::AutoPhase ap2(rt->gcStats, gcstats::PHASE_SWEEP_MARK);
-                gcstats::AutoPhase ap3(rt->gcStats, gcstats::PHASE_SWEEP_MARK_TYPES);
-                rt->gcIncrementalState = MARK_ROOTS;
-                c->markTypes(&rt->gcMarker);
-                rt->gcIncrementalState = SWEEP;
-            }
-        }
+        for (CompartmentsIter c(rt); !c.done(); c.next())
+            c->scheduledForDestruction = false;
 
-        /* If we had started sweeping then sweep to completion here. */
+        /* Finish sweeping the current compartment group, then abort. */
+        rt->gcAbortSweepAfterCurrentGroup = true;
         IncrementalCollectSlice(rt, SliceBudget::Unlimited, gcreason::RESET, GC_NORMAL);
 
         {
-Original file line number
+Diff line change
@@ Expand Up / @@ -578,8 +578,6 @@ JSCompartment::markTypes(JSTracer *trc) @@
             MarkTypeObjectRoot(trc, &type, "mark_types_scan");
             JS_ASSERT(type == i.get<types::TypeObject>());
         }
-        gcTypesMarked = true;
     }
     void
@@ Expand Down @@