Bug 1380030 - Simplify and refactor use of isMarked*() methods r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 12 Jul 2017 18:31:55 +0100
changeset 607722 04364ec85017d7646b11b1ed1f3e1cf61a59075b
parent 607721 e2339ab06af56cb05c0d5b917d6f660952b45a69
child 607723 933aa2989b9af7f7746c5b45b41c5ff3ce52cd20
push id68095
push userbmo:rbarker@mozilla.com
push dateWed, 12 Jul 2017 20:01:47 +0000
reviewerssfink
bugs1380030
milestone56.0a1
Bug 1380030 - Simplify and refactor use of isMarked*() methods r=sfink
js/src/gc/Marking.cpp
js/src/gc/Verifier.cpp
js/src/jsapi-tests/testGCGrayMarking.cpp
js/src/jsapi-tests/testGCHeapPostBarriers.cpp
js/src/jsapi-tests/testGCMarking.cpp
js/src/jsfriendapi.cpp
js/src/jsgc.cpp
js/src/jsscript.cpp
js/src/jsweakmap.cpp
js/src/proxy/Wrapper.cpp
js/src/shell/js.cpp
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -1058,17 +1058,17 @@ Shape::traceChildren(JSTracer* trc)
         TraceManuallyBarrieredEdge(trc, &asAccessorShape().getterObj, "getter");
     if (hasSetterObject())
         TraceManuallyBarrieredEdge(trc, &asAccessorShape().setterObj, "setter");
 }
 inline void
 js::GCMarker::eagerlyMarkChildren(Shape* shape)
 {
     MOZ_ASSERT_IF(markColor() == GRAY, shape->isMarkedGray());
-    MOZ_ASSERT_IF(markColor() == BLACK, shape->isMarkedAny());
+    MOZ_ASSERT_IF(markColor() == BLACK, shape->isMarkedBlack());
 
     do {
         // Special case: if a base shape has a shape table then all its pointers
         // must point to this shape or an anscestor.  Since these pointers will
         // be traced by this loop they do not need to be traced here as well.
         BaseShape* base = shape->base();
         CheckTraversedEdge(shape, base);
         if (mark(base)) {
@@ -2436,17 +2436,17 @@ GCMarker::pushValueArray(JSObject* obj, 
     if (!stack.push(obj, start, end))
         delayMarkingChildren(obj);
 }
 
 void
 GCMarker::repush(JSObject* obj)
 {
     MOZ_ASSERT_IF(markColor() == GRAY, gc::TenuredCell::fromPointer(obj)->isMarkedGray());
-    MOZ_ASSERT_IF(markColor() == BLACK, gc::TenuredCell::fromPointer(obj)->isMarkedAny());
+    MOZ_ASSERT_IF(markColor() == BLACK, gc::TenuredCell::fromPointer(obj)->isMarkedBlack());
     pushTaggedPtr(obj);
 }
 
 void
 GCMarker::enterWeakMarkingMode()
 {
     MOZ_ASSERT(tag_ == TracerKindTag::Marking);
     if (linearWeakMarkingDisabled_)
@@ -3314,18 +3314,17 @@ FOR_EACH_PUBLIC_TAGGED_GC_POINTER_TYPE(I
  *   of the containers, we must add unmark-graying read barriers to these
  *   containers.
  */
 
 #ifdef DEBUG
 struct AssertNonGrayTracer : public JS::CallbackTracer {
     explicit AssertNonGrayTracer(JSRuntime* rt) : JS::CallbackTracer(rt) {}
     void onChild(const JS::GCCellPtr& thing) override {
-        MOZ_ASSERT_IF(thing.asCell()->isTenured(),
-                      !thing.asCell()->asTenured().isMarkedGray());
+        MOZ_ASSERT(!thing.asCell()->isMarkedGray());
     }
 };
 #endif
 
 class UnmarkGrayTracer : public JS::CallbackTracer
 {
   public:
     // We set weakMapAction to DoNotTraceWeakMaps because the cycle collector
@@ -3443,17 +3442,17 @@ MarkInfo
 GetMarkInfo(Cell* rawCell)
 {
     if (!rawCell->isTenured())
         return MarkInfo::NURSERY;
 
     TenuredCell* cell = &rawCell->asTenured();
     if (cell->isMarkedGray())
         return MarkInfo::GRAY;
-    if (cell->isMarkedAny())
+    if (cell->isMarkedBlack())
         return MarkInfo::BLACK;
     return MarkInfo::UNMARKED;
 }
 
 uintptr_t*
 GetMarkWordAddress(Cell* cell)
 {
     if (!cell->isTenured())
--- a/js/src/gc/Verifier.cpp
+++ b/js/src/gc/Verifier.cpp
@@ -648,18 +648,17 @@ void
 CheckGrayMarkingTracer::checkCell(Cell* cell)
 {
     Cell* parent = parentCell();
     if (!cell->isTenured() || !parent || !parent->isTenured())
         return;
 
     TenuredCell* tenuredCell = &cell->asTenured();
     TenuredCell* tenuredParent = &parent->asTenured();
-    if (tenuredParent->isMarkedAny() && !tenuredParent->isMarkedGray() &&
-        tenuredCell->isMarkedGray())
+    if (tenuredParent->isMarkedBlack() && tenuredCell->isMarkedGray())
     {
         failures++;
         fprintf(stderr, "Found black to gray edge to %s %p\n",
                 GCTraceKindToAscii(cell->getTraceKind()), cell);
         dumpCellPath();
     }
 }
 
--- a/js/src/jsapi-tests/testGCGrayMarking.cpp
+++ b/js/src/jsapi-tests/testGCGrayMarking.cpp
@@ -812,17 +812,17 @@ bool IterateObjectChain(JSObject* chain,
 
     return true;
 }
 
 static bool
 IsMarkedBlack(Cell* cell)
 {
     TenuredCell* tc = &cell->asTenured();
-    return tc->isMarkedAny() && !tc->isMarkedGray();
+    return tc->isMarkedBlack();
 }
 
 static bool
 IsMarkedGray(Cell* cell)
 {
     TenuredCell* tc = &cell->asTenured();
     bool isGray = tc->isMarkedGray();
     MOZ_ASSERT_IF(isGray, tc->isMarkedAny());
--- a/js/src/jsapi-tests/testGCHeapPostBarriers.cpp
+++ b/js/src/jsapi-tests/testGCHeapPostBarriers.cpp
@@ -241,18 +241,18 @@ BEGIN_TEST(testUnbarrieredEquality)
     }
 
     // Sanity check that the barriers normally mark things black.
     {
         JS::Heap<JSObject*> heap(obj);
         JS::Heap<JSObject*> heap2(obj2);
         heap.get();
         heap2.get();
-        CHECK(cell->isMarkedAny());
-        CHECK(cell2->isMarkedAny());
+        CHECK(cell->isMarkedBlack());
+        CHECK(cell2->isMarkedBlack());
     }
 
     return true;
 }
 
 template <typename ObjectT, typename WrapperT>
 bool
 TestWrapper(ObjectT obj, ObjectT obj2, WrapperT& wrapper, WrapperT& wrapper2)
--- a/js/src/jsapi-tests/testGCMarking.cpp
+++ b/js/src/jsapi-tests/testGCMarking.cpp
@@ -362,19 +362,19 @@ BEGIN_TEST(testIncrementalRoots)
     rt->gc.startDebugGC(GC_NORMAL, budget);
 
     // We'd better be between iGC slices now. There's always a risk that
     // something will decide that we need to do a full GC (such as gczeal, but
     // that is turned off.)
     MOZ_ASSERT(JS::IsIncrementalGCInProgress(cx));
 
     // And assert that the mark bits are as we expect them to be.
-    MOZ_ASSERT(vec[0]->asTenured().isMarkedAny());
-    MOZ_ASSERT(!leafHandle->asTenured().isMarkedAny());
-    MOZ_ASSERT(!leafOwnerHandle->asTenured().isMarkedAny());
+    MOZ_ASSERT(vec[0]->asTenured().isMarkedBlack());
+    MOZ_ASSERT(!leafHandle->asTenured().isMarkedBlack());
+    MOZ_ASSERT(!leafOwnerHandle->asTenured().isMarkedBlack());
 
 #ifdef DEBUG
     // Remember the current GC number so we can assert that no GC occurs
     // between operations.
     auto currentGCNumber = rt->gc.gcNumber();
 #endif
 
     // Now do the incremental GC's worst nightmare: rip an unmarked object
@@ -382,37 +382,37 @@ BEGIN_TEST(testIncrementalRoots)
     // it off the un-prebarriered root, in fact). The pre-barrier on the
     // overwrite of the source location should cause this object to be marked.
     if (!JS_SetProperty(cx, leafOwnerHandle, "obj", JS::UndefinedHandleValue))
         return false;
     MOZ_ASSERT(rt->gc.gcNumber() == currentGCNumber);
     if (!JS_SetProperty(cx, vec[0], "newobj", leafValueHandle))
         return false;
     MOZ_ASSERT(rt->gc.gcNumber() == currentGCNumber);
-    MOZ_ASSERT(leafHandle->asTenured().isMarkedAny());
+    MOZ_ASSERT(leafHandle->asTenured().isMarkedBlack());
 
     // Also take an unmarked object 'leaf2' from the graph and add an
     // additional edge from the root to it. This will not be marked by any
     // pre-barrier, but it is still in the live graph so it will eventually get
     // marked.
     //
     // Note that the root->leaf2 edge will *not* be marked through, since the
     // root is already marked, but that only matters if doing a compacting GC
     // and the compacting GC repeats the whole marking phase to update
     // pointers.
     {
         JS::RootedValue leaf2(cx);
         if (!JS_GetProperty(cx, leafOwnerHandle, "leaf2", &leaf2))
             return false;
         MOZ_ASSERT(rt->gc.gcNumber() == currentGCNumber);
-        MOZ_ASSERT(!leaf2.toObject().asTenured().isMarkedAny());
+        MOZ_ASSERT(!leaf2.toObject().asTenured().isMarkedBlack());
         if (!JS_SetProperty(cx, vec[0], "leafcopy", leaf2))
             return false;
         MOZ_ASSERT(rt->gc.gcNumber() == currentGCNumber);
-        MOZ_ASSERT(!leaf2.toObject().asTenured().isMarkedAny());
+        MOZ_ASSERT(!leaf2.toObject().asTenured().isMarkedBlack());
     }
 
     // Finish the GC using an unlimited budget.
     auto unlimited = js::SliceBudget::unlimited();
     rt->gc.debugGCSlice(unlimited);
 
     // Access the leaf object to try to trigger a crash if it is dead.
     if (!JS_SetProperty(cx, leafHandle, "toes", JS::UndefinedHandleValue))
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -617,17 +617,17 @@ struct VisitGrayCallbackFunctor {
     GCThingCallback callback_;
     void* closure_;
     VisitGrayCallbackFunctor(GCThingCallback callback, void* closure)
       : callback_(callback), closure_(closure)
     {}
 
     template <class T>
     void operator()(T tp) const {
-        if ((*tp)->isTenured() && (*tp)->asTenured().isMarkedGray())
+        if ((*tp)->isMarkedGray())
             callback_(closure_, JS::GCCellPtr(*tp));
     }
 };
 } // namespace (anonymous)
 
 JS_FRIEND_API(void)
 js::VisitGrayWrapperTargets(Zone* zone, GCThingCallback callback, void* closure)
 {
@@ -1099,20 +1099,23 @@ struct DumpHeapTracer : public JS::Callb
 
     void onChild(const JS::GCCellPtr& thing) override;
 };
 
 static char
 MarkDescriptor(void* thing)
 {
     gc::TenuredCell* cell = gc::TenuredCell::fromPointer(thing);
+    if (cell->isMarkedBlack())
+        return 'B';
+    if (cell->isMarkedGray())
+        return 'G';
     if (cell->isMarkedAny())
-        return cell->isMarkedGray() ? 'G' : 'B';
-    else
-        return cell->isMarkedGray() ? 'X' : 'W';
+        return 'X';
+    return 'W';
 }
 
 static void
 DumpHeapVisitZone(JSRuntime* rt, void* data, Zone* zone)
 {
     DumpHeapTracer* dtrc = static_cast<DumpHeapTracer*>(data);
     fprintf(dtrc->output, "# zone %p\n", (void*)zone);
 }
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2042,17 +2042,17 @@ RelocateArena(Arena* arena, SliceBudget&
         sliceBudget.step();
     }
 
 #ifdef DEBUG
     for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) {
         TenuredCell* src = i.getCell();
         MOZ_ASSERT(RelocationOverlay::isCellForwarded(src));
         TenuredCell* dest = Forwarded(src);
-        MOZ_ASSERT(src->isMarkedAny() == dest->isMarkedAny());
+        MOZ_ASSERT(src->isMarkedBlack() == dest->isMarkedBlack());
         MOZ_ASSERT(src->isMarkedGray() == dest->isMarkedGray());
     }
 #endif
 }
 
 static inline bool
 ShouldProtectRelocatedArenas(JS::gcreason::Reason reason)
 {
@@ -4587,17 +4587,17 @@ void
 JSCompartment::findOutgoingEdges(ZoneComponentFinder& finder)
 {
     for (js::WrapperMap::Enum e(crossCompartmentWrappers); !e.empty(); e.popFront()) {
         CrossCompartmentKey& key = e.front().mutableKey();
         MOZ_ASSERT(!key.is<JSString*>());
         bool needsEdge = true;
         if (key.is<JSObject*>()) {
             TenuredCell& other = key.as<JSObject*>()->asTenured();
-            needsEdge = !other.isMarkedAny() || other.isMarkedGray();
+            needsEdge = !other.isMarkedBlack();
         }
         key.applyToWrapped(AddOutgoingEdgeFunctor(needsEdge, finder));
     }
 }
 
 void
 Zone::findOutgoingEdges(ZoneComponentFinder& finder)
 {
@@ -6900,17 +6900,17 @@ GCRuntime::maybeDoCycleCollection()
     const static double ExcessiveGrayCompartments = 0.8;
     const static size_t LimitGrayCompartments = 200;
 
     size_t compartmentsTotal = 0;
     size_t compartmentsGray = 0;
     for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) {
         ++compartmentsTotal;
         GlobalObject* global = c->unsafeUnbarrieredMaybeGlobal();
-        if (global && global->asTenured().isMarkedGray())
+        if (global && global->isMarkedGray())
             ++compartmentsGray;
     }
     double grayFraction = double(compartmentsGray) / double(compartmentsTotal);
     if (grayFraction > ExcessiveGrayCompartments || compartmentsGray > LimitGrayCompartments)
         callDoCycleCollectionCallback(rt->activeContextFromOwnThread());
 }
 
 void
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -3481,17 +3481,17 @@ js::detail::CopyScript(JSContext* cx, Ha
     if (src->treatAsRunOnce() && !src->functionNonDelazifying()) {
         JS_ReportErrorASCII(cx, "No cloning toplevel run-once scripts");
         return false;
     }
 
     /* NB: Keep this in sync with XDRScript. */
 
     /* Some embeddings are not careful to use ExposeObjectToActiveJS as needed. */
-    MOZ_ASSERT(!src->sourceObject()->asTenured().isMarkedGray());
+    MOZ_ASSERT(!src->sourceObject()->isMarkedGray());
 
     uint32_t nconsts   = src->hasConsts()   ? src->consts()->length   : 0;
     uint32_t nobjects  = src->hasObjects()  ? src->objects()->length  : 0;
     uint32_t nscopes   = src->scopes()->length;
     uint32_t ntrynotes = src->hasTrynotes() ? src->trynotes()->length : 0;
     uint32_t nscopenotes = src->hasScopeNotes() ? src->scopeNotes()->length : 0;
     uint32_t nyieldoffsets = src->hasYieldAndAwaitOffsets() ? src->yieldAndAwaitOffsets().length() : 0;
 
--- a/js/src/jsweakmap.cpp
+++ b/js/src/jsweakmap.cpp
@@ -134,17 +134,17 @@ ObjectValueMap::findZoneEdges()
     /*
      * For unmarked weakmap keys with delegates in a different zone, add a zone
      * edge to ensure that the delegate zone finishes marking before the key
      * zone.
      */
     JS::AutoSuppressGCAnalysis nogc;
     for (Range r = all(); !r.empty(); r.popFront()) {
         JSObject* key = r.front().key();
-        if (key->asTenured().isMarkedAny() && !key->asTenured().isMarkedGray())
+        if (key->asTenured().isMarkedBlack())
             continue;
         JSObject* delegate = getDelegate(key);
         if (!delegate)
             continue;
         Zone* delegateZone = delegate->zone();
         if (delegateZone == zone() || !delegateZone->isGCMarking())
             continue;
         if (!delegateZone->gcSweepGroupEdges().put(key->zone()))
--- a/js/src/proxy/Wrapper.cpp
+++ b/js/src/proxy/Wrapper.cpp
@@ -336,17 +336,17 @@ Wrapper::wrappedObject(JSObject* wrapper
     MOZ_ASSERT(wrapper->is<WrapperObject>());
     JSObject* target = wrapper->as<ProxyObject>().target();
 
     // Eagerly unmark gray wrapper targets so we can assert that we don't create
     // black to gray edges. An incremental GC will eventually mark the targets
     // of black wrappers black but while it is in progress we can observe gray
     // targets. Expose rather than returning a gray object in this case.
     if (target) {
-        if (wrapper->isMarkedAny() && !wrapper->isMarkedGray())
+        if (wrapper->isMarkedBlack())
             MOZ_ASSERT(JS::ObjectIsNotGray(target));
         if (!wrapper->isMarkedGray())
             JS::ExposeObjectToActiveJS(target);
     }
 
     return target;
 }
 
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -5819,17 +5819,17 @@ GetMarks(JSContext* cx, unsigned argc, V
         const char* color;
         JSObject* obj = observers->get()[i];
         if (!obj) {
             color = "dead";
         } else {
             gc::TenuredCell* cell = &obj->asTenured();
             if (cell->isMarkedGray())
                 color = "gray";
-            else if (cell->isMarkedAny())
+            else if (cell->isMarkedBlack())
                 color = "black";
             else
                 color = "unmarked";
         }
         JSString* s = JS_NewStringCopyZ(cx, color);
         if (!s)
             return false;
         if (!NewbornArrayPush(cx, ret, StringValue(s)))