Bug 1369444 - Sweep the atoms table incrementally r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 02 Jun 2017 10:32:37 +0100
changeset 362012 98b894115e896c9e96b95edc8470fb0c645713b9
parent 362011 9e5ac6fa7858a4c399dd482090b29723c5a991d7
child 362013 26697002542a4bf569e5241ae093193e34693c83
push id31955
push userryanvm@gmail.com
push dateFri, 02 Jun 2017 15:10:12 +0000
treeherdermozilla-central@95d2d23ff510 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1369444
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 1369444 - Sweep the atoms table incrementally r=sfink
js/public/HeapAPI.h
js/src/gc/GCRuntime.h
js/src/gc/GenerateStatsPhases.py
js/src/jsatom.cpp
js/src/jsgc.cpp
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
--- a/js/public/HeapAPI.h
+++ b/js/public/HeapAPI.h
@@ -158,22 +158,22 @@ struct Zone
     // Note: Unrestricted access to the zone's runtime from an arbitrary
     // thread can easily lead to races. Use this method very carefully.
     JSRuntime* runtimeFromAnyThread() const {
         return runtime_;
     }
 
     GCState gcState() const { return gcState_; }
     bool wasGCStarted() const { return gcState_ != NoGC; }
-    bool isGCMarkingBlack() { return gcState_ == Mark; }
-    bool isGCMarkingGray() { return gcState_ == MarkGray; }
-    bool isGCSweeping() { return gcState_ == Sweep; }
-    bool isGCFinished() { return gcState_ == Finished; }
-    bool isGCCompacting() { return gcState_ == Compact; }
-    bool isGCSweepingOrCompacting() { return gcState_ == Sweep || gcState_ == Compact; }
+    bool isGCMarkingBlack() const { return gcState_ == Mark; }
+    bool isGCMarkingGray() const { return gcState_ == MarkGray; }
+    bool isGCSweeping() const { return gcState_ == Sweep; }
+    bool isGCFinished() const { return gcState_ == Finished; }
+    bool isGCCompacting() const { return gcState_ == Compact; }
+    bool isGCSweepingOrCompacting() const { return gcState_ == Sweep || gcState_ == Compact; }
 
     static MOZ_ALWAYS_INLINE JS::shadow::Zone* asShadowZone(JS::Zone* zone) {
         return reinterpret_cast<JS::shadow::Zone*>(zone);
     }
 };
 
 } /* namespace shadow */
 
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef gc_GCRuntime_h
 #define gc_GCRuntime_h
 
 #include "mozilla/Atomics.h"
 #include "mozilla/EnumSet.h"
+#include "mozilla/Maybe.h"
 
 #include "jsfriendapi.h"
 #include "jsgc.h"
 
 #include "gc/AtomMarking.h"
 #include "gc/Heap.h"
 #include "gc/Nursery.h"
 #include "gc/Statistics.h"
@@ -993,16 +994,19 @@ class GCRuntime
     void sweepJitDataOnMainThread(FreeOp* fop);
     void endSweepingSweepGroup();
     IncrementalProgress performSweepActions(SliceBudget& sliceBudget,
                                             AutoLockForExclusiveAccess& lock);
     static IncrementalProgress sweepTypeInformation(GCRuntime* gc, FreeOp* fop, Zone* zone,
                                                     SliceBudget& budget, AllocKind kind);
     static IncrementalProgress mergeSweptObjectArenas(GCRuntime* gc, FreeOp* fop, Zone* zone,
                                                       SliceBudget& budget, AllocKind kind);
+    static IncrementalProgress sweepAtomsTable(GCRuntime* gc, FreeOp* fop, Zone* zone,
+                                               SliceBudget& budget, AllocKind kind);
+    IncrementalProgress sweepAtomsTable(SliceBudget& budget);
     static IncrementalProgress finalizeAllocKind(GCRuntime* gc, FreeOp* fop, Zone* zone,
                                                  SliceBudget& budget, AllocKind kind);
     static IncrementalProgress sweepShapeTree(GCRuntime* gc, FreeOp* fop, Zone* zone,
                                               SliceBudget& budget, AllocKind kind);
     void endSweepPhase(bool lastGC, AutoLockForExclusiveAccess& lock);
     bool allCCVisibleZonesWereCollected() const;
     void sweepZones(FreeOp* fop, ZoneGroup* group, bool lastGC);
     void sweepZoneGroups(FreeOp* fop, bool destroyingRuntime);
