Bug 1369444 - Sweep the atoms table incrementally r=sfink

js/public/HeapAPI.h

@@ -163,12 +163,12 @@ struct Zone
 
     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);

js/src/gc/GCRuntime.h

@@ -9,6 +9,7 @@
 
 #include "mozilla/Atomics.h"
 #include "mozilla/EnumSet.h"
+#include "mozilla/Maybe.h"
 
 #include "jsfriendapi.h"
 #include "jsgc.h"
@@ -998,6 +999,9 @@ class GCRuntime
                                                     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,
@@ -1223,6 +1227,7 @@ class GCRuntime
     ActiveThreadData<size_t> sweepPhaseIndex;
     ActiveThreadData<JS::Zone*> sweepZone;
     ActiveThreadData<size_t> sweepActionIndex;
+    ActiveThreadData<mozilla::Maybe<AtomSet::Enum>> maybeAtomsToSweep;
     ActiveThreadData<bool> abortSweepAfterCurrentGroup;
 
     /*

js/src/gc/GenerateStatsPhases.py

@@ -97,7 +97,8 @@ PhaseKindGraphRoots = [
             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),

js/src/jsatom.cpp

@@ -226,13 +226,6 @@ js::TraceWellKnownSymbols(JSTracer* trc)
     }
 }
 
-void
-JSRuntime::sweepAtoms()
-{
-    if (atoms_)
-        atoms_->sweep();
-}
-
 bool
 JSRuntime::transformToPermanentAtoms(JSContext* cx)
 {
@@ -352,8 +345,30 @@ AtomizeAndCopyChars(JSContext* cx, const CharT* tbchars, size_t length, PinningB
 
     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));

js/src/jsgc.cpp

@@ -4911,7 +4911,7 @@ class SweepWeakCacheTask : public GCParallelTask
 };
 
 static void
-SweepAtoms(JSRuntime* runtime)
+UpdateAtomsBitmap(JSRuntime* runtime)
 {
     DenseBitmap marked;
     if (runtime->gc.atomMarking.computeBitmapFromChunkMarkBits(runtime, marked)) {
@@ -4924,7 +4924,9 @@ SweepAtoms(JSRuntime* runtime)
     }
 
     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();
@@ -5229,9 +5231,9 @@ GCRuntime::beginSweepingSweepGroup()
     {
         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);
 
@@ -5473,6 +5475,64 @@ GCRuntime::mergeSweptObjectArenas(GCRuntime* gc, FreeOp* fop, Zone* zone, SliceB
     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)
@@ -5532,6 +5592,7 @@ GCRuntime::initializeSweepActions()
     bool ok = true;
 
     AddSweepPhase(&ok);
+    AddSweepAction(&ok, GCRuntime::sweepAtomsTable);
     for (auto kind : ForegroundObjectFinalizePhase.kinds)
         AddSweepAction(&ok, GCRuntime::finalizeAllocKind, kind);
 

js/src/vm/Runtime.cpp

@@ -156,6 +156,7 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime)
     beingDestroyed_(false),
     allowContentJS_(true),
     atoms_(nullptr),
+    atomsAddedWhileSweeping_(nullptr),
     atomsCompartment_(nullptr),
     staticStrings(nullptr),
     commonNames(nullptr),
@@ -843,6 +844,34 @@ JSRuntime::activeGCInAtomsZone()
            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)
 {

js/src/vm/Runtime.h

@@ -815,6 +815,9 @@ struct JSRuntime : public js::MallocProvider<JSRuntime>
     // 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.
@@ -830,14 +833,26 @@ struct JSRuntime : public js::MallocProvider<JSRuntime>
     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_;
     }
@@ -849,6 +864,10 @@ struct JSRuntime : public js::MallocProvider<JSRuntime>
         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;