Bug 820422 - GC: Store buffered gray roots per-compartment r=billm

--HG--
extra : rebase_source : 0fb2e6d96c8a4cb91c045444be0ff12bf5eb4010

js/src/gc/GCInternals.h

@@ -16,6 +16,9 @@ namespace gc {
 void
 MarkRuntime(JSTracer *trc, bool useSavedRoots = false);
 
+void
+BufferGrayRoots(GCMarker *gcmarker);
+
 class AutoCopyFreeListToArenas {
     JSRuntime *runtime;
 

js/src/gc/RootMarking.cpp

@@ -791,15 +791,20 @@ js::gc::MarkRuntime(JSTracer *trc, bool useSavedRoots)
     if (JSTraceDataOp op = rt->gcBlackRootsTraceOp)
         (*op)(trc, rt->gcBlackRootsData);
 
-    /* During GC, this buffers up the gray roots and doesn't mark them. */
+    /* During GC, we don't mark gray roots at this stage. */
     if (JSTraceDataOp op = rt->gcGrayRootsTraceOp) {
-        if (IS_GC_MARKING_TRACER(trc)) {
-            GCMarker *gcmarker = static_cast<GCMarker *>(trc);
-            gcmarker->startBufferingGrayRoots();
+        if (!IS_GC_MARKING_TRACER(trc))
             (*op)(trc, rt->gcGrayRootsData);
-            gcmarker->endBufferingGrayRoots();
-        } else {
-            (*op)(trc, rt->gcGrayRootsData);
-        }
+    }
+}
+
+void
+js::gc::BufferGrayRoots(GCMarker *gcmarker)
+{
+    JSRuntime *rt = gcmarker->runtime;
+    if (JSTraceDataOp op = rt->gcGrayRootsTraceOp) {
+        gcmarker->startBufferingGrayRoots();
+        (*op)(gcmarker, rt->gcGrayRootsData);
+        gcmarker->endBufferingGrayRoots();
     }
 }

js/src/jscompartment.cpp

@@ -75,6 +75,7 @@ JSCompartment::JSCompartment(JSRuntime *rt)
     gcIncomingGrayPointers(NULL),
     gcLiveArrayBuffers(NULL),
     gcWeakMapList(NULL),
+    gcGrayRoots(),
     gcMallocBytes(0),
     debugModeBits(rt->debugMode ? DebugFromC : 0),
     watchpointMap(NULL),

js/src/jscompartment.h

@@ -375,6 +375,9 @@ struct JSCompartment : private JS::shadow::Compartment, public js::gc::GraphNode
     /* Linked list of live weakmaps in this compartment. */
     js::WeakMapBase              *gcWeakMapList;
 
+    /* This compartment's gray roots. */
+    js::Vector<js::GrayRoot, 0, js::SystemAllocPolicy> gcGrayRoots;
+
   private:
     /*
      * Malloc counter to measure memory pressure for GC scheduling. It runs from

js/src/jsgc.cpp

@@ -1558,9 +1558,6 @@ GCMarker::start(JSRuntime *rt)
     JS_ASSERT(!unmarkedArenaStackTop);
     JS_ASSERT(markLaterArenas == 0);
 
-    JS_ASSERT(grayRoots.empty());
-    JS_ASSERT(!grayFailed);
-
     /*
      * The GC is recomputing the liveness of WeakMap entries, so we delay
      * visting entries.
@@ -1579,11 +1576,10 @@ GCMarker::stop()
     JS_ASSERT(!unmarkedArenaStackTop);
     JS_ASSERT(markLaterArenas == 0);
 
-    grayRoots.clearAndFree();
-    grayFailed = false;
-
     /* Free non-ballast stack memory. */
     stack.reset();
+
+    resetBufferedGrayRoots();
 }
 
 void
@@ -1606,9 +1602,6 @@ GCMarker::reset()
     }
     JS_ASSERT(isDrained());
     JS_ASSERT(!markLaterArenas);
