Bug 820871 - GC: Validate gray marking r=billm
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 10 Dec 2012 13:42:41 +0000
changeset 115892 34840088cc10a7fb9cd94ea56ff07f449bbf1568
parent 115891 37b4c4b9d7d6814e559d7a47d83652e68a5689aa
child 115893 903c331107c2dcca8400368cce112792db3bd221
push id24028
push useremorley@mozilla.com
push dateThu, 13 Dec 2012 15:56:02 +0000
treeherderautoland@9db79b97abbb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs820871
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 820871 - GC: Validate gray marking r=billm
js/src/jsgc.cpp
js/src/jsgc.h
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1735,33 +1735,31 @@ void
 GCMarker::resetBufferedGrayRoots()
 {
     for (GCCompartmentsIter c(runtime); !c.done(); c.next())
         c->gcGrayRoots.clearAndFree();
     grayFailed = false;
 }
 
 void
-GCMarker::markBufferedGrayRoots()
+GCMarker::markBufferedGrayRoots(JSCompartment *comp)
 {
     JS_ASSERT(!grayFailed);
-
-    for (GCCompartmentGroupIter c(runtime); !c.done(); c.next()) {
-        JS_ASSERT(c->isGCMarkingGray());
-        for (GrayRoot *elem = c->gcGrayRoots.begin(); elem != c->gcGrayRoots.end(); elem++) {
+    JS_ASSERT(comp->isGCMarkingGray());
+
+    for (GrayRoot *elem = comp->gcGrayRoots.begin(); elem != comp->gcGrayRoots.end(); elem++) {
 #ifdef DEBUG
-            debugPrinter = elem->debugPrinter;
-            debugPrintArg = elem->debugPrintArg;
-            debugPrintIndex = elem->debugPrintIndex;
+        debugPrinter = elem->debugPrinter;
+        debugPrintArg = elem->debugPrintArg;
+        debugPrintIndex = elem->debugPrintIndex;
 #endif
-            void *tmp = elem->thing;
-            JS_SET_TRACING_LOCATION(this, (void *)&elem->thing);
-            MarkKind(this, &tmp, elem->kind);
-            JS_ASSERT(tmp == elem->thing);
-        }
+        void *tmp = elem->thing;
+        JS_SET_TRACING_LOCATION(this, (void *)&elem->thing);
+        MarkKind(this, &tmp, elem->kind);
+        JS_ASSERT(tmp == elem->thing);
     }
 }
 
 void
 GCMarker::appendGrayRoot(void *thing, JSGCTraceKind kind)
 {
     JS_ASSERT(started);
 
@@ -2716,52 +2714,65 @@ MarkWeakReferences(JSRuntime *rt, gcstat
 }
 
 static void
 MarkWeakReferencesInCurrentGroup(JSRuntime *rt, gcstats::Phase phase)
 {
     MarkWeakReferences<GCCompartmentGroupIter>(rt, phase);
 }
 
-#ifdef DEBUG
-static void
-MarkAllWeakReferences(JSRuntime *rt, gcstats::Phase phase)
-{
-    MarkWeakReferences<GCCompartmentsIter>(rt, phase);
-}
-#endif
-
+template <class CompartmentIterT>
 static void
 MarkGrayReferences(JSRuntime *rt)
 {
     GCMarker *gcmarker = &rt->gcMarker;
 
     {
         gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP_MARK);
         gcstats::AutoPhase ap1(rt->gcStats, gcstats::PHASE_SWEEP_MARK_GRAY);
         gcmarker->setMarkColorGray();
         if (gcmarker->hasBufferedGrayRoots()) {
-            gcmarker->markBufferedGrayRoots();
+            for (CompartmentIterT c(rt); !c.done(); c.next())
+                gcmarker->markBufferedGrayRoots(c);
         } else {
+            JS_ASSERT(!rt->gcIsIncremental);
             if (JSTraceDataOp op = rt->gcGrayRootsTraceOp)
                 (*op)(gcmarker, rt->gcGrayRootsData);
         }
         SliceBudget budget;
         gcmarker->drainMarkStack(budget);
     }
 
-    MarkWeakReferencesInCurrentGroup(rt, gcstats::PHASE_SWEEP_MARK_GRAY_WEAK);
+    MarkWeakReferences<CompartmentIterT>(rt, gcstats::PHASE_SWEEP_MARK_GRAY_WEAK);
 
     JS_ASSERT(gcmarker->isDrained());
 
     gcmarker->setMarkColorBlack();
 }
 
+static void
+MarkGrayReferencesInCurrentGroup(JSRuntime *rt)
+{
+    MarkGrayReferences<GCCompartmentGroupIter>(rt);
+}
+
 #ifdef DEBUG
 
+static void
+MarkAllWeakReferences(JSRuntime *rt, gcstats::Phase phase)
+{
+    MarkWeakReferences<GCCompartmentsIter>(rt, phase);
+}
+
+static void
+MarkAllGrayReferences(JSRuntime *rt)
+{
+    MarkGrayReferences<GCCompartmentsIter>(rt);
+}
+
 class js::gc::MarkingValidator
 {
   public:
     MarkingValidator(JSRuntime *rt);
     ~MarkingValidator();
     void nonIncrementalMark();
     void validate();
 
@@ -2861,16 +2872,30 @@ js::gc::MarkingValidator::nonIncremental
 
     SliceBudget budget;
     runtime->gcIncrementalState = MARK;
     runtime->gcMarker.drainMarkStack(budget);
 
     {
         gcstats::AutoPhase ap(runtime->gcStats, gcstats::PHASE_SWEEP);
         MarkAllWeakReferences(runtime, gcstats::PHASE_SWEEP_MARK_WEAK);
+
+        /* Update compartment state for gray marking. */
+        for (GCCompartmentsIter c(runtime); !c.done(); c.next()) {
+            JS_ASSERT(c->isGCMarkingBlack());
+            c->setGCState(JSCompartment::MarkGray);
+        }
+
+        MarkAllGrayReferences(runtime);
+
+        /* Restore compartment state. */
+        for (GCCompartmentsIter c(runtime); !c.done(); c.next()) {
+            JS_ASSERT(c->isGCMarkingGray());
+            c->setGCState(JSCompartment::Mark);
+        }
     }
 
     /* Take a copy of the non-incremental mark state and restore the original. */
     for (GCChunkSet::Range r(runtime->gcChunkSet.all()); !r.empty(); r.popFront()) {
         Chunk *chunk = r.front();
         ChunkBitmap *bitmap = &chunk->bitmap;
         ChunkBitmap *entry = map.lookup(chunk)->value;
         js::Swap(*entry, *bitmap);
@@ -2925,16 +2950,23 @@ js::gc::MarkingValidator::validate()
                 Cell *cell = (Cell *)thing;
 
                 /*
                  * If a non-incremental GC wouldn't have collected a cell, then
                  * an incremental GC won't collect it.
                  */
                 JS_ASSERT_IF(bitmap->isMarked(cell, BLACK), incBitmap->isMarked(cell, BLACK));
 
+                /*
+                 * If the cycle collector isn't allowed to collect an object
+                 * after a non-incremental GC has run, then it isn't allowed to
+                 * collected it after an incremental GC.
+                 */
+                JS_ASSERT_IF(!bitmap->isMarked(cell, GRAY), !incBitmap->isMarked(cell, GRAY));
+
                 thing += Arena::thingSize(kind);
             }
         }
     }
 }
 
 #endif
 
