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 346027 b2caae037c26ad8e4106ef7562523aead73a8a21
parent 346026 e68394fb539542f291d2804d4d08811cef02534a
child 346028 5d5f51eff4380778908063d6fcd9bb9d73df1b11
push id38387
push usercbook@mozilla.com
push dateMon, 06 Mar 2017 10:11:11 +0000
treeherderautoland@937f89775395 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1343986
milestone54.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 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)