Bug 1144925 - Buffer gray roots using a CallbackTracer instead of the GCMarker; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Thu, 19 Mar 2015 13:37:52 -0700
changeset 265469 55e5147d448ee5ab3a017017402f7926bd5219e2
parent 265468 21695668595dc62c662ea2c84489e5237aa700d9
child 265470 734541f861ceea74d3138b09c2e0e4e1cc1ec4de
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1144925
milestone39.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 1144925 - Buffer gray roots using a CallbackTracer instead of the GCMarker; r=jonco
js/src/gc/Marking.cpp
js/src/gc/RootMarking.cpp
js/src/gc/Tracer.cpp
js/src/gc/Tracer.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -189,19 +189,23 @@ CheckMarkedThing(JSTracer *trc, T **thin
     MOZ_ASSERT_IF(!MovingTracer::IsMovingTracer(trc), CurrentThreadCanAccessRuntime(rt));
 
     MOZ_ASSERT(zone->runtimeFromAnyThread() == trc->runtime());
     MOZ_ASSERT(trc->hasTracingDetails());
 
     MOZ_ASSERT(thing->isAligned());
     MOZ_ASSERT(MapTypeToTraceKind<T>::kind == GetGCThingTraceKind(thing));
 
-    bool isGcMarkingTracer = IsMarkingGray(trc) || IsMarkingTracer(trc);
+    /*
+     * Do not check IsMarkingTracer directly -- it should only be used in paths
+     * where we cannot be the gray buffering tracer.
+     */
+    bool isGcMarkingTracer = (trc->callback == nullptr);
 
-    MOZ_ASSERT_IF(zone->requireGCTracer(), isGcMarkingTracer);
+    MOZ_ASSERT_IF(zone->requireGCTracer(), isGcMarkingTracer || IsBufferingGrayRoots(trc));
 
     if (isGcMarkingTracer) {
         GCMarker *gcMarker = static_cast<GCMarker *>(trc);
         MOZ_ASSERT_IF(gcMarker->shouldCheckCompartments(),
                       zone->isCollecting() || rt->isAtomsZone(zone));
 
         MOZ_ASSERT_IF(gcMarker->markColor() == GRAY,
                       !zone->isGCMarkingBlack() || rt->isAtomsZone(zone));
--- a/js/src/gc/RootMarking.cpp
+++ b/js/src/gc/RootMarking.cpp
@@ -413,17 +413,16 @@ js::gc::MarkPersistentRootedChains(JSTra
 
 void
 js::gc::GCRuntime::markRuntime(JSTracer *trc,
                                TraceOrMarkRuntime traceOrMark,
                                TraceRootsOrUsedSaved rootsSource)
 {
     gcstats::AutoPhase ap(stats, gcstats::PHASE_MARK_ROOTS);
 
-    MOZ_ASSERT(trc->callback != GCMarker::GrayCallback);
     MOZ_ASSERT(traceOrMark == TraceRuntime || traceOrMark == MarkRuntime);
     MOZ_ASSERT(rootsSource == TraceRoots || rootsSource == UseSavedRoots);
 
     MOZ_ASSERT(!rt->mainThread.suppressGC);
 
     if (traceOrMark == MarkRuntime) {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_MARK_CCWS);
 
@@ -557,34 +556,21 @@ void
 js::gc::GCRuntime::bufferGrayRoots()
 {
     // Precondition: the state has been reset to "unused" after the last GC
     //               and the zone's buffers have been cleared.
     MOZ_ASSERT(grayBufferState == GrayBufferState::Unused);
     for (GCZonesIter zone(rt); !zone.done(); zone.next())
         MOZ_ASSERT(zone->gcGrayRoots.empty());
 
-    // Transform the GCMarker into an unholy CallbackTracer doppleganger.
-    MOZ_ASSERT(!IsMarkingGray(&marker));
-    MOZ_ASSERT(IsMarkingTracer(&marker));
-    MOZ_ASSERT(!marker.callback);
-    MOZ_ASSERT(!marker.bufferingGrayRootsFailed);
-    marker.callback = GCMarker::GrayCallback;
-    MOZ_ASSERT(IsMarkingGray(&marker));
 
+    BufferGrayRootsTracer grayBufferer(rt);
     if (JSTraceDataOp op = grayRootTracer.op)
-        (*op)(&marker, grayRootTracer.data);
+        (*op)(&grayBufferer, grayRootTracer.data);
 
     // Propagate the failure flag from the marker to the runtime.
-    if (marker.bufferingGrayRootsFailed) {
+    if (grayBufferer.failed()) {
       grayBufferState = GrayBufferState::Failed;
       resetBufferedGrayRoots();
     } else {
       grayBufferState = GrayBufferState::Okay;
     }
-
-    // Restore the GCMarker to its former correctness.
-    MOZ_ASSERT(IsMarkingGray(&marker));
-    marker.bufferingGrayRootsFailed = false;
-    marker.callback = nullptr;
-    MOZ_ASSERT(!IsMarkingGray(&marker));
-    MOZ_ASSERT(IsMarkingTracer(&marker));
 }
--- a/js/src/gc/Tracer.cpp
+++ b/js/src/gc/Tracer.cpp
@@ -508,17 +508,16 @@ MarkStack::sizeOfExcludingThis(mozilla::
 }
 
 /*
  * DoNotTraceWeakMaps: the GC is recomputing the liveness of WeakMap entries,
  * so we delay visting entries.
  */
 GCMarker::GCMarker(JSRuntime *rt)
   : JSTracer(rt, nullptr, DoNotTraceWeakMaps),
-    bufferingGrayRootsFailed(false),
     stack(size_t(-1)),
     color(BLACK),
     unmarkedArenaStackTop(nullptr),
     markLaterArenas(0),
     started(false),
     strictCompartmentChecking(false)
 {
 }
@@ -663,19 +662,19 @@ GCRuntime::markBufferedGrayRoots(JS::Zon
 #ifdef DEBUG
         marker.setTracingDetails(elem->debugPrinter, elem->debugPrintArg, elem->debugPrintIndex);
 #endif
         MarkKind(&marker, &elem->thing, elem->kind);
     }
 }
 
 void
-GCMarker::appendGrayRoot(void *thing, JSGCTraceKind kind)
+BufferGrayRootsTracer::appendGrayRoot(void *thing, JSGCTraceKind kind)
 {
-    MOZ_ASSERT(started);
+    MOZ_ASSERT(runtime()->isHeapBusy());
 
     if (bufferingGrayRootsFailed)
         return;
 
     GrayRoot root(thing, kind);
 #ifdef DEBUG
     root.debugPrinter = debugPrinter();
     root.debugPrintArg = debugPrintArg();
@@ -698,25 +697,16 @@ GCMarker::appendGrayRoot(void *thing, JS
           default:
             break;
         }
         if (!zone->gcGrayRoots.append(root))
             bufferingGrayRootsFailed = true;
     }
 }
 
-void
-GCMarker::GrayCallback(JSTracer *trc, void **thingp, JSGCTraceKind kind)
-{
-    MOZ_ASSERT(thingp);
-    MOZ_ASSERT(*thingp);
-    GCMarker *gcmarker = static_cast<GCMarker *>(trc);
-    gcmarker->appendGrayRoot(*thingp, kind);
-}
-
 size_t
 GCMarker::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
     size_t size = stack.sizeOfExcludingThis(mallocSizeOf);
     for (ZonesIter zone(runtime(), WithAtoms); !zone.done(); zone.next())
         size += zone->gcGrayRoots.sizeOfExcludingThis(mallocSizeOf);
     return size;
 }
--- a/js/src/gc/Tracer.h
+++ b/js/src/gc/Tracer.h
@@ -196,21 +196,16 @@ class GCMarker : public JSTracer
     }
 
     bool isDrained() {
         return isMarkStackEmpty() && !unmarkedArenaStackTop;
     }
 
     bool drainMarkStack(SliceBudget &budget);
 
-    /* Set to false if we OOM while buffering gray roots. */
-    bool bufferingGrayRootsFailed;
-
-    static void GrayCallback(JSTracer *trc, void **thing, JSGCTraceKind kind);
-
     void setGCMode(JSGCMode mode) { stack.setGCMode(mode); }
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 #ifdef DEBUG
     bool shouldCheckCompartments() { return strictCompartmentChecking; }
 #endif
 
@@ -325,33 +320,54 @@ class GCMarker : public JSTracer
 
     /*
      * If this is true, all marked objects must belong to a compartment being
      * GCed. This is used to look for compartment bugs.
      */
     mozilla::DebugOnly<bool> strictCompartmentChecking;
 };
 
