Bug 1463462 - Delay gray marking assertions when we are doing incremental gray marking r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 06 Dec 2018 16:28:14 -0500
changeset 450672 42f073dedf5fd708e118833b4ddf63a19907485a
parent 450671 1544326ba29a387f1240415af38da7a33f5083ef
child 450673 d5542a14b416d3d7e4e846368ceb37d6714827aa
push id110516
push userjcoppeard@mozilla.com
push dateFri, 14 Dec 2018 14:22:31 +0000
treeherdermozilla-inbound@fd4d12eb1b97 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1463462
milestone66.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 1463462 - Delay gray marking assertions when we are doing incremental gray marking r=sfink
js/public/RootingAPI.h
js/src/gc/GC.cpp
js/src/gc/GCRuntime.h
js/src/vm/JSScript.cpp
js/src/vm/NativeObject.h
js/src/vm/ProxyObject.cpp
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -365,17 +365,17 @@ static MOZ_ALWAYS_INLINE bool ObjectIsMa
 }
 
 // The following *IsNotGray functions take account of the eventual
 // gray marking state at the end of any ongoing incremental GC by
 // delaying the checks if necessary.
 
 #ifdef DEBUG
 
-inline void AssertCellIsNotGray(js::gc::Cell* maybeCell) {
+inline void AssertCellIsNotGray(const js::gc::Cell* maybeCell) {
   if (maybeCell) {
     js::gc::detail::AssertCellIsNotGray(maybeCell);
   }
 }
 
 inline void AssertObjectIsNotGray(JSObject* maybeObj) {
   AssertCellIsNotGray(reinterpret_cast<js::gc::Cell*>(maybeObj));
 }
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -5254,16 +5254,18 @@ static inline void MaybeCheckWeakMapMark
 IncrementalProgress GCRuntime::markGrayReferencesInCurrentGroup(
     FreeOp* fop, SliceBudget& budget) {
   MOZ_ASSERT(marker.markColor() == MarkColor::Black);
 
   if (hasMarkedGrayRoots) {
     return Finished;
   }
 
+  MOZ_ASSERT(cellsToAssertNotGray.ref().empty());
+
   gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP_MARK);
 
   // Mark any incoming gray pointers from previously swept compartments that
   // have subsequently been marked black. This can occur when gray cells
   // become black by the action of UnmarkGray.
   markIncomingCrossCompartmentPointers(MarkColor::Black);
   drainMarkStack();
 
@@ -5616,16 +5618,23 @@ IncrementalProgress GCRuntime::beginSwee
 
 #ifdef DEBUG
     zone->gcLastSweepGroupIndex = sweepGroupIndex;
 #endif
   }
 
   validateIncrementalMarking();
 
