From 40e8eb46609dcb8780764774ec550afff1eed3a5 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Wed, 25 Oct 2017 22:10:11 -0400 Subject: [PATCH] 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 | 5 --- js/src/jsapi-tests/testGCFinalizeCallback.cpp | 14 +++---- js/src/jsgc.cpp | 39 ++++++------------- 3 files changed, 19 insertions(+), 39 deletions(-) diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index e26fc78ee484..01f4733f95b4 100644 --- a/js/src/gc/GCRuntime.h +++ b/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 releaseObservedTypes; - /* Whether the atoms table will be swept. */ - ActiveThreadOrGCTaskData shouldSweepAtomsZone_; - /* Singly linked list of zones to be swept in the background. */ ActiveThreadOrGCTaskData backgroundSweepZones; diff --git a/js/src/jsapi-tests/testGCFinalizeCallback.cpp b/js/src/jsapi-tests/testGCFinalizeCallback.cpp index f7b26649cd6d..14204b3923d0 100644 --- a/js/src/jsapi-tests/testGCFinalizeCallback.cpp +++ b/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; } diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 801465eeb234..e7e10293487e 100644 --- a/js/src/jsgc.cpp +++ b/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 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);