+// Append traced things to a buffer on the zone for use later in the GC.
+// See the comment in GCRuntime.h above grayBufferState for details.
+class BufferGrayRootsTracer : public JSTracer
+{
+    // Set to false if we OOM while buffering gray roots.
+    bool bufferingGrayRootsFailed;
+
+    void appendGrayRoot(void *thing, JSGCTraceKind kind);
+
+  public:
+    explicit BufferGrayRootsTracer(JSRuntime *rt)
+      : JSTracer(rt, grayTraceCallback), bufferingGrayRootsFailed(false)
+    {}
+
+    static void grayTraceCallback(JSTracer *trc, void **thingp, JSGCTraceKind kind) {
+        static_cast<BufferGrayRootsTracer *>(trc)->appendGrayRoot(*thingp, kind);
+    }
+
+    bool failed() const { return bufferingGrayRootsFailed; }
+};
+
 void
 SetMarkStackLimit(JSRuntime *rt, size_t limit);
 
 // Return true if this trace is happening on behalf of gray buffering during
 // the marking phase of incremental GC.
 inline bool
-IsMarkingGray(JSTracer *trc)
+IsBufferingGrayRoots(JSTracer *trc)
 {
-    return trc->callback == js::GCMarker::GrayCallback;
+    return trc->callback == BufferGrayRootsTracer::grayTraceCallback;
 }
 
 // Return true if this trace is happening on behalf of the marking phase of GC.
 inline bool
 IsMarkingTracer(JSTracer *trc)
 {
     // If we call this on the gray-buffering tracer, then we have encountered a
     // marking path that will be wrong when tracing with a callback marker to
     // enqueue for deferred gray marking.
-    MOZ_ASSERT(!IsMarkingGray(trc));
+    MOZ_ASSERT(!IsBufferingGrayRoots(trc));
     return trc->callback == nullptr;
 }
 
 } /* namespace js */
 
 #endif /* js_Tracer_h */