Backed out changeset 2c36f41ed77c (bug 1410123) for causing frequent Windows mochitest-gl leaks.

MozReview-Commit-ID: LyBJTmVOJmE

--HG--
extra : transplant_source : %1B%FA%EA%0C%03%EC%2B%00%EC%E8%F7ir%0F%F8%16%D6%83%2C%A9

js/src/gc/GCRuntime.h

@@ -909,8 +909,6 @@ class GCRuntime
 
     bool fullGCForAtomsRequested() const { return fullGCForAtomsRequested_; }
 
-    bool shouldSweepAtomsZone() { return shouldSweepAtomsZone_; }
-
     double computeHeapGrowthFactor(size_t lastBytes);
     size_t computeTriggerBytes(double growthFactor, size_t lastBytes);
 
@@ -1303,9 +1301,6 @@ class GCRuntime
     /* Whether observed type information is being released in the current GC. */
     ActiveThreadData<bool> releaseObservedTypes;
 
-    /* Whether the atoms table will be swept. */
-    ActiveThreadOrGCTaskData<bool> shouldSweepAtomsZone_;
-
     /* Singly linked list of zones to be swept in the background. */
     ActiveThreadOrGCTaskData<ZoneList> backgroundSweepZones;
 

js/src/jsapi-tests/testGCFinalizeCallback.cpp

@@ -4,7 +4,7 @@
 
 #include "jsapi-tests/tests.h"
 
-static const unsigned BufferSize = 32;
+static const unsigned BufferSize = 20;
 static unsigned FinalizeCalls = 0;
 static JSFinalizeStatus StatusBuffer[BufferSize];
 
@@ -16,7 +16,7 @@ BEGIN_TEST(testGCFinalizeCallback)
     FinalizeCalls = 0;
     JS_GC(cx);
     CHECK(cx->runtime()->gc.isFullGc());
-    CHECK(checkGroupCount(1));
+    CHECK(checkSingleGroup());
     CHECK(checkFinalizeStatus());
 
     /* Full GC, incremental. */
@@ -50,7 +50,7 @@ BEGIN_TEST(testGCFinalizeCallback)
     JS::PrepareZoneForGC(global1->zone());
     JS::GCForReason(cx, GC_NORMAL, JS::gcreason::API);
     CHECK(!cx->runtime()->gc.isFullGc());
-    CHECK(checkGroupCount(1));
+    CHECK(checkSingleGroup());
     CHECK(checkFinalizeStatus());
 
     /* Zone GC, non-incremental, multiple zones. */
@@ -60,7 +60,7 @@ BEGIN_TEST(testGCFinalizeCallback)
     JS::PrepareZoneForGC(global3->zone());
     JS::GCForReason(cx, GC_NORMAL, JS::gcreason::API);
     CHECK(!cx->runtime()->gc.isFullGc());
-    CHECK(checkGroupCount(1));
+    CHECK(checkSingleGroup());
     CHECK(checkFinalizeStatus());
 
     /* Zone GC, incremental, single zone. */
@@ -73,7 +73,7 @@ BEGIN_TEST(testGCFinalizeCallback)
     }
     CHECK(!cx->runtime()->gc.isIncrementalGCInProgress());
     CHECK(!cx->runtime()->gc.isFullGc());
-    CHECK(checkGroupCount(2)); // One for our zone, one for the atoms zone.
+    CHECK(checkSingleGroup());
     CHECK(checkFinalizeStatus());
 
     /* Zone GC, incremental, multiple zones. */
@@ -152,10 +152,10 @@ virtual void uninit() override
     JSAPITest::uninit();
 }
 
-bool checkGroupCount(size_t count)
+bool checkSingleGroup()
 {
     CHECK(FinalizeCalls < BufferSize);
-    CHECK(FinalizeCalls == count * 3 + 1);
+    CHECK(FinalizeCalls == 4);
     return true;
 }
 

js/src/jsgc.cpp

@@ -4039,6 +4039,10 @@ ShouldCollectZone(Zone* zone, JS::gcreason::Reason reason)
     // Off-thread parsing is inhibited after the start of GC which prevents
     // races between creating atoms during parsing and sweeping atoms on the
     // active thread.
+    //
+    // Otherwise, we always schedule a GC in the atoms zone so that atoms which
+    // the other collected zones are using are marked, and we can update the
+    // set of atoms in use by the other collected zones at the end of the GC.
     if (zone->isAtomsZone())
         return TlsContext.get()->canCollectAtoms();
 
@@ -5233,11 +5237,9 @@ UpdateAtomsBitmap(JSRuntime* runtime)
 
     // For convenience sweep these tables non-incrementally as part of bitmap
     // sweeping; they are likely to be much smaller than the main atoms table.