@@ -1218,16 +1222,17 @@ class GCRuntime
     /*
      * Incremental sweep state.
      */
     ActiveThreadData<JS::Zone*> sweepGroups;
     ActiveThreadOrGCTaskData<JS::Zone*> currentSweepGroup;
     ActiveThreadData<size_t> sweepPhaseIndex;
     ActiveThreadData<JS::Zone*> sweepZone;
     ActiveThreadData<size_t> sweepActionIndex;
+    ActiveThreadData<mozilla::Maybe<AtomSet::Enum>> maybeAtomsToSweep;
     ActiveThreadData<bool> abortSweepAfterCurrentGroup;
 
     /*
      * Concurrent sweep infrastructure.
      */
     void startTask(GCParallelTask& task, gcstats::PhaseKind phase, AutoLockHelperThreadState& locked);
     void joinTask(GCParallelTask& task, gcstats::PhaseKind phase, AutoLockHelperThreadState& locked);
     friend class AutoRunParallelTask;
--- a/js/src/gc/GenerateStatsPhases.py
+++ b/js/src/gc/GenerateStatsPhases.py
@@ -92,17 +92,18 @@ PhaseKindGraphRoots = [
             PhaseKind("SWEEP_MARK_INCOMING_GRAY", "Mark Incoming Gray Pointers", 14),
             PhaseKind("SWEEP_MARK_GRAY", "Mark Gray", 15),
             PhaseKind("SWEEP_MARK_GRAY_WEAK", "Mark Gray and Weak", 16)
         ]),
         PhaseKind("FINALIZE_START", "Finalize Start Callbacks", 17, [
             PhaseKind("WEAK_ZONES_CALLBACK", "Per-Slice Weak Callback", 57),
             PhaseKind("WEAK_COMPARTMENT_CALLBACK", "Per-Compartment Weak Callback", 58)
         ]),
