Bug 1352430 - Update the GC finalize callback to communicate the new state r=sfink r=mccr8
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 26 Apr 2017 10:57:08 +0100
changeset 403179 42dcd77916bf4f07c76218a2a42f1d4a1e472a51
parent 403178 b56c9e5ce7fe14846883efa87a0818121c32a84e
child 403180 09be4ae7bbf0217af921b4fdb93360b8389338a6
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, mccr8
bugs1352430
milestone55.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 1352430 - Update the GC finalize callback to communicate the new state r=sfink r=mccr8
js/src/jsapi-tests/testGCFinalizeCallback.cpp
js/src/jsapi.h
js/src/jsgc.cpp
js/xpconnect/src/XPCJSRuntime.cpp
--- a/js/src/jsapi-tests/testGCFinalizeCallback.cpp
+++ b/js/src/jsapi-tests/testGCFinalizeCallback.cpp
@@ -155,39 +155,40 @@ virtual void uninit() override
 {
     JS_RemoveFinalizeCallback(cx, FinalizeCallback);
     JSAPITest::uninit();
 }
 
 bool checkSingleGroup()
 {
     CHECK(FinalizeCalls < BufferSize);
-    CHECK(FinalizeCalls == 3);
+    CHECK(FinalizeCalls == 4);
     return true;
 }
 
 bool checkMultipleGroups()
 {
     CHECK(FinalizeCalls < BufferSize);
-    CHECK(FinalizeCalls % 2 == 1);
-    CHECK((FinalizeCalls - 1) / 2 > 1);
+    CHECK(FinalizeCalls % 3 == 1);
+    CHECK((FinalizeCalls - 1) / 3 > 1);
     return true;
 }
 
 bool checkFinalizeStatus()
 {
     /*
      * The finalize callback should be called twice for each sweep group
      * finalized, with status JSFINALIZE_GROUP_START and JSFINALIZE_GROUP_END,
      * and then once more with JSFINALIZE_COLLECTION_END.
      */
 
-    for (unsigned i = 0; i < FinalizeCalls - 1; i += 2) {
-        CHECK(StatusBuffer[i] == JSFINALIZE_GROUP_START);
-        CHECK(StatusBuffer[i + 1] == JSFINALIZE_GROUP_END);
+    for (unsigned i = 0; i < FinalizeCalls - 1; i += 3) {
+        CHECK(StatusBuffer[i] == JSFINALIZE_GROUP_PREPARE);
+        CHECK(StatusBuffer[i + 1] == JSFINALIZE_GROUP_START);
+        CHECK(StatusBuffer[i + 2] == JSFINALIZE_GROUP_END);
     }
 
     CHECK(StatusBuffer[FinalizeCalls - 1] == JSFINALIZE_COLLECTION_END);
 
     return true;
 }
 
 bool checkFinalizeIsZoneGC(bool isZoneGC)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -558,25 +558,30 @@ typedef void
 
 typedef void
 (* JSObjectsTenuredCallback)(JSContext* cx, void* data);
 
 typedef enum JSFinalizeStatus {
     /**
      * Called when preparing to sweep a group of zones, before anything has been
      * swept.  The collector will not yield to the mutator before calling the
-     * callback with JSFINALIZE_GROUP_END status.
+     * callback with JSFINALIZE_GROUP_START status.
+     */
+    JSFINALIZE_GROUP_PREPARE,
+
+    /**
+     * Called after preparing to sweep a group of zones. Weak references to
+     * unmarked things have been removed at this point, but no GC things have
+     * been swept. The collector may yield to the mutator after this point.
      */
     JSFINALIZE_GROUP_START,
 
     /**
-     * Called when preparing to sweep a group of zones. Weak references to
-     * unmarked things have been removed and things that are not swept
-     * incrementally have been finalized at this point.  The collector may yield
-     * to the mutator after this point.
+     * Called after sweeping a group of zones. All dead GC things have been
+     * swept at this point.
      */
     JSFINALIZE_GROUP_END,
 
     /**
      * Called at the end of collection when everything has been swept.
      */
     JSFINALIZE_COLLECTION_END
 } JSFinalizeStatus;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -5125,28 +5125,29 @@ GCRuntime::beginSweepingSweepGroup(AutoL
         /* No need to look up any more weakmap keys from this sweep group. */
         AutoEnterOOMUnsafeRegion oomUnsafe;
         if (!zone->gcWeakKeys().clear())
             oomUnsafe.crash("clearing weak keys in beginSweepingSweepGroup()");
     }
 
     {
         gcstats::AutoPhase ap(stats(), gcstats::PHASE_FINALIZE_START);
-        callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_START);
+        callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_PREPARE);
         {
             gcstats::AutoPhase ap2(stats(), gcstats::PHASE_WEAK_ZONES_CALLBACK);
             callWeakPointerZonesCallbacks();
         }
         {
             gcstats::AutoPhase ap2(stats(), gcstats::PHASE_WEAK_COMPARTMENT_CALLBACK);
             for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
                 for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next())
                     callWeakPointerCompartmentCallbacks(comp);
             }
         }
