Bug 1502946 - Tighten up some gray marking checks r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 31 Oct 2018 13:32:37 +0000
changeset 443707 637be7630f26d1e43ee1e1dbe1060ac8789abd51
parent 443706 960b295f45afb55ad43ac6c68e9bd7bb2d85196a
child 443708 6015a119c1547d1e718517201a973459d3e55a54
child 443753 d71404ed02ef6861773ab39abd70e1977aaeec7b
push id109430
push userjcoppeard@mozilla.com
push dateWed, 31 Oct 2018 13:37:09 +0000
treeherdermozilla-inbound@637be7630f26 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1502946
milestone65.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 1502946 - Tighten up some gray marking checks r=sfink
js/src/gc/GC.cpp
js/src/gc/GCMarker.h
js/src/gc/Marking.cpp
js/src/jsapi-tests/testGCGrayMarking.cpp
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -9684,34 +9684,17 @@ js::gc::detail::CellIsNotGray(const Cell
         return true;
     }
 
     // TODO: I'd like to AssertHeapIsIdle() here, but this ends up getting
     // called during GC and while iterating the heap for memory reporting.
     MOZ_ASSERT(!JS::RuntimeHeapIsCycleCollecting());
 
     auto tc = &cell->asTenured();
-    if (!detail::CellIsMarkedGray(tc)) {
-        return true;
-    }
-
-    // The cell is gray, but may eventually be marked black if we are in an
-    // incremental GC and the cell is reachable by something on the mark stack.
-
-    auto rt = tc->runtimeFromAnyThread();
-    if (!rt->gc.isIncrementalGCInProgress() || tc->zone()->wasGCStarted()) {
-        return false;
-    }
-
-    Zone* sourceZone = rt->gc.marker.stackContainsCrossZonePointerTo(tc);
-    if (sourceZone && sourceZone->wasGCStarted()) {
-        return true;
-    }
-
-    return false;
+    return !detail::CellIsMarkedGray(tc);
 }
 
 extern JS_PUBLIC_API(bool)
 js::gc::detail::ObjectIsMarkedBlack(const JSObject* obj)
 {
     return obj->isMarkedBlack();
 }
 
--- a/js/src/gc/GCMarker.h
+++ b/js/src/gc/GCMarker.h
@@ -303,21 +303,17 @@ class GCMarker : public JSTracer
 
     MOZ_MUST_USE bool markUntilBudgetExhaused(SliceBudget& budget);
 
     void setGCMode(JSGCMode mode) { stack.setGCMode(mode); }
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 #ifdef DEBUG
-
     bool shouldCheckCompartments() { return strictCompartmentChecking; }
-
-    JS::Zone* stackContainsCrossZonePointerTo(const gc::Cell* cell);
-
 #endif
 
     void markEphemeronValues(gc::Cell* markedCell, gc::WeakEntryVector& entry);
 
     static GCMarker* fromTracer(JSTracer* trc) {
         MOZ_ASSERT(trc->isMarkingTracer());
         return static_cast<GCMarker*>(trc);
     }
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2721,55 +2721,16 @@ GCMarker::sizeOfExcludingThis(mozilla::M
 {
     size_t size = stack.sizeOfExcludingThis(mallocSizeOf);
     for (ZonesIter zone(runtime(), WithAtoms); !zone.done(); zone.next()) {
         size += zone->gcGrayRoots().sizeOfExcludingThis(mallocSizeOf);
     }
     return size;
 }
 
-#ifdef DEBUG
-Zone*
-GCMarker::stackContainsCrossZonePointerTo(const Cell* target)
-{
-    MOZ_ASSERT(!JS::RuntimeHeapIsCollecting());
-
-    Zone* targetZone = target->asTenured().zone();
-
-    for (MarkStackIter iter(stack); !iter.done(); iter.next()) {
-        if (iter.peekTag() != MarkStack::ObjectTag) {
-            continue;
-        }
-
-        auto source = iter.peekPtr().as<JSObject>();
-        Zone* sourceZone = source->zone();
-        if (sourceZone == targetZone) {
-            continue;
-        }
-
-        // The private slot of proxy objects might contain a cross-compartment
-        // pointer.
-        if (source->is<ProxyObject>()) {
-            Value value = source->as<ProxyObject>().private_();
-            MOZ_ASSERT_IF(!IsCrossCompartmentWrapper(source),
-                          IsObjectValueInCompartment(value, source->compartment()));
-            if (value.isObject() && &value.toObject() == target) {
-                return sourceZone;
-            }
-        }
-
-        if (Debugger::isDebuggerCrossCompartmentEdge(source, target)) {
-            return sourceZone;
-        }
-    }
-
-    return nullptr;
-}
-#endif // DEBUG
-
 
 /*** Tenuring Tracer *****************************************************************************/
 
 namespace js {
 template <typename T>
 void
 TenuringTracer::traverse(T** tp)
 {
--- a/js/src/jsapi-tests/testGCGrayMarking.cpp
+++ b/js/src/jsapi-tests/testGCGrayMarking.cpp
@@ -501,24 +501,25 @@ TestCCWs()
     cx->runtime()->gc.startDebugGC(GC_NORMAL, budget);
     CHECK(JS::IsIncrementalGCInProgress(cx));
     CHECK(wrapper->zone()->isGCMarkingBlack());
     CHECK(!target->zone()->wasGCStarted());
     CHECK(!IsMarkedBlack(wrapper));
     CHECK(!IsMarkedGray(wrapper));
     CHECK(IsMarkedGray(target));
 
-    // Betweeen GC slices: source marked black by barrier, target is still gray.
-    // ObjectIsMarkedGray() and CheckObjectIsNotMarkedGray() should handle this
-    // case and report that target is not marked gray.
+    // Betweeen GC slices: source marked black by barrier, target is still
+    // gray. Target will be marked gray eventually. ObjectIsMarkedGray() is
+    // conservative and reports that target is not marked gray; ObjectIsNotGray
+    // reports the actual state.
     grayRoots.grayRoot1.get();
     CHECK(IsMarkedBlack(wrapper));
     CHECK(IsMarkedGray(target));
     CHECK(!JS::ObjectIsMarkedGray(target));
-    MOZ_ASSERT(JS::ObjectIsNotGray(target));
+    MOZ_ASSERT(!JS::ObjectIsNotGray(target));
 
     // Final state: source and target are black.
     JS::FinishIncrementalGC(cx, JS::gcreason::API);
     CHECK(IsMarkedBlack(wrapper));
     CHECK(IsMarkedBlack(target));
 
     grayRoots.grayRoot1 = nullptr;
     grayRoots.grayRoot2 = nullptr;