Bug 817343 - Ensure that incremental GC validation happens (r=jonco)

--HG--
extra : rebase_source : 63096f25eec9f1008169662eb4011ff30045aed9

js/src/jsgc.cpp

@@ -1592,6 +1592,32 @@ GCMarker::stop()
     stack.reset();
 }
 
+bool
+GCMarker::saveGrayRoots(GrayRootVector &vec)
+{
+    if (grayFailed)
+        return false;
+
+    JS_ASSERT(vec.empty());
+    size_t len = grayRoots.length();
+    GrayRoot *buf = grayRoots.extractRawBuffer();
+    if (!buf)
+        return false;
+    vec.replaceRawBuffer(buf, len);
+    return true;
+}
+
+void
+GCMarker::restoreGrayRoots(GrayRootVector &vec)
+{
+    grayRoots.clearAndFree();
+
+    size_t len = vec.length();
+    GrayRoot *buf = vec.extractRawBuffer();
+    JS_ASSERT(buf); /* This must succeed since vec is not inline. */
+    grayRoots.replaceRawBuffer(buf, len);
+}
+
 void
 GCMarker::reset()
 {
@@ -2754,11 +2780,6 @@ MarkGrayReferences(JSRuntime *rt)
     gcmarker->setMarkColorBlack();
 }
 
-#ifdef DEBUG
-static void
-ValidateIncrementalMarking(JSRuntime *rt);
-#endif
-
 #ifdef DEBUG
 static void
 ValidateIncrementalMarking(JSRuntime *rt)
@@ -2782,14 +2803,10 @@ ValidateIncrementalMarking(JSRuntime *rt)
             return;
     }
 
-    /* Save and reset the lists of live weakmaps for the compartments we are collecting. */
-    WeakMapVector weakmaps;
-    for (GCCompartmentsIter c(rt); !c.done(); c.next()) {
-        if (!WeakMapBase::saveCompartmentWeakMapList(c, weakmaps))
-            return;
-    }
-    for (GCCompartmentsIter c(rt); !c.done(); c.next())
-        WeakMapBase::resetCompartmentWeakMapList(c);
+    /* Save the gray roots. */
+    GCMarker::GrayRootVector grayRoots;
+    if (!gcmarker->saveGrayRoots(grayRoots))
+        return;
 
     /*
      * After this point, the function should run to completion, so we shouldn't
@@ -2811,8 +2828,6 @@ ValidateIncrementalMarking(JSRuntime *rt)
     SliceBudget budget;
     rt->gcIncrementalState = MARK;
     rt->gcMarker.drainMarkStack(budget);
-    MarkWeakReferences(rt, gcstats::PHASE_SWEEP_MARK_WEAK);
-    MarkGrayReferences(rt);
 
     /* Now verify that we have the same mark bits as before. */
     for (GCChunkSet::Range r(rt->gcChunkSet.all()); !r.empty(); r.popFront()) {
@@ -2847,13 +2862,6 @@ ValidateIncrementalMarking(JSRuntime *rt)
                  */
                 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);
             }
         }
@@ -2861,16 +2869,22 @@ ValidateIncrementalMarking(JSRuntime *rt)
         memcpy(bitmap->bitmap, incBitmap.bitmap, sizeof(incBitmap.bitmap));
     }
 
-    /* Restore the weak map lists. */
-    for (GCCompartmentsIter c(rt); !c.done(); c.next())
-        WeakMapBase::resetCompartmentWeakMapList(c);
-    WeakMapBase::restoreCompartmentWeakMapLists(weakmaps);
+    /* Restore gray roots. */
+    gcmarker->restoreGrayRoots(grayRoots);
 
     rt->gcIncrementalState = state;
 }
-
 #endif
 
+static void
+EndMarkPhase(JSRuntime *rt)
+{
+#ifdef DEBUG
+    if (rt->gcIsIncremental && rt->gcValidate)
+        ValidateIncrementalMarking(rt);
+#endif
+}
+
 static void
 DropStringWrappers(JSRuntime *rt)
 {
@@ -3245,11 +3259,6 @@ EndMarkingCompartmentGroup(JSRuntime *rt)
         c->setGCState(JSCompartment::Mark);
     }
 
-#ifdef DEBUG
-    if (rt->gcIsIncremental && rt->gcValidate && rt->gcCompartmentGroupIndex == 0)
-        ValidateIncrementalMarking(rt);
-#endif
-
     JS_ASSERT(rt->gcMarker.isDrained());
 }
 
@@ -3863,6 +3872,8 @@ IncrementalCollectSlice(JSRuntime *rt,
             break;
         }
 
+        EndMarkPhase(rt);
+
         rt->gcIncrementalState = SWEEP;
 
         /*

js/src/jsgc.h

@@ -933,6 +933,24 @@ struct GCMarker : public JSTracer {
     void stop();
     void reset();
 
+    struct GrayRoot {
+        void *thing;
+        JSGCTraceKind kind;
+#ifdef DEBUG
+        JSTraceNamePrinter debugPrinter;
+        const void *debugPrintArg;
+        size_t debugPrintIndex;
+#endif
+
+        GrayRoot(void *thing, JSGCTraceKind kind)
+          : thing(thing), kind(kind) {}
+    };
+
+    typedef Vector<GrayRoot, 0, SystemAllocPolicy> GrayRootVector;
+
+    bool saveGrayRoots(GrayRootVector &vec);
+    void restoreGrayRoots(GrayRootVector &vec);
+
     void pushObject(JSObject *obj) {
         pushTaggedPtr(ObjectTag, obj);
     }
@@ -1065,21 +1083,8 @@ struct GCMarker : public JSTracer {
     /* Count of arenas that are currently in the stack. */
     mozilla::DebugOnly<size_t> markLaterArenas;
 
-    struct GrayRoot {
-        void *thing;
-        JSGCTraceKind kind;
-#ifdef DEBUG
-        JSTraceNamePrinter debugPrinter;
-        const void *debugPrintArg;
-        size_t debugPrintIndex;
-#endif
-
-        GrayRoot(void *thing, JSGCTraceKind kind)
-          : thing(thing), kind(kind) {}
-    };
-
     bool grayFailed;
-    Vector<GrayRoot, 0, SystemAllocPolicy> grayRoots;
+    GrayRootVector grayRoots;
 };
 
 void

js/src/jsweakmap.cpp

@@ -76,30 +76,6 @@ WeakMapBase::resetCompartmentWeakMapList(JSCompartment *c)
     }
 }
 
-bool
-WeakMapBase::saveCompartmentWeakMapList(JSCompartment *c, WeakMapVector &vector)
-{
-    WeakMapBase *m = c->gcWeakMapList;
-    while (m) {
-        if (!vector.append(m))
-            return false;
-        m = m->next;
-    }
-    return true;
-}
-
-void
-WeakMapBase::restoreCompartmentWeakMapLists(WeakMapVector &vector)
-{
-    for (WeakMapBase **p = vector.begin(); p != vector.end(); p++) {
-        WeakMapBase *m = *p;
-        JS_ASSERT(m->next == WeakMapNotInList);
-        JSCompartment *c = m->compartment;
-        m->next = c->gcWeakMapList;
-        c->gcWeakMapList = m;
-    }
-}
-
 void
 WeakMapBase::removeWeakMapFromList(WeakMapBase *weakmap)
 {