+        callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_START);
     }
 
     if (sweepingAtoms) {
         AutoLockHelperThreadState helperLock;
         startTask(sweepAtomsTask, gcstats::PHASE_SWEEP_ATOMS, helperLock);
     }
 
     {
@@ -5270,26 +5271,27 @@ GCRuntime::beginSweepingSweepGroup(AutoL
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
         gcstats::AutoSCC scc(stats(), sweepGroupIndex);
         zone->arenas.queueForegroundThingsForSweep(&fop);
     }
 
     sweepPhaseIndex = 0;
     sweepZone = currentSweepGroup;
     sweepActionIndex = 0;
-
-    {
-        gcstats::AutoPhase ap(stats(), gcstats::PHASE_FINALIZE_END);
-        callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_END);
-    }
 }
 
 void
 GCRuntime::endSweepingSweepGroup()
 {
+    {
+        gcstats::AutoPhase ap(stats(), gcstats::PHASE_FINALIZE_END);
+        FreeOp fop(rt);
+        callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_END);
+    }
+
     /* Update the GC state for zones we have swept. */
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
         MOZ_ASSERT(zone->isGCSweeping());
         AutoLockGC lock(rt);
         zone->setGCState(Zone::Finished);
         zone->threshold.updateAfterGC(zone->usage.gcBytes(), invocationKind, tunables,
                                       schedulingState, lock);
     }
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -808,37 +808,44 @@ XPCJSRuntime::FinalizeCallback(JSFreeOp*
                                bool isZoneGC,
                                void* data)
 {
     XPCJSRuntime* self = nsXPConnect::GetRuntimeInstance();
     if (!self)
         return;
 
     switch (status) {
-        case JSFINALIZE_GROUP_START:
+        case JSFINALIZE_GROUP_PREPARE:
         {
             MOZ_ASSERT(!self->mDoingFinalization, "bad state");
 
             MOZ_ASSERT(!self->mGCIsRunning, "bad state");
             self->mGCIsRunning = true;
 
             self->mDoingFinalization = true;
+
+            break;
+        }
+        case JSFINALIZE_GROUP_START:
+        {
+            MOZ_ASSERT(self->mDoingFinalization, "bad state");
+
+            // Sweep scopes needing cleanup
+            XPCWrappedNativeScope::KillDyingScopes();
+
+            MOZ_ASSERT(self->mGCIsRunning, "bad state");
+            self->mGCIsRunning = false;
+
             break;
         }
         case JSFINALIZE_GROUP_END:
         {
             MOZ_ASSERT(self->mDoingFinalization, "bad state");
             self->mDoingFinalization = false;
 
-            // Sweep scopes needing cleanup
-            XPCWrappedNativeScope::KillDyingScopes();
-
-            MOZ_ASSERT(self->mGCIsRunning, "bad state");
-            self->mGCIsRunning = false;
-
             break;
         }
         case JSFINALIZE_COLLECTION_END:
         {
             MOZ_ASSERT(!self->mGCIsRunning, "bad state");
             self->mGCIsRunning = true;
 
             // For now we only have one context. Eventually we'll need to