-        PhaseKind("SWEEP_ATOMS", "Sweep Atoms", 18),
+        PhaseKind("UPDATE_ATOMS_BITMAP", "Sweep Atoms Bitmap", 68),
+        PhaseKind("SWEEP_ATOMS_TABLE", "Sweep Atoms Table", 18),
         PhaseKind("SWEEP_COMPARTMENTS", "Sweep Compartments", 20, [
             PhaseKind("SWEEP_DISCARD_CODE", "Sweep Discard Code", 21),
             PhaseKind("SWEEP_INNER_VIEWS", "Sweep Inner Views", 22),
             PhaseKind("SWEEP_CC_WRAPPER", "Sweep Cross Compartment Wrappers", 23),
             PhaseKind("SWEEP_BASE_SHAPE", "Sweep Base Shapes", 24),
             PhaseKind("SWEEP_INITIAL_SHAPE", "Sweep Initial Shapes", 25),
             PhaseKind("SWEEP_TYPE_OBJECT", "Sweep Type Objects", 26),
             PhaseKind("SWEEP_BREAKPOINT", "Sweep Breakpoints", 27),
--- a/js/src/jsatom.cpp
+++ b/js/src/jsatom.cpp
@@ -221,23 +221,16 @@ js::TraceWellKnownSymbols(JSTracer* trc)
         return;
 
     if (WellKnownSymbols* wks = rt->wellKnownSymbols) {
         for (size_t i = 0; i < JS::WellKnownSymbolLimit; i++)
             TraceProcessGlobalRoot(trc, wks->get(i).get(), "well_known_symbol");
     }
 }
 
-void
-JSRuntime::sweepAtoms()
-{
-    if (atoms_)
-        atoms_->sweep();
-}
-
 bool
 JSRuntime::transformToPermanentAtoms(JSContext* cx)
 {
     MOZ_ASSERT(!parentRuntime);
 
     // All static strings were created as permanent atoms, now move the contents
     // of the atoms table into permanentAtoms and mark each as permanent.
 
@@ -347,18 +340,40 @@ AtomizeAndCopyChars(JSContext* cx, const
 
     // Validate the length before taking the exclusive access lock, as throwing
     // an exception here may reenter this code.
     if (MOZ_UNLIKELY(!JSString::validateLength(cx, length)))
         return nullptr;
 
     AutoLockForExclusiveAccess lock(cx);
 
-    AtomSet& atoms = cx->atoms(lock);
-    AtomSet::AddPtr p = atoms.lookupForAdd(lookup);
+    JSRuntime* rt = cx->runtime();
+    AtomSet& atoms = rt->atoms(lock);
+    AtomSet* atomsAddedWhileSweeping = rt->atomsAddedWhileSweeping();
+    AtomSet::AddPtr p;
+
+    if (!atomsAddedWhileSweeping) {
+        p = atoms.lookupForAdd(lookup);
+    } else {
+        // We're currently sweeping the main atoms table and all new atoms will
+        // be added to a secondary table. Check this first.
+        MOZ_ASSERT(rt->atomsZone(lock)->isGCSweeping());
+        p = atomsAddedWhileSweeping->lookupForAdd(lookup);
+
+        // If that fails check the main table but check if any atom found there
+        // is dead.
+        if (!p) {
+            if (AtomSet::AddPtr p2 = atoms.lookupForAdd(lookup)) {
+                JSAtom* atom = p2->asPtr(cx);
+                if (!IsAboutToBeFinalizedUnbarriered(&atom))
+                    p = p2;
+            }
+        }
+    }
+
     if (p) {
         JSAtom* atom = p->asPtr(cx);
         p->setPinned(bool(pin));
         cx->atomMarking().inlinedMarkAtom(cx, atom);
         if (zonePtr)
             mozilla::Unused << zone->atomCache().add(*zonePtr, AtomStateEntry(atom, false));
         return atom;
     }
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -4906,30 +4906,32 @@ class SweepWeakCacheTask : public GCPara
     {}
 
     void run() override {
         cache.sweep();
     }
 };
 
 static void
-SweepAtoms(JSRuntime* runtime)
+UpdateAtomsBitmap(JSRuntime* runtime)
 {
     DenseBitmap marked;
     if (runtime->gc.atomMarking.computeBitmapFromChunkMarkBits(runtime, marked)) {
         for (GCZonesIter zone(runtime); !zone.done(); zone.next())
             runtime->gc.atomMarking.updateZoneBitmap(zone, marked);
     } else {
         // Ignore OOM in computeBitmapFromChunkMarkBits. The updateZoneBitmap
         // call can only remove atoms from the zone bitmap, so it is
         // conservative to just not call it.
     }
 
     runtime->gc.atomMarking.updateChunkMarkBits(runtime);
-    runtime->sweepAtoms();
+
+    // For convenience sweep these tables non-incrementally as part of bitmap
+    // sweeping; they are likely to be much smaller than the main atoms table.
     runtime->unsafeSymbolRegistry().sweep();
     for (CompartmentsIter comp(runtime, SkipAtoms); !comp.done(); comp.next())
         comp->sweepVarNames();
 }
 
 static void
 SweepCCWrappers(JSRuntime* runtime)
 {
@@ -5224,19 +5226,19 @@ GCRuntime::beginSweepingSweepGroup()
         callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_START);
     }
 
     sweepDebuggerOnMainThread(&fop);
 
     {
         AutoLockHelperThreadState lock;
 
-        Maybe<AutoRunParallelTask> sweepAtoms;
+        Maybe<AutoRunParallelTask> updateAtomsBitmap;
         if (sweepingAtoms)
-            sweepAtoms.emplace(rt, SweepAtoms, PhaseKind::SWEEP_ATOMS, lock);
+            updateAtomsBitmap.emplace(rt, UpdateAtomsBitmap, PhaseKind::UPDATE_ATOMS_BITMAP, lock);
 
         AutoPhase ap(stats(), PhaseKind::SWEEP_COMPARTMENTS);
 
         AutoRunParallelTask sweepCCWrappers(rt, SweepCCWrappers, PhaseKind::SWEEP_CC_WRAPPER, lock);
         AutoRunParallelTask sweepObjectGroups(rt, SweepObjectGroups, PhaseKind::SWEEP_TYPE_OBJECT, lock);
         AutoRunParallelTask sweepRegExps(rt, SweepRegExps, PhaseKind::SWEEP_REGEXP, lock);
         AutoRunParallelTask sweepMisc(rt, SweepMisc, PhaseKind::SWEEP_MISC, lock);
         AutoRunParallelTask sweepCompTasks(rt, SweepCompressionTasks, PhaseKind::SWEEP_COMPRESSION, lock);
@@ -5469,16 +5471,74 @@ GCRuntime::mergeSweptObjectArenas(GCRunt
     // available for allocation.
 
     MOZ_ASSERT(kind == AllocKind::LIMIT);
     zone->arenas.mergeForegroundSweptObjectArenas();
     return Finished;
 }
 
 /* static */ IncrementalProgress
+GCRuntime::sweepAtomsTable(GCRuntime* gc, FreeOp* fop, Zone* zone, SliceBudget& budget,
+                           AllocKind kind)
+{
+    if (!zone->isAtomsZone())
+        return Finished;
+
+    return gc->sweepAtomsTable(budget);
+}
+
+IncrementalProgress
+GCRuntime::sweepAtomsTable(SliceBudget& budget)
+{
+    gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::SWEEP_ATOMS_TABLE);
+
+    auto& maybeAtoms = maybeAtomsToSweep.ref();
+    MOZ_ASSERT_IF(maybeAtoms.isSome(), !maybeAtoms.ref().empty());
+
+    AtomSet* atomsTable = rt->atomsForSweeping();
+    if (!atomsTable)
+        return Finished;
+
+    if (maybeAtoms.isNothing()) {
+        // Create a secondary table to hold new atoms added while we're sweeping
+        // the main table incrementally.
+        if (!rt->createAtomsAddedWhileSweepingTable()) {
+            atomsTable->sweep();
+            return Finished;
+        }
+
+        // Initialize remaining atoms to sweep.
+        maybeAtoms.emplace(*atomsTable);
+    }
+
+    // Sweep the table incrementally until we run out of work or budget.
+    auto& atomsToSweep = *maybeAtoms;
+    while (!atomsToSweep.empty()) {
+        if (budget.isOverBudget())
+            return NotFinished;
+
+        JSAtom* atom = atomsToSweep.front().asPtrUnbarriered();
+        if (IsAboutToBeFinalizedUnbarriered(&atom))
+            atomsToSweep.removeFront();
+        atomsToSweep.popFront();
+    }
+
+    // Add any new atoms from the secondary table.
+    AutoEnterOOMUnsafeRegion oomUnsafe;
+    for (auto r = rt->atomsAddedWhileSweeping()->all(); !r.empty(); r.popFront()) {
+        if (!atomsTable->putNew(AtomHasher::Lookup(r.front().asPtrUnbarriered()), r.front()))
+            oomUnsafe.crash("Adding atom from secondary table after sweep");
+    }
+    rt->destroyAtomsAddedWhileSweepingTable();
+
+    maybeAtoms.reset();
+    return Finished;
+}
+
+/* static */ IncrementalProgress
 GCRuntime::finalizeAllocKind(GCRuntime* gc, FreeOp* fop, Zone* zone, SliceBudget& budget,
                              AllocKind kind)
 {
     // Set the number of things per arena for this AllocKind.
     size_t thingsPerArena = Arena::thingsPerArena(kind);
     auto& sweepList = gc->incrementalSweepList.ref();
     sweepList.setThingsPerArena(thingsPerArena);
 
@@ -5527,16 +5587,17 @@ AddSweepAction(bool* ok, SweepAction::Fu
 }
 
 /* static */ bool
 GCRuntime::initializeSweepActions()
 {
     bool ok = true;
 
     AddSweepPhase(&ok);
+    AddSweepAction(&ok, GCRuntime::sweepAtomsTable);
     for (auto kind : ForegroundObjectFinalizePhase.kinds)
         AddSweepAction(&ok, GCRuntime::finalizeAllocKind, kind);
 
     AddSweepPhase(&ok);
     AddSweepAction(&ok, GCRuntime::sweepTypeInformation);
     AddSweepAction(&ok, GCRuntime::mergeSweptObjectArenas);
 
     for (const auto& finalizePhase : IncrementalFinalizePhases) {
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -151,16 +151,17 @@ JSRuntime::JSRuntime(JSRuntime* parentRu
 #if !EXPOSE_INTL_API
     thousandsSeparator(nullptr),
     decimalSeparator(nullptr),
     numGrouping(nullptr),
 #endif
     beingDestroyed_(false),
     allowContentJS_(true),
     atoms_(nullptr),
+    atomsAddedWhileSweeping_(nullptr),
     atomsCompartment_(nullptr),
     staticStrings(nullptr),
     commonNames(nullptr),
     permanentAtoms(nullptr),
     wellKnownSymbols(nullptr),
     jitSupportsFloatingPoint(false),
     jitSupportsUnalignedAccesses(false),
     jitSupportsSimd(false),
@@ -838,16 +839,44 @@ JSRuntime::onOutOfMemoryCanGC(AllocFunct
 bool
 JSRuntime::activeGCInAtomsZone()
 {
     Zone* zone = atomsCompartment_->zone();
     return (zone->needsIncrementalBarrier() && !gc.isVerifyPreBarriersEnabled()) ||
            zone->wasGCStarted();
 }
 
+bool
+JSRuntime::createAtomsAddedWhileSweepingTable()
+{
+    MOZ_ASSERT(JS::CurrentThreadIsHeapCollecting());
+    MOZ_ASSERT(!atomsAddedWhileSweeping_);
+
+    atomsAddedWhileSweeping_ = js_new<AtomSet>();
+    if (!atomsAddedWhileSweeping_)
+        return false;
+
+    if (!atomsAddedWhileSweeping_->init()) {
+        destroyAtomsAddedWhileSweepingTable();
+        return false;
+    }
+
+    return true;
+}
+
+void
+JSRuntime::destroyAtomsAddedWhileSweepingTable()
+{
+    MOZ_ASSERT(JS::CurrentThreadIsHeapCollecting());
+    MOZ_ASSERT(atomsAddedWhileSweeping_);
+
+    js_delete(atomsAddedWhileSweeping_.ref());
+    atomsAddedWhileSweeping_ = nullptr;
+}
+
 void
 JSRuntime::setUsedByHelperThread(Zone* zone)
 {
     MOZ_ASSERT(!zone->group()->usedByHelperThread);
     MOZ_ASSERT(!zone->wasGCStarted());
     zone->group()->usedByHelperThread = true;
     numHelperThreadZones++;
 }
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -810,50 +810,69 @@ struct JSRuntime : public js::MallocProv
     friend class js::AutoAssertNoContentJS;
 
   private:
     // Set of all atoms other than those in permanentAtoms and staticStrings.
     // Reading or writing this set requires the calling thread to use
     // AutoLockForExclusiveAccess.
     js::ExclusiveAccessLockOrGCTaskData<js::AtomSet*> atoms_;
 
+    // Set of all atoms added while the main atoms table is being swept.
+    js::ExclusiveAccessLockData<js::AtomSet*> atomsAddedWhileSweeping_;
+
     // Compartment and associated zone containing all atoms in the runtime, as
     // well as runtime wide IonCode stubs. Modifying the contents of this
     // compartment requires the calling thread to use AutoLockForExclusiveAccess.
     js::WriteOnceData<JSCompartment*> atomsCompartment_;
 
     // Set of all live symbols produced by Symbol.for(). All such symbols are
     // allocated in the atomsCompartment. Reading or writing the symbol
     // registry requires the calling thread to use AutoLockForExclusiveAccess.
     js::ExclusiveAccessLockOrGCTaskData<js::SymbolRegistry> symbolRegistry_;
 
   public:
     bool initializeAtoms(JSContext* cx);
     void finishAtoms();
     bool atomsAreFinished() const { return !atoms_; }
 
-    void sweepAtoms();
+    js::AtomSet* atomsForSweeping() {
+        MOZ_ASSERT(JS::CurrentThreadIsHeapCollecting());
+        return atoms_;
+    }
 
     js::AtomSet& atoms(js::AutoLockForExclusiveAccess& lock) {
+        MOZ_ASSERT(atoms_);
         return *atoms_;
     }
     js::AtomSet& unsafeAtoms() {
+        MOZ_ASSERT(atoms_);
         return *atoms_;
     }
+
+    bool createAtomsAddedWhileSweepingTable();
+    void destroyAtomsAddedWhileSweepingTable();
+    js::AtomSet* atomsAddedWhileSweeping() {
+        return atomsAddedWhileSweeping_;
+    }
+
     JSCompartment* atomsCompartment(js::AutoLockForExclusiveAccess& lock) {
         return atomsCompartment_;
     }
     JSCompartment* unsafeAtomsCompartment() {
         return atomsCompartment_;
     }
 
     bool isAtomsCompartment(JSCompartment* comp) {
         return comp == atomsCompartment_;
     }
 
+    const JS::Zone* atomsZone(js::AutoLockForExclusiveAccess& lock) const {
+        return gc.atomsZone;
+    }
+
     // The atoms compartment is the only one in its zone.
     bool isAtomsZone(const JS::Zone* zone) const {
         return zone == gc.atomsZone;
     }
 
     bool activeGCInAtomsZone();
 
     js::SymbolRegistry& symbolRegistry(js::AutoLockForExclusiveAccess& lock) {