From aab3d5eeb8c8978424648fd9dff821b7ca209199 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Tue, 11 Dec 2012 17:03:44 +0000 Subject: [PATCH] Bug 820422 - GC: Store buffered gray roots per-compartment r=billm --HG-- extra : rebase_source : 0fb2e6d96c8a4cb91c045444be0ff12bf5eb4010 --- js/src/gc/GCInternals.h | 3 ++ js/src/gc/RootMarking.cpp | 21 ++++++---- js/src/jscompartment.cpp | 1 + js/src/jscompartment.h | 3 ++ js/src/jsgc.cpp | 87 ++++++++++++++++++--------------------- js/src/jsgc.h | 36 ++++++++-------- 6 files changed, 77 insertions(+), 74 deletions(-) diff --git a/js/src/gc/GCInternals.h b/js/src/gc/GCInternals.h index 492a2b0e1acc..b5c6c799a3f6 100644 --- a/js/src/gc/GCInternals.h +++ b/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; diff --git a/js/src/gc/RootMarking.cpp b/js/src/gc/RootMarking.cpp index 033ea187b9b6..1206acaddd3e 100644 --- a/js/src/gc/RootMarking.cpp +++ b/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(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(); } } diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index d85914366b1d..bd7ac27f0a5a 100644 --- a/js/src/jscompartment.cpp +++ b/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), diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index 897c2434bf46..5906207ccf02 100644 --- a/js/src/jscompartment.h +++ b/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 gcGrayRoots; + 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 index c00e6a714cd8..fa7299cac170 100644 --- a/js/src/jsgc.cpp +++ b/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(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(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(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 +template 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); diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 3ebb13cdd72b..fb70b691c1c2 100644 --- a/js/src/jsgc.h +++ b/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 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 grayRoots; }; void