Bug 1343986 - Fix setting of gray bits valid flag after GC r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Sat, 04 Mar 2017 08:59:42 +0000
changeset 375037 b2caae037c26ad8e4106ef7562523aead73a8a21
parent 375036 e68394fb539542f291d2804d4d08811cef02534a
child 375038 5d5f51eff4380778908063d6fcd9bb9d73df1b11
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1343986
milestone54.0a1
Bug 1343986 - Fix setting of gray bits valid flag after GC r=sfink
js/src/gc/GCRuntime.h
js/src/gc/Marking.cpp
js/src/jsgc.cpp
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -982,16 +982,17 @@ class GCRuntime
     MOZ_MUST_USE bool findZoneEdgesForWeakMaps();
     void getNextZoneGroup();
     void endMarkingZoneGroup();
     void beginSweepingZoneGroup(AutoLockForExclusiveAccess& lock);
     bool shouldReleaseObservedTypes();
     void endSweepingZoneGroup();
     IncrementalProgress sweepPhase(SliceBudget& sliceBudget, AutoLockForExclusiveAccess& lock);
     void endSweepPhase(bool lastGC, AutoLockForExclusiveAccess& lock);
+    bool allCCVisibleZonesWereCollected() const;
     void sweepZones(FreeOp* fop, ZoneGroup* group, bool lastGC);
     void sweepZoneGroups(FreeOp* fop, bool destroyingRuntime);
     void decommitAllWithoutUnlocking(const AutoLockGC& lock);
     void startDecommit();
     void queueZonesForBackgroundSweep(ZoneList& zones);
     void sweepBackgroundThings(ZoneList& zones, LifoAlloc& freeBlocks);
     void assertBackgroundSweepingFinished();
     bool shouldCompact();
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -216,16 +216,19 @@ js::CheckTracedThing(JSTracer* trc, T* t
     Zone* zone = thing->zoneFromAnyThread();
     JSRuntime* rt = trc->runtime();
 
     MOZ_ASSERT_IF(!IsMovingTracer(trc), CurrentThreadCanAccessZone(zone));
     MOZ_ASSERT_IF(!IsMovingTracer(trc), CurrentThreadCanAccessRuntime(rt));
 
     MOZ_ASSERT(zone->runtimeFromAnyThread() == trc->runtime());
 
+    // It shouldn't be possible to trace into zones used by helper threads.
+    MOZ_ASSERT(!zone->usedByHelperThread());
+
     MOZ_ASSERT(thing->isAligned());
     MOZ_ASSERT(MapTypeToTraceKind<typename mozilla::RemovePointer<T>::Type>::kind ==
                thing->getTraceKind());
 
     /*
      * Do not check IsMarkingTracer directly -- it should only be used in paths
      * where we cannot be the gray buffering tracer.
      */
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -5435,16 +5435,46 @@ GCRuntime::sweepPhase(SliceBudget& slice
         if (!currentZoneGroup)
             return Finished;
 
         endMarkingZoneGroup();
         beginSweepingZoneGroup(lock);
     }
 }
 
+bool
+GCRuntime::allCCVisibleZonesWereCollected() const
+{
+    // Calculate whether the gray marking state is now valid.
+    //
+    // The gray bits change from invalid to valid if we finished a full GC from
+    // the point of view of the cycle collector. We ignore the following:
+    //
+    //  - Helper thread zones, as these are not reachable from the main heap.
+    //  - The atoms zone, since strings and symbols are never marked gray.
+    //  - Empty zones.
+    //
+    // These exceptions ensure that when the CC requests a full GC the gray mark
+    // state ends up valid even it we don't collect all of the zones.
+
+    if (isFull)
+        return true;
+
+    for (ZonesIter zone(rt, SkipAtoms); !zone.done(); zone.next()) {
+        if (!zone->isCollecting() &&
+            !zone->usedByHelperThread() &&
+            !zone->arenas.arenaListsAreEmpty())
+        {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void
 GCRuntime::endSweepPhase(bool destroyingRuntime, AutoLockForExclusiveAccess& lock)
 {
     AutoSetThreadIsSweeping threadIsSweeping;
 
     gcstats::AutoPhase ap(stats(), gcstats::PHASE_SWEEP);
     FreeOp fop(rt);
 
@@ -5480,18 +5510,17 @@ GCRuntime::endSweepPhase(bool destroying
             rt->jitRuntime()->backedgeExecAlloc().purge();
         }
     }
 
     {
         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)
+        if (allCCVisibleZonesWereCollected())
             grayBitsValid = true;
     }
 
     finishMarkingValidation();
 
 #ifdef DEBUG
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
         for (auto i : AllAllocKinds()) {
@@ -7829,17 +7858,19 @@ js::gc::detail::CellIsMarkedGrayIfKnown(
     // call this while parsing, and they are not allowed to inspect the
     // runtime's incremental state. The objects being operated on are not able
     // to be collected and will not be marked any color.
 
     if (!CanCheckGrayBits(cell))
         return false;
 
     auto tc = &cell->asTenured();
-    auto rt = tc->runtimeFromAnyThread();
+    MOZ_ASSERT(!tc->zoneFromAnyThread()->usedByHelperThread());
+
+    auto rt = tc->runtimeFromActiveCooperatingThread();
     if (rt->gc.isIncrementalGCInProgress() && !tc->zone()->wasGCStarted())
         return false;
 
     return detail::CellIsMarkedGray(tc);
 }
 
 #ifdef DEBUG
 JS_PUBLIC_API(bool)