From f9b398f422e55b1e98a2183728acd567fc0e4253 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Wed, 3 May 2017 11:26:36 +0100 Subject: [PATCH] Bug 1360526 - Sweep weakmaps in parallel with other sweeping r=sfink --HG-- extra : rebase_source : 77d50ed900253ce2d37f2b53355fc7d2f462b49f --- js/src/gc/Zone.h | 16 ++++++------- js/src/jscompartment.cpp | 7 ++---- js/src/jscompartment.h | 2 +- js/src/jsgc.cpp | 52 ++++++++++++++++++++++++---------------- js/src/vm/Debugger.cpp | 20 +++++++++------- js/src/vm/Debugger.h | 3 ++- 6 files changed, 56 insertions(+), 44 deletions(-) diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h index 20fa9c465759..a93e72bfea36 100644 --- a/js/src/gc/Zone.h +++ b/js/src/gc/Zone.h @@ -276,16 +276,16 @@ struct Zone : public JS::shadow::Zone, unsigned lastSweepGroupIndex() { return gcLastSweepGroupIndex; } #endif - using DebuggerVector = js::Vector; - - private: - js::ZoneGroupData debuggers; - void sweepBreakpoints(js::FreeOp* fop); void sweepUniqueIds(js::FreeOp* fop); void sweepWeakMaps(); void sweepCompartments(js::FreeOp* fop, bool keepAtleastOne, bool lastGC); + using DebuggerVector = js::Vector; + + private: + js::ZoneGroupData debuggers; + js::jit::JitZone* createJitZone(JSContext* cx); bool isQueuedForBackgroundSweep() { @@ -321,7 +321,7 @@ struct Zone : public JS::shadow::Zone, private: /* Live weakmaps in this zone. */ - js::ZoneGroupData> gcWeakMapList_; + js::ZoneGroupOrGCTaskData> gcWeakMapList_; public: mozilla::LinkedList& gcWeakMapList() { return gcWeakMapList_.ref(); } @@ -344,7 +344,7 @@ struct Zone : public JS::shadow::Zone, // preserved for re-scanning during sweeping. using WeakEdges = js::Vector; private: - js::ZoneGroupData gcWeakRefs_; + js::ZoneGroupOrGCTaskData gcWeakRefs_; public: WeakEdges& gcWeakRefs() { return gcWeakRefs_.ref(); } @@ -362,7 +362,7 @@ struct Zone : public JS::shadow::Zone, * Mapping from not yet marked keys to a vector of all values that the key * maps to in any live weak map. */ - js::ZoneGroupData gcWeakKeys_; + js::ZoneGroupOrGCTaskData gcWeakKeys_; public: js::gc::WeakKeyTable& gcWeakKeys() { return gcWeakKeys_.ref(); } diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index b1a52c97f759..26fc25443af9 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -838,13 +838,10 @@ JSCompartment::sweepTemplateLiteralMap() } void -JSCompartment::sweepGlobalObject(FreeOp* fop) +JSCompartment::sweepGlobalObject() { - if (global_ && IsAboutToBeFinalized(&global_)) { - if (isDebuggee()) - Debugger::detachAllDebuggersFromGlobal(fop, global_.unbarrieredGet()); + if (global_ && IsAboutToBeFinalized(&global_)) global_.set(nullptr); - } } void diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index 375b5cd69731..9850425542e2 100644 --- a/js/src/jscompartment.h +++ b/js/src/jscompartment.h @@ -708,7 +708,7 @@ struct JSCompartment void sweepCrossCompartmentWrappers(); void sweepSavedStacks(); void sweepTemplateLiteralMap(); - void sweepGlobalObject(js::FreeOp* fop); + void sweepGlobalObject(); void sweepSelfHostingScriptSource(); void sweepJitCompartment(js::FreeOp* fop); void sweepRegExps(); diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index cfbc951f28a3..88414ab2540b 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -2217,7 +2217,7 @@ GCRuntime::sweepZoneAfterCompacting(Zone* zone) c->sweepSavedStacks(); c->sweepTemplateLiteralMap(); c->sweepVarNames(); - c->sweepGlobalObject(fop); + c->sweepGlobalObject(); c->sweepSelfHostingScriptSource(); c->sweepDebugEnvironments(); c->sweepJitCompartment(fop); @@ -4971,6 +4971,7 @@ MAKE_GC_SWEEP_TASK(SweepObjectGroupsTask); MAKE_GC_SWEEP_TASK(SweepRegExpsTask); MAKE_GC_SWEEP_TASK(SweepMiscTask); MAKE_GC_SWEEP_TASK(SweepCompressionTasksTask); +MAKE_GC_SWEEP_TASK(SweepWeakMapsTask); #undef MAKE_GC_SWEEP_TASK /* virtual */ void @@ -5048,6 +5049,27 @@ SweepCompressionTasksTask::run() } } +/* virtual */ void +SweepWeakMapsTask::run() +{ + for (GCSweepGroupIter zone(runtime()); !zone.done(); zone.next()) { + /* Clear all weakrefs that point to unmarked things. */ + for (auto edge : zone->gcWeakRefs()) { + /* Edges may be present multiple times, so may already be nulled. */ + if (*edge && IsAboutToBeFinalizedDuringSweep(**edge)) + *edge = nullptr; + } + zone->gcWeakRefs().clear(); + + /* 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()"); + + zone->sweepWeakMaps(); + } +} + void GCRuntime::startTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked) { @@ -5127,23 +5149,9 @@ GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock) SweepRegExpsTask sweepRegExpsTask(rt); SweepMiscTask sweepMiscTask(rt); SweepCompressionTasksTask sweepCompressionTasksTask(rt); + SweepWeakMapsTask sweepWeakMapsTask(rt); WeakCacheTaskVector sweepCacheTasks = PrepareWeakCacheTasks(rt); - for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) { - /* Clear all weakrefs that point to unmarked things. */ - for (auto edge : zone->gcWeakRefs()) { - /* Edges may be present multiple times, so may already be nulled. */ - if (*edge && IsAboutToBeFinalizedDuringSweep(**edge)) - *edge = nullptr; - } - zone->gcWeakRefs().clear(); - - /* 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_PREPARE); @@ -5161,6 +5169,10 @@ GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock) callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_START); } + // Detach unreachable debuggers and global objects from each other. + // This can modify weakmaps and so must happen before weakmap sweeping. + Debugger::sweepAll(&fop); + if (sweepingAtoms) { AutoLockHelperThreadState helperLock; startTask(sweepAtomsTask, gcstats::PHASE_SWEEP_ATOMS, helperLock); @@ -5177,6 +5189,7 @@ GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock) startTask(sweepRegExpsTask, gcstats::PHASE_SWEEP_REGEXP, helperLock); startTask(sweepMiscTask, gcstats::PHASE_SWEEP_MISC, helperLock); startTask(sweepCompressionTasksTask, gcstats::PHASE_SWEEP_MISC, helperLock); + startTask(sweepWeakMapsTask, gcstats::PHASE_SWEEP_MISC, helperLock); for (auto& task : sweepCacheTasks) startTask(task, gcstats::PHASE_SWEEP_MISC, helperLock); } @@ -5190,14 +5203,13 @@ GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock) js::CancelOffThreadIonCompile(rt, JS::Zone::Sweep); for (GCCompartmentGroupIter c(rt); !c.done(); c.next()) { - c->sweepGlobalObject(&fop); + c->sweepGlobalObject(); c->sweepDebugEnvironments(); c->sweepJitCompartment(&fop); c->sweepTemplateObjects(); } for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) { - zone->sweepWeakMaps(); if (jit::JitZone* jitZone = zone->jitZone()) jitZone->sweep(&fop); } @@ -5208,9 +5220,6 @@ GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock) // Collect watch points associated with unreachable objects. WatchpointMap::sweepAll(rt); - // Detach unreachable debuggers and global objects from each other. - Debugger::sweepAll(&fop); - // Sweep entries containing about-to-be-finalized JitCode and // update relocated TypeSet::Types inside the JitcodeGlobalTable. jit::JitRuntime::SweepJitcodeGlobalTable(rt); @@ -5262,6 +5271,7 @@ GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock) joinTask(sweepRegExpsTask, gcstats::PHASE_SWEEP_REGEXP, helperLock); joinTask(sweepMiscTask, gcstats::PHASE_SWEEP_MISC, helperLock); joinTask(sweepCompressionTasksTask, gcstats::PHASE_SWEEP_MISC, helperLock); + joinTask(sweepWeakMapsTask, gcstats::PHASE_SWEEP_MISC, helperLock); for (auto& task : sweepCacheTasks) joinTask(task, gcstats::PHASE_SWEEP_MISC, helperLock); } diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 1c73ac07d1d5..6b2aa8739b11 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -3223,16 +3223,20 @@ Debugger::sweepAll(FreeOp* fop) Debugger* dbg = group->debuggerList().getFirst(); while (dbg) { Debugger* next = dbg->getNext(); - if (IsAboutToBeFinalized(&dbg->object)) { - /* - * dbg is being GC'd. Detach it from its debuggees. The debuggee - * might be GC'd too. Since detaching requires access to both - * objects, this must be done before finalize time. - */ - for (WeakGlobalObjectSet::Enum e(dbg->debuggees); !e.empty(); e.popFront()) + + // Detach dying debuggers and debuggees from each other. Since this + // requires access to both objects it must be done before either + // object is finalized. + bool debuggerDying = IsAboutToBeFinalized(&dbg->object); + for (WeakGlobalObjectSet::Enum e(dbg->debuggees); !e.empty(); e.popFront()) { + GlobalObject* global = e.front().unbarrieredGet(); + if (debuggerDying || IsAboutToBeFinalizedUnbarriered(&global)) dbg->removeDebuggeeGlobal(fop, e.front().unbarrieredGet(), &e); - fop->delete_(dbg); } + + if (debuggerDying) + fop->delete_(dbg); + dbg = next; } } diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index c52dbd912081..e3f9d82850a4 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -162,9 +162,10 @@ class DebuggerWeakMap : private WeakMap, HeapPtr(this)); !e.empty(); e.popFront()) { if (gc::IsAboutToBeFinalized(&e.front().mutableKey())) { - decZoneCount(e.front().key()->zone()); + decZoneCount(e.front().key()->zoneFromAnyThread()); e.removeFront(); } }