Bug 1249367 - Make background finalization a GC phase (and clean up Zones properly); r=jonco
authorTerrence Cole <terrence@mozilla.com>
Mon, 22 Feb 2016 08:51:50 -0800
changeset 321792 d8da9bc56468f58be505e61e72e78bff1d2b4b61
parent 321791 5a1a5726b179155823013cd617738540753eb4de
child 321793 2b0aa1cffeeaabb524b9d2936321af18c7445fbc
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1249367
milestone47.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 1249367 - Make background finalization a GC phase (and clean up Zones properly); r=jonco
js/src/builtin/TestingFunctions.cpp
js/src/gc/GCRuntime.h
js/src/jit-test/tests/gc/incremental-state.js
js/src/jsapi-tests/testGCFinalizeCallback.cpp
js/src/jsapi-tests/testWeakMap.cpp
js/src/jsgc.cpp
js/src/jsgc.h
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -789,16 +789,18 @@ GCState(JSContext* cx, unsigned argc, Va
     const char* state;
     gc::State globalState = cx->runtime()->gc.state();
     if (globalState == gc::NO_INCREMENTAL)
         state = "none";
     else if (globalState == gc::MARK)
         state = "mark";
     else if (globalState == gc::SWEEP)
         state = "sweep";
+    else if (globalState == gc::FINALIZE)
+        state = "finalize";
     else if (globalState == gc::COMPACT)
         state = "compact";
     else
         MOZ_CRASH("Unobserveable global GC state");
 
     JSString* str = JS_NewStringCopyZ(cx, state);
     if (!str)
         return false;
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -944,17 +944,17 @@ class GCRuntime
     void sweepZones(FreeOp* fop, bool lastGC);
     void decommitAllWithoutUnlocking(const AutoLockGC& lock);
     void decommitArenas(AutoLockGC& lock);
     void expireChunksAndArenas(bool shouldShrink, AutoLockGC& lock);
     void queueZonesForBackgroundSweep(ZoneList& zones);
     void sweepBackgroundThings(ZoneList& zones, LifoAlloc& freeBlocks, ThreadType threadType);
     void assertBackgroundSweepingFinished();
     bool shouldCompact();
-    IncrementalProgress beginCompactPhase();
+    void beginCompactPhase();
     IncrementalProgress compactPhase(JS::gcreason::Reason reason, SliceBudget& sliceBudget);
     void endCompactPhase(JS::gcreason::Reason reason);
     void sweepTypesAfterCompacting(Zone* zone);
     void sweepZoneAfterCompacting(Zone* zone);
     bool relocateArenas(Zone* zone, JS::gcreason::Reason reason, ArenaHeader*& relocatedListOut,
                         SliceBudget& sliceBudget);
     void updateAllCellPointersParallel(MovingTracer* trc, Zone* zone);
     void updateAllCellPointersSerial(MovingTracer* trc, Zone* zone);
--- a/js/src/jit-test/tests/gc/incremental-state.js
+++ b/js/src/jit-test/tests/gc/incremental-state.js
@@ -1,49 +1,58 @@
-/*
- * Test expected state changes during collection.
- */
-
+// Test expected state changes during collection.
 if (!("gcstate" in this))
     quit();
 
 gczeal(0);
 
-/* Non-incremental GC. */
+// Non-incremental GC.
 gc();
 assertEq(gcstate(), "none");
 
-/* Incremental GC in one slice. */
+// Incremental GC in minimal slice. Note that finalization always uses zero-
+// sized slices while background finalization is on-going, so we need to loop.
 gcslice(1000000);
+while (gcstate() == "finalize") { gcslice(1); }
 assertEq(gcstate(), "none");
 
-/* 
- * Incremental GC in multiple slices: if marking takes more than one slice,
- * we yield before we start sweeping.
- */
+// Incremental GC in multiple slices: if marking takes more than one slice,
+// we yield before we start sweeping.
 gczeal(0);
 gcslice(1);
 assertEq(gcstate(), "mark");
 gcslice(1000000);
 assertEq(gcstate(), "mark");
 gcslice(1000000);
+while (gcstate() == "finalize") { gcslice(1); }
 assertEq(gcstate(), "none");
 
-/* Zeal mode 8: Incremental GC in two slices: 1) mark roots 2) finish collection. */
+// Zeal mode 8: Incremental GC in two main slices:
+//   1) mark roots
+//   2) mark and sweep
+//   *) finalize.
 gczeal(8);
 gcslice(1);
 assertEq(gcstate(), "mark");
 gcslice(1);
+while (gcstate() == "finalize") { gcslice(1); }
 assertEq(gcstate(), "none");
 
-/* Zeal mode 9: Incremental GC in two slices: 1) mark all 2) new marking and finish. */
+// Zeal mode 9: Incremental GC in two main slices:
+//   1) mark roots and marking
+//   2) new marking and sweeping
+//   *) finalize.
 gczeal(9);
 gcslice(1);
 assertEq(gcstate(), "mark");
 gcslice(1);
+while (gcstate() == "finalize") { gcslice(1); }
 assertEq(gcstate(), "none");
 
-/* Zeal mode 10: Incremental GC in multiple slices (always yeilds before sweeping). */
+// Zeal mode 10: Incremental GC in multiple slices (always yeilds before
+// sweeping). This test uses long slices to prove that this zeal mode yields
+// in sweeping, where normal IGC (above) does not.
 gczeal(10);
 gcslice(1000000);
 assertEq(gcstate(), "sweep");
 gcslice(1000000);
+while (gcstate() == "finalize") { gcslice(1); }
 assertEq(gcstate(), "none");
--- a/js/src/jsapi-tests/testGCFinalizeCallback.cpp
+++ b/js/src/jsapi-tests/testGCFinalizeCallback.cpp
@@ -20,16 +20,20 @@ BEGIN_TEST(testGCFinalizeCallback)
     CHECK(checkSingleGroup());
     CHECK(checkFinalizeStatus());
     CHECK(checkFinalizeIsCompartmentGC(false));
 
     /* Full GC, incremental. */
     FinalizeCalls = 0;
     JS::PrepareForFullGC(rt);
     JS::StartIncrementalGC(rt, GC_NORMAL, JS::gcreason::API, 1000000);
+    while (rt->gc.isIncrementalGCInProgress()) {
+        JS::PrepareForFullGC(rt);
+        JS::IncrementalGCSlice(rt, JS::gcreason::API, 1000000);
+    }
     CHECK(!rt->gc.isIncrementalGCInProgress());
     CHECK(rt->gc.isFullGc());
     CHECK(checkMultipleGroups());
     CHECK(checkFinalizeStatus());
     CHECK(checkFinalizeIsCompartmentGC(false));
 
     JS::RootedObject global1(cx, createTestGlobal());
     JS::RootedObject global2(cx, createTestGlobal());
@@ -57,28 +61,38 @@ BEGIN_TEST(testGCFinalizeCallback)
     CHECK(checkSingleGroup());
     CHECK(checkFinalizeStatus());
     CHECK(checkFinalizeIsCompartmentGC(true));
 
     /* Compartment GC, incremental, single compartment. */
     FinalizeCalls = 0;
     JS::PrepareZoneForGC(global1->zone());
     JS::StartIncrementalGC(rt, GC_NORMAL, JS::gcreason::API, 1000000);
+    while (rt->gc.isIncrementalGCInProgress()) {
+        JS::PrepareZoneForGC(global1->zone());
+        JS::IncrementalGCSlice(rt, JS::gcreason::API, 1000000);
+    }
     CHECK(!rt->gc.isIncrementalGCInProgress());
     CHECK(!rt->gc.isFullGc());
     CHECK(checkSingleGroup());
     CHECK(checkFinalizeStatus());
     CHECK(checkFinalizeIsCompartmentGC(true));
 
     /* Compartment GC, incremental, multiple compartments. */
     FinalizeCalls = 0;
     JS::PrepareZoneForGC(global1->zone());
     JS::PrepareZoneForGC(global2->zone());
     JS::PrepareZoneForGC(global3->zone());
     JS::StartIncrementalGC(rt, GC_NORMAL, JS::gcreason::API, 1000000);
+    while (rt->gc.isIncrementalGCInProgress()) {
+        JS::PrepareZoneForGC(global1->zone());
+        JS::PrepareZoneForGC(global2->zone());
+        JS::PrepareZoneForGC(global3->zone());
+        JS::IncrementalGCSlice(rt, JS::gcreason::API, 1000000);
+    }
     CHECK(!rt->gc.isIncrementalGCInProgress());
     CHECK(!rt->gc.isFullGc());
     CHECK(checkMultipleGroups());
     CHECK(checkFinalizeStatus());
     CHECK(checkFinalizeIsCompartmentGC(true));
 
 #ifdef JS_GC_ZEAL
 
@@ -90,16 +104,18 @@ BEGIN_TEST(testGCFinalizeCallback)
     js::SliceBudget budget(js::WorkBudget(1));
     rt->gc.startDebugGC(GC_NORMAL, budget);
     CHECK(rt->gc.state() == js::gc::MARK);
     CHECK(rt->gc.isFullGc());
 
     JS::RootedObject global4(cx, createTestGlobal());
     budget = js::SliceBudget(js::WorkBudget(1));
     rt->gc.debugGCSlice(budget);
+    while (rt->gc.isIncrementalGCInProgress())
+        rt->gc.debugGCSlice(budget);
     CHECK(!rt->gc.isIncrementalGCInProgress());
     CHECK(!rt->gc.isFullGc());
     CHECK(checkMultipleGroups());
     CHECK(checkFinalizeStatus());
 
     for (unsigned i = 0; i < FinalizeCalls - 1; ++i)
         CHECK(!IsCompartmentGCBuffer[i]);
     CHECK(IsCompartmentGCBuffer[FinalizeCalls - 1]);
--- a/js/src/jsapi-tests/testWeakMap.cpp
+++ b/js/src/jsapi-tests/testWeakMap.cpp
@@ -94,32 +94,34 @@ BEGIN_TEST(testWeakMap_keyDelegates)
 
     /*
      * Perform an incremental GC, introducing an unmarked CCW to force the map
      * zone to finish marking before the delegate zone.
      */
     CHECK(newCCW(map, delegateRoot));
     js::SliceBudget budget(js::WorkBudget(1000000));
     rt->gc.startDebugGC(GC_NORMAL, budget);
-    CHECK(!JS::IsIncrementalGCInProgress(rt));
+    while (JS::IsIncrementalGCInProgress(rt))
+        rt->gc.debugGCSlice(budget);
 #ifdef DEBUG
     CHECK(map->zone()->lastZoneGroupIndex() < delegateRoot->zone()->lastZoneGroupIndex());
 #endif
 
     /* Add our entry to the weakmap. */
     JS::RootedValue val(cx, JS::Int32Value(1));
     CHECK(SetWeakMapEntry(cx, map, key, val));
     CHECK(checkSize(map, 1));
 
     /* Check the delegate keeps the entry alive even if the key is not reachable. */
     key = nullptr;
     CHECK(newCCW(map, delegateRoot));
     budget = js::SliceBudget(js::WorkBudget(100000));
     rt->gc.startDebugGC(GC_NORMAL, budget);
-    CHECK(!JS::IsIncrementalGCInProgress(rt));
+    while (JS::IsIncrementalGCInProgress(rt))
+        rt->gc.debugGCSlice(budget);
     CHECK(checkSize(map, 1));
 
     /*
      * Check that the zones finished marking at the same time, which is
      * necessary because of the presence of the delegate and the CCW.
      */
 #ifdef DEBUG
     CHECK(map->zone()->lastZoneGroupIndex() == delegateRoot->zone()->lastZoneGroupIndex());
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3828,40 +3828,41 @@ Zone::sweepCompartments(FreeOp* fop, boo
 
 void
 GCRuntime::sweepZones(FreeOp* fop, bool destroyingRuntime)
 {
     MOZ_ASSERT_IF(destroyingRuntime, rt->gc.numActiveZoneIters == 0);
     if (rt->gc.numActiveZoneIters)
         return;
 
-    AutoLockGC lock(rt); // Avoid race with background sweeping.
+    assertBackgroundSweepingFinished();
 
     JSZoneCallback callback = rt->destroyZoneCallback;
 
     /* Skip the atomsCompartment zone. */
     Zone** read = zones.begin() + 1;
     Zone** end = zones.end();
     Zone** write = read;
     MOZ_ASSERT(zones.length() >= 1);
     MOZ_ASSERT(zones[0]->isAtomsZone());
 
     while (read < end) {
         Zone* zone = *read++;
 
         if (zone->wasGCStarted()) {
-            if ((!zone->isQueuedForBackgroundSweep() &&
-                 zone->arenas.arenaListsAreEmpty() &&
-                 !zone->hasMarkedCompartments()) || destroyingRuntime)
+            MOZ_ASSERT(!zone->isQueuedForBackgroundSweep());
+            const bool zoneIsDead = zone->arenas.arenaListsAreEmpty() &&
+                                    !zone->hasMarkedCompartments();
+            if (zoneIsDead || destroyingRuntime)
             {
                 zone->arenas.checkEmptyFreeLists();
-                AutoUnlockGC unlock(lock);
 
                 if (callback)
                     callback(zone);
+
                 zone->sweepCompartments(fop, false, destroyingRuntime);
                 MOZ_ASSERT(zone->compartments.empty());
                 fop->delete_(zone);
                 continue;
             }
             zone->sweepCompartments(fop, true, destroyingRuntime);
         }
         *write++ = zone;
@@ -5650,23 +5651,16 @@ GCRuntime::endSweepPhase(bool destroying
         if (isFull)
             SweepScriptData(rt);
 
         /* Clear out any small pools that we're hanging on to. */
         if (jit::JitRuntime* jitRuntime = rt->jitRuntime()) {
             jitRuntime->execAlloc().purge();
             jitRuntime->backedgeExecAlloc().purge();
         }
-
-        /*
-         * This removes compartments from rt->compartment, so we do it last to make
-         * sure we don't miss sweeping any compartments.
-         */
-        if (!destroyingRuntime)
-            sweepZones(&fop, destroyingRuntime);
     }
 
     {
         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)
@@ -5684,20 +5678,16 @@ GCRuntime::endSweepPhase(bool destroying
          * safely use IsAboutToBeFinalized(). This is done on the
          * GCHelperState if possible. We acquire the lock only because
          * Expire needs to unlock it for other callers.
          */
         {
             AutoLockGC lock(rt);
             expireChunksAndArenas(invocationKind == GC_SHRINK, lock);
         }
-
-        /* Ensure the compartments get swept if it's the last GC. */
-        if (destroyingRuntime)
-            sweepZones(&fop, destroyingRuntime);
     }
 
     finishMarkingValidation();
 
 #ifdef DEBUG
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
         for (auto i : AllAllocKinds()) {
             MOZ_ASSERT_IF(!IsBackgroundFinalized(i) ||
@@ -5712,39 +5702,31 @@ GCRuntime::endSweepPhase(bool destroying
         for (JSCompartment::WrapperEnum e(c); !e.empty(); e.popFront()) {
             if (e.front().key().kind != CrossCompartmentKey::StringWrapper)
                 AssertNotOnGrayList(&e.front().value().unbarrieredGet().toObject());
         }
     }
 #endif
 }
 
-GCRuntime::IncrementalProgress
+void
 GCRuntime::beginCompactPhase()
 {
+    MOZ_ASSERT(!isBackgroundSweeping());
+
     gcstats::AutoPhase ap(stats, gcstats::PHASE_COMPACT);
 
-    if (isIncremental) {
-        // Poll for end of background sweeping
-        AutoLockGC lock(rt);
-        if (isBackgroundSweeping())
-            return NotFinished;
-    } else {
-        waitBackgroundSweepEnd();
-    }
-
     MOZ_ASSERT(zonesToMaybeCompact.isEmpty());
     for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
         if (CanRelocateZone(zone))
             zonesToMaybeCompact.append(zone);
     }
 
     MOZ_ASSERT(!relocatedArenasToRelease);
     startedCompacting = true;
-    return Finished;
 }
 
 GCRuntime::IncrementalProgress
 GCRuntime::compactPhase(JS::gcreason::Reason reason, SliceBudget& sliceBudget)
 {
     MOZ_ASSERT(rt->gc.nursery.isEmpty());
     assertBackgroundSweepingFinished();
     MOZ_ASSERT(startedCompacting);
@@ -5966,23 +5948,35 @@ GCRuntime::resetIncrementalGC(const char
 
         {
             gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
             rt->gc.waitBackgroundSweepOrAllocEnd();
         }
         break;
       }
 
-      case COMPACT: {
+      case FINALIZE: {
         {
             gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
             rt->gc.waitBackgroundSweepOrAllocEnd();
         }
 
         bool wasCompacting = isCompacting;
+        isCompacting = false;
+
+        auto unlimited = SliceBudget::unlimited();
+        incrementalCollectSlice(unlimited, JS::gcreason::RESET);
+
+        isCompacting = wasCompacting;
+
+        break;
+      }
+
+      case COMPACT: {
+        bool wasCompacting = isCompacting;
 
         isCompacting = true;
         startedCompacting = true;
         zonesToMaybeCompact.clear();
 
         auto unlimited = SliceBudget::unlimited();
         incrementalCollectSlice(unlimited, JS::gcreason::RESET);
 
@@ -6198,29 +6192,58 @@ GCRuntime::incrementalCollectSlice(Slice
         MOZ_FALLTHROUGH;
 
       case SWEEP:
         if (sweepPhase(budget) == NotFinished)
             break;
 
         endSweepPhase(destroyingRuntime);
 
-        incrementalState = COMPACT;
-        MOZ_ASSERT(!startedCompacting);
+        incrementalState = FINALIZE;
 
         /* Yield before compacting since it is not incremental. */
         if (isCompacting && isIncremental)
             break;
 
         MOZ_FALLTHROUGH;
 
+      case FINALIZE:
+        {
+            gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
+
+            // Yield until background finalization is done.
+            if (isIncremental) {
+                // Poll for end of background sweeping
+                AutoLockGC lock(rt);
+                if (isBackgroundSweeping())
+                    break;
+            } else {
+                waitBackgroundSweepEnd();
+            }
+        }
+
+        {
+            // Re-sweep the zones list, now that background finalization is
+            // finished to actually remove and free dead zones.
+            gcstats::AutoPhase ap1(stats, gcstats::PHASE_SWEEP);
+            gcstats::AutoPhase ap2(stats, gcstats::PHASE_DESTROY);
+            AutoSetThreadIsSweeping threadIsSweeping;
+            FreeOp fop(rt);
+            sweepZones(&fop, destroyingRuntime);
+        }
+
+        MOZ_ASSERT(!startedCompacting);
+        incrementalState = COMPACT;
+
+        MOZ_FALLTHROUGH;
+
       case COMPACT:
         if (isCompacting) {
-            if (!startedCompacting && beginCompactPhase() == NotFinished)
-                break;
+            if (!startedCompacting)
+                beginCompactPhase();
 
             if (compactPhase(reason, budget) == NotFinished)
                 break;
 
             endCompactPhase(reason);
         }
 
         finishCollection(reason);
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -43,16 +43,17 @@ namespace gc {
 
 struct FinalizePhase;
 
 enum State {
     NO_INCREMENTAL,
     MARK_ROOTS,
     MARK,
     SWEEP,
+    FINALIZE,
     COMPACT
 };
 
 /* Map from C++ type to alloc kind. JSObject does not have a 1:1 mapping, so must use Arena::thingSize. */
 template <typename T> struct MapTypeToFinalizeKind {};
 template <> struct MapTypeToFinalizeKind<JSScript>          { static const AllocKind kind = AllocKind::SCRIPT; };
 template <> struct MapTypeToFinalizeKind<LazyScript>        { static const AllocKind kind = AllocKind::LAZY_SCRIPT; };
 template <> struct MapTypeToFinalizeKind<Shape>             { static const AllocKind kind = AllocKind::SHAPE; };