-    if (runtime->gc.shouldSweepAtomsZone()) {
-        runtime->unsafeSymbolRegistry().sweep();
-        for (CompartmentsIter comp(runtime, SkipAtoms); !comp.done(); comp.next())
-            comp->sweepVarNames();
-    }
+    runtime->unsafeSymbolRegistry().sweep();
+    for (CompartmentsIter comp(runtime, SkipAtoms); !comp.done(); comp.next())
+        comp->sweepVarNames();
 }
 
 static void
@@ -5493,7 +5495,7 @@ GCRuntime::beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget)
 
     AutoSCC scc(stats(), sweepGroupIndex);
 
-    bool groupIncludesAtomsZone = false;
+    bool sweepingAtoms = false;
     for (SweepGroupZonesIter zone(rt); !zone.done(); zone.next()) {
         /* Set the GC state to sweeping. */
         zone->changeGCState(Zone::Mark, Zone::Sweep);
@@ -5502,7 +5504,7 @@ GCRuntime::beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget)
         zone->arenas.purge();
 
         if (zone->isAtomsZone())
-            groupIncludesAtomsZone = true;
+            sweepingAtoms = true;
 
 #ifdef DEBUG
         zone->gcLastSweepGroupIndex = sweepGroupIndex;
@@ -5534,7 +5536,7 @@ GCRuntime::beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget)
         AutoLockHelperThreadState lock;
 
         Maybe<AutoRunParallelTask> updateAtomsBitmap;
-        if (groupIncludesAtomsZone)
+        if (sweepingAtoms)
             updateAtomsBitmap.emplace(rt, UpdateAtomsBitmap, PhaseKind::UPDATE_ATOMS_BITMAP, lock);
 
         AutoPhase ap(stats(), PhaseKind::SWEEP_COMPARTMENTS);
@@ -5563,15 +5565,13 @@ GCRuntime::beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget)
             joinTask(task, PhaseKind::SWEEP_WEAK_CACHES, lock);
     }
 
-    if (groupIncludesAtomsZone && shouldSweepAtomsZone())
+    if (sweepingAtoms)
         startSweepingAtomsTable();
 
     // Queue all GC things in all zones for sweeping, either on the foreground
     // or on the background thread.
 
     for (SweepGroupZonesIter zone(rt); !zone.done(); zone.next()) {
-        if (zone->isAtomsZone() && !shouldSweepAtomsZone())
-            continue;
 
         zone->arenas.queueForForegroundSweep(fop, ForegroundObjectFinalizePhase);
         zone->arenas.queueForForegroundSweep(fop, ForegroundNonObjectFinalizePhase);
@@ -5655,9 +5655,6 @@ GCRuntime::beginSweepPhase(JS::gcreason::Reason reason, AutoLockForExclusiveAcce
 
     MOZ_ASSERT(!abortSweepAfterCurrentGroup);
 
-    MOZ_ASSERT(maybeAtomsToSweep.ref().isNothing());
-    MOZ_ASSERT(!rt->atomsAddedWhileSweeping());
-
     AutoSetThreadIsSweeping threadIsSweeping;
 
     releaseHeldRelocatedArenas();
@@ -5832,7 +5829,7 @@ GCRuntime::startSweepingAtomsTable()
 IncrementalProgress
 GCRuntime::sweepAtomsTable(FreeOp* fop, SliceBudget& budget)
 {
-    if (!atomsZone->isGCSweeping() || !shouldSweepAtomsZone())
+    if (!atomsZone->isGCSweeping())
         return Finished;
 
     gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP_ATOMS_TABLE);
@@ -6478,9 +6475,6 @@ GCRuntime::endSweepPhase(bool destroyingRuntime, AutoLockForExclusiveAccess& loc
 #endif
 
     AssertNoWrappersInGrayList(rt);
-
-    MOZ_ASSERT(maybeAtomsToSweep.ref().isNothing());
-    MOZ_ASSERT(!rt->atomsAddedWhileSweeping());
 }
 
 void
@@ -7244,15 +7238,6 @@ GCRuntime::gcCycle(bool nonincrementalByAPI, SliceBudget& budget, JS::gcreason::
     // Note that GC callbacks are allowed to re-enter GC.
     AutoCallGCCallbacks callCallbacks(*this);
 
-    // We always schedule a GC in the atoms zone so that atoms which the other
-    // collected zones are using are marked, and we can update the set of atoms
-    // in use by the other collected zones at the end of the GC.  However we
-    // only collect the atoms table if the atoms zone was actually scheduled.
-    if (!isIncrementalGCInProgress()) {
-        shouldSweepAtomsZone_ = atomsZone->isGCScheduled();
-        atomsZone->scheduleGC();
-    }
-
     gcstats::AutoGCSlice agc(stats(), scanZonesBeforeGC(), invocationKind, budget, reason);
 
     minorGC(reason, gcstats::PhaseKind::EVICT_NURSERY_FOR_MAJOR_GC);