+#ifdef DEBUG
+  for (auto cell : cellsToAssertNotGray.ref()) {
+    JS::AssertCellIsNotGray(cell);
+  }
+  cellsToAssertNotGray.ref().clearAndFree();
+#endif
+
   {
     AutoPhase ap(stats(), PhaseKind::FINALIZE_START);
     callFinalizeCallbacks(fop, JSFINALIZE_GROUP_PREPARE);
     {
       AutoPhase ap2(stats(), PhaseKind::WEAK_ZONES_CALLBACK);
       callWeakPointerZonesCallbacks();
     }
     {
@@ -6693,16 +6702,17 @@ void GCRuntime::finishCollection() {
     }
 
     MOZ_ASSERT(!zone->wasGCStarted());
     MOZ_ASSERT(!zone->needsIncrementalBarrier());
     MOZ_ASSERT(!zone->isOnList());
   }
 
   MOZ_ASSERT(zonesToMaybeCompact.ref().isEmpty());
+  MOZ_ASSERT(cellsToAssertNotGray.ref().empty());
 
   lastGCTime = currentTime;
 }
 
 static const char* HeapStateToLabel(JS::HeapState heapState) {
   switch (heapState) {
     case JS::HeapState::MinorCollecting:
       return "js::Nursery::collect";
@@ -8938,16 +8948,28 @@ JS_PUBLIC_API void js::gc::detail::Asser
     return;
   }
 
   // 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 (tc->zone()->isGCMarkingBlackAndGray()) {
+    // We are doing gray marking in the cell's zone. Even if the cell is
+    // currently marked gray it may eventually be marked black. Delay the check
+    // until we finish gray marking.
+    JSRuntime* rt = tc->zone()->runtimeFromMainThread();
+    AutoEnterOOMUnsafeRegion oomUnsafe;
+    if (!rt->gc.cellsToAssertNotGray.ref().append(cell)) {
+      oomUnsafe.crash("Can't append to delayed gray checks list");
+    }
+    return;
+  }
+
   MOZ_ASSERT(!detail::CellIsMarkedGray(tc));
 }
 
 extern JS_PUBLIC_API bool js::gc::detail::ObjectIsMarkedBlack(
     const JSObject* obj) {
   return obj->isMarkedBlack();
 }
 
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -891,16 +891,25 @@ class GCRuntime {
   MainThreadData<UniquePtr<SweepAction<GCRuntime*, FreeOp*, SliceBudget&>>>
       sweepActions;
   MainThreadOrGCTaskData<JS::Zone*> sweepZone;
   MainThreadData<mozilla::Maybe<AtomsTable::SweepIterator>> maybeAtomsToSweep;
   MainThreadOrGCTaskData<JS::detail::WeakCacheBase*> sweepCache;
   MainThreadData<bool> hasMarkedGrayRoots;
   MainThreadData<bool> abortSweepAfterCurrentGroup;
 
+#ifdef DEBUG
+  // During gray marking, delay AssertCellIsNotGray checks by
+  // recording the cell pointers here and checking after marking has
+  // finished.
+  MainThreadData<Vector<const Cell*, 0, SystemAllocPolicy>>
+      cellsToAssertNotGray;
+  friend void js::gc::detail::AssertCellIsNotGray(const Cell*);
+#endif
+
   friend class SweepGroupsIter;
   friend class WeakCacheSweepIterator;
 
   /*
    * Incremental compacting state.
    */
   MainThreadData<bool> startedCompacting;
   MainThreadData<ZoneList> zonesToMaybeCompact;
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -3825,17 +3825,17 @@ bool js::detail::CopyScript(JSContext* c
   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()->isMarkedGray());
+  JS::AssertObjectIsNotGray(src->sourceObject());
 
   uint32_t nscopes = src->scopes().size();
   uint32_t nconsts = src->hasConsts() ? src->consts().size() : 0;
   uint32_t nobjects = src->hasObjects() ? src->objects().size() : 0;
 
   /* Script data */
 
   size_t size = src->dataSize();
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -1460,17 +1460,21 @@ class NativeObject : public ShapedObject
   void* getPrivate() const { return privateRef(numFixedSlots()); }
   void setPrivate(void* data) {
     void** pprivate = &privateRef(numFixedSlots());
     privateWriteBarrierPre(pprivate);
     *pprivate = data;
   }
 
   void setPrivateGCThing(gc::Cell* cell) {
-    MOZ_ASSERT_IF(IsMarkedBlack(this), !cell->isMarkedGray());
+#ifdef DEBUG
+    if (IsMarkedBlack(this)) {
+      JS::AssertCellIsNotGray(cell);
+    }
+#endif
     void** pprivate = &privateRef(numFixedSlots());
     privateWriteBarrierPre(pprivate);
     *pprivate = reinterpret_cast<void*>(cell);
     privateWriteBarrierPost(pprivate);
   }
 
   void setPrivateUnbarriered(void* data) {
     void** pprivate = &privateRef(numFixedSlots());
--- a/js/src/vm/ProxyObject.cpp
+++ b/js/src/vm/ProxyObject.cpp
@@ -48,23 +48,26 @@ static gc::AllocKind GetProxyGCObjectKin
 /* static */ ProxyObject* ProxyObject::New(JSContext* cx,
                                            const BaseProxyHandler* handler,
                                            HandleValue priv, TaggedProto proto_,
                                            const ProxyOptions& options) {
   Rooted<TaggedProto> proto(cx, proto_);
 
   const Class* clasp = options.clasp();
 
+#ifdef DEBUG
   MOZ_ASSERT(isValidProxyClass(clasp));
   MOZ_ASSERT(clasp->shouldDelayMetadataBuilder());
   MOZ_ASSERT_IF(proto.isObject(),
                 cx->compartment() == proto.toObject()->compartment());
   MOZ_ASSERT(clasp->hasFinalize());
-  MOZ_ASSERT_IF(priv.isGCThing(),
-                !JS::GCThingIsMarkedGray(JS::GCCellPtr(priv)));
+  if (priv.isGCThing()) {
+    JS::AssertCellIsNotGray(priv.toGCThing());
+  }
+#endif
 
   /*
    * Eagerly mark properties unknown for proxies, so we don't try to track
    * their properties and so that we don't need to walk the compartment if
    * their prototype changes later.  But don't do this for DOM proxies,
    * because we want to be able to keep track of them in typesets in useful
    * ways.
    */