@@ -3325,17 +3357,17 @@ EndMarkingCompartmentGroup(JSRuntime *rt
     }
 
     /* Mark incoming gray pointers from previously swept compartments. */
     rt->gcMarker.setMarkColorGray();
     MarkIncomingCrossCompartmentPointers(rt, GRAY);
     rt->gcMarker.setMarkColorBlack();
 
     /* Mark gray roots and mark transitively inside the current compartment group. */
-    MarkGrayReferences(rt);
+    MarkGrayReferencesInCurrentGroup(rt);
 
     /* Restore marking state. */
     for (GCCompartmentGroupIter c(rt); !c.done(); c.next()) {
         JS_ASSERT(c->isGCMarkingGray());
         c->setGCState(JSCompartment::Mark);
     }
 
     JS_ASSERT(rt->gcMarker.isDrained());
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1019,17 +1019,17 @@ struct GCMarker : public JSTracer {
      * then mark them later, after black marking is complete for each
      * compartment. This accumulation can fail, but in that case we switch to
      * non-incremental GC.
      */
     bool hasBufferedGrayRoots() const;
     void startBufferingGrayRoots();
     void endBufferingGrayRoots();
     void resetBufferedGrayRoots();
-    void markBufferedGrayRoots();
+    void markBufferedGrayRoots(JSCompartment *comp);
 
     static void GrayCallback(JSTracer *trc, void **thing, JSGCTraceKind kind);
 
     size_t sizeOfExcludingThis(JSMallocSizeOfFun mallocSizeOf) const;
 
     MarkStack<uintptr_t> stack;
 
   private: