Bug 1323241 - Only report things as gray if gray marking state is valid. r=sfink, a=jcristau
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 06 Jan 2017 11:23:21 +0000
changeset 354162 4ed3524d4d665206b23ad94bd984c1ef6a6296e5
parent 354161 ec7ea9c6e94a6c640b9512f43473508dbfea25da
child 354163 b888fcaaa83975b0601b718d7628de0b820deeeb
push id6892
push userryanvm@gmail.com
push dateTue, 14 Feb 2017 16:13:02 +0000
treeherdermozilla-esr52@35e191e72900 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, jcristau
bugs1323241
milestone52.0
Bug 1323241 - Only report things as gray if gray marking state is valid. r=sfink, a=jcristau
js/public/GCAPI.h
js/public/HeapAPI.h
js/public/RootingAPI.h
js/src/gc/GCRuntime.h
js/src/gc/Marking.cpp
js/src/jsapi-tests/testGCCellPtr.cpp
js/src/jsfriendapi.cpp
js/src/jsgc.cpp
js/src/jspubtd.h
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -638,22 +638,22 @@ ExposeGCThingToActiveJS(JS::GCCellPtr th
     if (IsInsideNursery(thing.asCell()))
         return;
 
     // There's nothing to do for permanent GC things that might be owned by
     // another runtime.
     if (thing.mayBeOwnedByOtherRuntime())
         return;
 
-    JS::shadow::Runtime* rt = detail::GetGCThingRuntime(thing.unsafeAsUIntPtr());
+    JS::shadow::Runtime* rt = detail::GetCellRuntime(thing.asCell());
     MOZ_DIAGNOSTIC_ASSERT(rt->allowGCBarriers());
 
     if (IsIncrementalBarrierNeededOnTenuredGCThing(rt, thing))
         JS::IncrementalReferenceBarrier(thing);
-    else if (JS::GCThingIsMarkedGray(thing))
+    else if (!thing.mayBeOwnedByOtherRuntime() && js::gc::detail::CellIsMarkedGray(thing.asCell()))
         JS::UnmarkGrayGCThingRecursively(thing);
 }
 
 static MOZ_ALWAYS_INLINE void
 MarkGCThingAsLive(JSRuntime* aRt, JS::GCCellPtr thing)
 {
     // Any object in the nursery will not be freed during any GC running at that
     // time.
--- a/js/public/HeapAPI.h
+++ b/js/public/HeapAPI.h
@@ -275,24 +275,16 @@ namespace detail {
 static MOZ_ALWAYS_INLINE uintptr_t*
 GetGCThingMarkBitmap(const uintptr_t addr)
 {
     MOZ_ASSERT(addr);
     const uintptr_t bmap_addr = (addr & ~ChunkMask) | ChunkMarkBitmapOffset;
     return reinterpret_cast<uintptr_t*>(bmap_addr);
 }
 
-static MOZ_ALWAYS_INLINE JS::shadow::Runtime*
-GetGCThingRuntime(const uintptr_t addr)
-{
-    MOZ_ASSERT(addr);
-    const uintptr_t rt_addr = (addr & ~ChunkMask) | ChunkRuntimeOffset;
-    return *reinterpret_cast<JS::shadow::Runtime**>(rt_addr);
-}
-
 static MOZ_ALWAYS_INLINE void
 GetGCThingMarkWordAndMask(const uintptr_t addr, uint32_t color,
                           uintptr_t** wordp, uintptr_t* maskp)
 {
     MOZ_ASSERT(addr);
     const size_t bit = (addr & js::gc::ChunkMask) / js::gc::CellSize + color;
     MOZ_ASSERT(bit < js::gc::ChunkMarkBitmapBits);
     uintptr_t* bitmap = GetGCThingMarkBitmap(addr);
@@ -305,26 +297,45 @@ static MOZ_ALWAYS_INLINE JS::Zone*
 GetGCThingZone(const uintptr_t addr)
 {
     MOZ_ASSERT(addr);
     const uintptr_t zone_addr = (addr & ~ArenaMask) | ArenaZoneOffset;
     return *reinterpret_cast<JS::Zone**>(zone_addr);
 
 }
 
+static MOZ_ALWAYS_INLINE JS::shadow::Runtime*
+GetCellRuntime(const Cell* cell)
+{
+    MOZ_ASSERT(cell);
+    const uintptr_t addr = uintptr_t(cell);
+    const uintptr_t rt_addr = (addr & ~ChunkMask) | ChunkRuntimeOffset;
+    return *reinterpret_cast<JS::shadow::Runtime**>(rt_addr);
+}
+
 static MOZ_ALWAYS_INLINE bool
 CellIsMarkedGray(const Cell* cell)
 {
     MOZ_ASSERT(cell);
-    MOZ_ASSERT(!js::gc::IsInsideNursery(cell));
+    if (js::gc::IsInsideNursery(cell))
+        return false;
+
     uintptr_t* word, mask;
     js::gc::detail::GetGCThingMarkWordAndMask(uintptr_t(cell), js::gc::GRAY, &word, &mask);
     return *word & mask;
 }
 
+static MOZ_ALWAYS_INLINE bool
+CellIsMarkedGrayIfKnown(const Cell* cell)
+{
+    MOZ_ASSERT(cell);
+    auto rt = js::gc::detail::GetCellRuntime(cell);
+    return rt->areGCGrayBitsValid() && CellIsMarkedGray(cell);
+}
+
 } /* namespace detail */
 
 MOZ_ALWAYS_INLINE bool
 IsInsideNursery(const js::gc::Cell* cell)
 {
     if (!cell)
         return false;
     uintptr_t addr = uintptr_t(cell);
@@ -354,21 +365,19 @@ GetStringZone(JSString* str)
 }
 
 extern JS_PUBLIC_API(Zone*)
 GetObjectZone(JSObject* obj);
 
 static MOZ_ALWAYS_INLINE bool
 GCThingIsMarkedGray(GCCellPtr thing)
 {
-    if (js::gc::IsInsideNursery(thing.asCell()))
-        return false;
     if (thing.mayBeOwnedByOtherRuntime())
         return false;
-    return js::gc::detail::CellIsMarkedGray(thing.asCell());
+    return js::gc::detail::CellIsMarkedGrayIfKnown(thing.asCell());
 }
 
 extern JS_PUBLIC_API(JS::TraceKind)
 GCThingTraceKind(void* thing);
 
 } /* namespace JS */
 
 namespace js {
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -306,36 +306,31 @@ static MOZ_ALWAYS_INLINE bool
 ObjectIsTenured(const Heap<JSObject*>& obj)
 {
     return ObjectIsTenured(obj.unbarrieredGet());
 }
 
 static MOZ_ALWAYS_INLINE bool
 ObjectIsMarkedGray(JSObject* obj)
 {
-    /*
-     * GC things residing in the nursery cannot be gray: they have no mark bits.
-     * All live objects in the nursery are moved to tenured at the beginning of
-     * each GC slice, so the gray marker never sees nursery things.
-     */
-    if (js::gc::IsInsideNursery(reinterpret_cast<js::gc::Cell*>(obj)))
-        return false;
-    return js::gc::detail::CellIsMarkedGray(reinterpret_cast<js::gc::Cell*>(obj));
+    auto cell = reinterpret_cast<js::gc::Cell*>(obj);
+    return js::gc::detail::CellIsMarkedGrayIfKnown(cell);
 }
 
 static MOZ_ALWAYS_INLINE bool
 ObjectIsMarkedGray(const JS::Heap<JSObject*>& obj)
 {
     return ObjectIsMarkedGray(obj.unbarrieredGet());
 }
 
 static MOZ_ALWAYS_INLINE bool
 ScriptIsMarkedGray(JSScript* script)
 {
-    return js::gc::detail::CellIsMarkedGray(reinterpret_cast<js::gc::Cell*>(script));
+    auto cell = reinterpret_cast<js::gc::Cell*>(script);
+    return js::gc::detail::CellIsMarkedGrayIfKnown(cell);
 }
 
 static MOZ_ALWAYS_INLINE bool
 ScriptIsMarkedGray(const Heap<JSScript*>& script)
 {
     return ScriptIsMarkedGray(script.unbarrieredGet());
 }
 
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -813,19 +813,16 @@ class GCRuntime
     void incMajorGcNumber() { ++majorGCNumber; ++number; }
 
     int64_t defaultSliceBudget() const { return defaultTimeBudget_; }
 
     bool isIncrementalGc() const { return isIncremental; }
     bool isFullGc() const { return isFull; }
     bool isCompactingGc() const { return isCompacting; }
 
-    bool areGrayBitsValid() const { return grayBitsValid; }
-    void setGrayBitsInvalid() { grayBitsValid = false; }
-
     bool minorGCRequested() const { return minorGCTriggerReason != JS::gcreason::NO_REASON; }
     bool majorGCRequested() const { return majorGCTriggerReason != JS::gcreason::NO_REASON; }
     bool isGcNeeded() { return minorGCRequested() || majorGCRequested(); }
 
     bool fullGCForAtomsRequested() const { return fullGCForAtomsRequested_; }
 
     double computeHeapGrowthFactor(size_t lastBytes);
     size_t computeTriggerBytes(double growthFactor, size_t lastBytes);
@@ -1105,22 +1102,16 @@ class GCRuntime
     void resetBufferedGrayRoots() const;
 
     // Reset the gray buffering state to Unused.
     void clearBufferedGrayRoots() {
         grayBufferState = GrayBufferState::Unused;
         resetBufferedGrayRoots();
     }
 
-    /*
-     * The gray bits can become invalid if UnmarkGray overflows the stack. A
-     * full GC will reset this bit, since it fills in all the gray bits.
-     */
-    bool grayBitsValid;
-
     mozilla::Atomic<JS::gcreason::Reason, mozilla::Relaxed> majorGCTriggerReason;
 
     JS::gcreason::Reason minorGCTriggerReason;
 
     /* Perform full GC if rt->keepAtoms() becomes false. */
     bool fullGCForAtomsRequested_;
 
     /* Incremented at the start of every minor GC. */
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2910,17 +2910,17 @@ UnmarkGrayTracer::onChild(const JS::GCCe
 {
     int stackDummy;
     JSContext* cx = runtime()->contextFromMainThread();
     if (!JS_CHECK_STACK_SIZE(cx->nativeStackLimit[StackForSystemCode], &stackDummy)) {
         /*
          * If we run out of stack, we take a more drastic measure: require that
          * we GC again before the next CC.
          */
-        runtime()->gc.setGrayBitsInvalid();
+        runtime()->setGCGrayBitsValid(false);
         return;
     }
 
     Cell* cell = thing.asCell();
 
     // Cells in the nursery cannot be gray, and therefore must necessarily point
     // to only black edges.
     if (!cell->isTenured()) {
--- a/js/src/jsapi-tests/testGCCellPtr.cpp
+++ b/js/src/jsapi-tests/testGCCellPtr.cpp
@@ -49,13 +49,13 @@ BEGIN_TEST(testGCCellPtr)
     JS::GCCellPtr objcell(obj.get());
     JS::GCCellPtr scriptcell = JS::GCCellPtr(script.get());
     CHECK(GivesAndTakesCells(objcell));
     CHECK(GivesAndTakesCells(scriptcell));
 
     JS::GCCellPtr copy = objcell;
     CHECK(copy == objcell);
 
-    CHECK(js::gc::detail::GetGCThingRuntime(scriptcell.unsafeAsUIntPtr()) == cx->runtime());
+    CHECK(js::gc::detail::GetCellRuntime(scriptcell.asCell()) == cx->runtime());
 
     return true;
 }
 END_TEST(testGCCellPtr)
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -585,17 +585,17 @@ js::TraceWeakMaps(WeakMapTracer* trc)
 {
     WeakMapBase::traceAllMappings(trc);
     WatchpointMap::traceAll(trc);
 }
 
 extern JS_FRIEND_API(bool)
 js::AreGCGrayBitsValid(JSContext* cx)
 {
-    return cx->gc.areGrayBitsValid();
+    return cx->areGCGrayBitsValid();
 }
 
 JS_FRIEND_API(bool)
 js::ZoneGlobalsAreAllGray(JS::Zone* zone)
 {
     for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) {
         JSObject* obj = comp->unsafeUnbarrieredMaybeGlobal();
         if (!obj || !JS::ObjectIsMarkedGray(obj))
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -813,17 +813,16 @@ GCRuntime::GCRuntime(JSRuntime* rt) :
     numArenasFreeCommitted(0),
     verifyPreData(nullptr),
     chunkAllocationSinceLastGC(false),
     lastGCTime(PRMJ_Now()),
     mode(JSGC_MODE_INCREMENTAL),
     numActiveZoneIters(0),
     cleanUpEverything(false),
     grayBufferState(GCRuntime::GrayBufferState::Unused),
-    grayBitsValid(false),
     majorGCTriggerReason(JS::gcreason::NO_REASON),
     minorGCTriggerReason(JS::gcreason::NO_REASON),
     fullGCForAtomsRequested_(false),
     minorGCNumber(0),
     majorGCNumber(0),
     jitReleaseNumber(0),
     number(0),
     startNumber(0),
@@ -5438,17 +5437,17 @@ GCRuntime::endSweepPhase(bool destroying
     }
 
     {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_FINALIZE_END);
         callFinalizeCallbacks(&fop, JSFINALIZE_COLLECTION_END);
 
         /* If we finished a full GC, then the gray bits are correct. */
         if (isFull)
-            grayBitsValid = true;
+            rt->setGCGrayBitsValid(true);
     }
 
     finishMarkingValidation();
 
 #ifdef DEBUG
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
         for (auto i : AllAllocKinds()) {
             MOZ_ASSERT_IF(!IsBackgroundFinalized(i) ||
--- a/js/src/jspubtd.h
+++ b/js/src/jspubtd.h
@@ -181,36 +181,44 @@ struct Runtime
 
     // In some cases, invoking GC barriers (incremental or otherwise) will break
     // things. These barriers assert if this flag is set.
     bool allowGCBarriers_;
     friend class JS::AutoAssertOnBarrier;
 
     js::gc::StoreBuffer* gcStoreBufferPtr_;
 
+    // The gray bits can become invalid if UnmarkGray overflows the stack. A
+    // full GC will reset this bit, since it fills in all the gray bits.
+    bool gcGrayBitsValid_;
+
   public:
     Runtime()
       : heapState_(JS::HeapState::Idle)
       , allowGCBarriers_(true)
       , gcStoreBufferPtr_(nullptr)
+      , gcGrayBitsValid_(false)
     {}
 
     bool isHeapBusy() const { return heapState() != JS::HeapState::Idle; }
     bool isHeapTracing() const { return heapState() == JS::HeapState::Tracing; }
     bool isHeapMajorCollecting() const { return heapState() == JS::HeapState::MajorCollecting; }
     bool isHeapMinorCollecting() const { return heapState() == JS::HeapState::MinorCollecting; }
     bool isHeapCollecting() const { return isHeapMinorCollecting() || isHeapMajorCollecting(); }
     bool isCycleCollecting() const {
         return heapState() == JS::HeapState::CycleCollecting;
     }
 
     bool allowGCBarriers() const { return allowGCBarriers_; }
 
     js::gc::StoreBuffer* gcStoreBufferPtr() { return gcStoreBufferPtr_; }
 
+    bool areGCGrayBitsValid() const { return gcGrayBitsValid_; }
+    void setGCGrayBitsValid(bool valid) { gcGrayBitsValid_ = valid; }
+
     const JSRuntime* asRuntime() const {
         return reinterpret_cast<const JSRuntime*>(this);
     }
 
     static JS::shadow::Runtime* asShadowRuntime(JSRuntime* rt) {
         return reinterpret_cast<JS::shadow::Runtime*>(rt);
     }