-
-    grayRoots.clearAndFree();
-    grayFailed = false;
 }
 
 /*
@@ -1721,6 +1714,10 @@ GCMarker::hasBufferedGrayRoots() const
 void
 GCMarker::startBufferingGrayRoots()
 {
+    JS_ASSERT(!grayFailed);
+    for (GCCompartmentsIter c(runtime); !c.done(); c.next())
+        JS_ASSERT(c->gcGrayRoots.empty());
+
     JS_ASSERT(!callback);
     callback = GrayCallback;
     JS_ASSERT(IS_GC_MARKING_TRACER(this));
@@ -1734,44 +1731,33 @@ GCMarker::endBufferingGrayRoots()
     JS_ASSERT(IS_GC_MARKING_TRACER(this));
 }
 
+void
+GCMarker::resetBufferedGrayRoots()
+{
+    for (GCCompartmentsIter c(runtime); !c.done(); c.next())
+        c->gcGrayRoots.clearAndFree();
+    grayFailed = false;
+}
+
 void
 GCMarker::markBufferedGrayRoots()
 {
     JS_ASSERT(!grayFailed);
 
-    unsigned markCount = 0;
-
-    GrayRoot *elem = grayRoots.begin();
-    GrayRoot *write = elem;
-    for (; elem != grayRoots.end(); elem++) {
+    for (GCCompartmentGroupIter c(runtime); !c.done(); c.next()) {
+        JS_ASSERT(c->isGCMarkingGray());
+        for (GrayRoot *elem = c->gcGrayRoots.begin(); elem != c->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;
-        if (static_cast<Cell *>(tmp)->compartment()->isGCMarkingGray()) {
+            void *tmp = elem->thing;
             JS_SET_TRACING_LOCATION(this, (void *)&elem->thing);
             MarkKind(this, &tmp, elem->kind);
             JS_ASSERT(tmp == elem->thing);
-            ++markCount;
-        } else {
-            if (write != elem)
-                *write = *elem;
-            ++write;
         }
     }
-    JS_ASSERT(markCount == elem - write);
-    grayRoots.shrinkBy(elem - write);
-}
-
-void
-GCMarker::markBufferedGrayRootCompartmentsAlive()
-{
-    for (GrayRoot *elem = grayRoots.begin(); elem != grayRoots.end(); elem++) {
-        Cell *thing = static_cast<Cell *>(elem->thing);
-        thing->compartment()->maybeAlive = true;
-    }
 }
 
 void
@@ -1789,9 +1775,13 @@ GCMarker::appendGrayRoot(void *thing, JSGCTraceKind kind)
     root.debugPrintIndex = debugPrintIndex;
 #endif
 
-    if (!grayRoots.append(root)) {
-        grayRoots.clearAndFree();
-        grayFailed = true;
+    JSCompartment *comp = static_cast<Cell *>(thing)->compartment();
+    if (comp->isCollecting()) {
+        comp->maybeAlive = true;
+        if (!comp->gcGrayRoots.append(root)) {
+            grayFailed = true;
+            resetBufferedGrayRoots();
+        }
     }
 }
 
@@ -1805,8 +1795,10 @@ GCMarker::GrayCallback(JSTracer *trc, void **thingp, JSGCTraceKind kind)
 size_t
 GCMarker::sizeOfExcludingThis(JSMallocSizeOfFun mallocSizeOf) const
 {
-    return stack.sizeOfExcludingThis(mallocSizeOf) +
-           grayRoots.sizeOfExcludingThis(mallocSizeOf);
+    size_t size = stack.sizeOfExcludingThis(mallocSizeOf);
+    for (CompartmentsIter c(runtime); !c.done(); c.next())
+        size += c->gcGrayRoots.sizeOfExcludingThis(mallocSizeOf);
+    return size;
 }
 
 void
@@ -2641,6 +2633,7 @@ BeginMarkPhase(JSRuntime *rt)
     }
 
     MarkRuntime(gcmarker);
+    BufferGrayRoots(gcmarker);
 
     /*
      * This code ensures that if a compartment is "dead", then it will be
@@ -2681,9 +2674,6 @@ BeginMarkPhase(JSRuntime *rt)
             c->maybeAlive = true;
     }
 
-    /* Set the maybeAlive flag based on gray roots. */
-    rt->gcMarker.markBufferedGrayRootCompartmentsAlive();
-
     /*
      * For black roots, code in gc/Marking.cpp will already have set maybeAlive
      * during MarkRuntime.
@@ -2698,7 +2688,7 @@ BeginMarkPhase(JSRuntime *rt)
     return true;
 }
 
-template <class CompartmentIter>
+template <class CompartmentIterT>
 static void
 MarkWeakReferences(JSRuntime *rt, gcstats::Phase phase)
 {
@@ -2710,7 +2700,7 @@ MarkWeakReferences(JSRuntime *rt, gcstats::Phase phase)
 
     for (;;) {
         bool markedAny = false;
-        for (CompartmentIter c(rt); !c.done(); c.next()) {
+        for (CompartmentIterT c(rt); !c.done(); c.next()) {
             markedAny |= WatchpointMap::markCompartmentIteratively(c, gcmarker);
             markedAny |= WeakMapBase::markCompartmentIteratively(c, gcmarker);
         }
@@ -3776,6 +3766,10 @@ ResetIncrementalGC(JSRuntime *rt, const char *reason)
       case MARK: {
         /* Cancel any ongoing marking. */
         AutoCopyFreeListToArenas copy(rt);
+
+        rt->gcMarker.reset();
+        rt->gcMarker.stop();
+
         for (GCCompartmentsIter c(rt); !c.done(); c.next()) {
             if (c->isGCMarking()) {
                 c->setNeedsBarrier(false, JSCompartment::UpdateIon);
@@ -3784,9 +3778,6 @@ ResetIncrementalGC(JSRuntime *rt, const char *reason)
             }
         }
 
-        rt->gcMarker.reset();
-        rt->gcMarker.stop();
-
         rt->gcIncrementalState = NO_INCREMENTAL;
 
         JS_ASSERT(!rt->gcStrictCompartmentChecking);

js/src/jsgc.h

@@ -902,6 +902,19 @@ struct SliceBudget {
 
 static const size_t MARK_STACK_LENGTH = 32768;
 
+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) {}
+};
+
 struct GCMarker : public JSTracer {
   private:
     /*
@@ -1002,15 +1015,16 @@ struct GCMarker : public JSTracer {
      * Gray marking must be done after all black marking is complete. However,
      * we do not have write barriers on XPConnect roots. Therefore, XPConnect
      * roots must be accumulated in the first slice of incremental GC. We
-     * accumulate these roots in the GrayRootMarker and then mark them later,
-     * after black marking is complete. This accumulation can fail, but in that
-     * case we switch to non-incremental GC.
+     * accumulate these roots in the each compartment's gcGrayRoots vector and
+     * 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 markBufferedGrayRootCompartmentsAlive();
 
     static void GrayCallback(JSTracer *trc, void **thing, JSGCTraceKind kind);
 
@@ -1070,21 +1084,7 @@ 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;
 };
 
 void