Bug 1360526 - Refactor to remove GCSweepTask and associated macros r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 04 May 2017 18:06:59 +0100
changeset 572841 b388bdf5bf385869136234cebd7fd0faf60fe11c
parent 572840 2c14238a5a52a8a6650171469b2c52dbb2469213
child 572842 9630a51eca0776e055e04797614d15f02aaf65f5
push id57195
push userbmo:rbarker@mozilla.com
push dateThu, 04 May 2017 20:08:56 +0000
reviewerssfink
bugs1360526
milestone55.0a1
Bug 1360526 - Refactor to remove GCSweepTask and associated macros r=sfink
js/src/gc/GCRuntime.h
js/src/jsgc.cpp
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -28,17 +28,17 @@ class AutoLockHelperThreadState;
 class VerifyPreTracer;
 
 namespace gc {
 
 typedef Vector<ZoneGroup*, 4, SystemAllocPolicy> ZoneGroupVector;
 using BlackGrayEdgeVector = Vector<TenuredCell*, 0, SystemAllocPolicy>;
 
 class AutoMaybeStartBackgroundAllocation;
-template <typename Task> class AutoRunGCSweepTask;
+class AutoRunParallelTask;
 class AutoTraceSession;
 class MarkingValidator;
 struct MovingTracer;
 
 enum IncrementalProgress
 {
     NotFinished = 0,
     Finished
@@ -982,17 +982,17 @@ class GCRuntime
     void markAllWeakReferences(gcstats::Phase phase);
     void markAllGrayReferences(gcstats::Phase phase);
 
     void beginSweepPhase(JS::gcreason::Reason reason, AutoLockForExclusiveAccess& lock);
     void groupZonesForSweeping(JS::gcreason::Reason reason, AutoLockForExclusiveAccess& lock);
     MOZ_MUST_USE bool findInterZoneEdges();
     void getNextSweepGroup();
     void endMarkingSweepGroup();
-    void beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock);
+    void beginSweepingSweepGroup();
     bool shouldReleaseObservedTypes();
     void sweepDebuggerOnMainThread(FreeOp* fop);
     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);
@@ -1225,17 +1225,17 @@ class GCRuntime
     ActiveThreadData<size_t> sweepActionIndex;
     ActiveThreadData<bool> abortSweepAfterCurrentGroup;
 
     /*
      * Concurrent sweep infrastructure.
      */
     void startTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked);
     void joinTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked);
-    template <typename Task> friend class AutoRunGCSweepTask;
+    friend class AutoRunParallelTask;
 
     /*
      * List head of arenas allocated during the sweep phase.
      */
     ActiveThreadData<Arena*> arenasAllocatedDuringSweep;
 
     /*
      * Incremental compacting state.
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -4923,144 +4923,119 @@ GCRuntime::endMarkingSweepGroup()
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
         MOZ_ASSERT(zone->isGCMarkingGray());
         zone->setGCState(Zone::Mark);
     }
     MOZ_ASSERT(marker.isDrained());
     marker.setMarkColorBlack();
 }
 
-class GCSweepTask : public GCParallelTask
-{
-    GCSweepTask(const GCSweepTask&) = delete;
-
-  public:
-    explicit GCSweepTask(JSRuntime* rt) : GCParallelTask(rt) {}
-    GCSweepTask(GCSweepTask&& other)
-      : GCParallelTask(mozilla::Move(other))
-    {}
-};
-
 // Causes the given WeakCache to be swept when run.
-class SweepWeakCacheTask : public GCSweepTask
+class SweepWeakCacheTask : public GCParallelTask
 {
     JS::WeakCache<void*>& cache;
 
     SweepWeakCacheTask(const SweepWeakCacheTask&) = delete;
 
   public:
-    SweepWeakCacheTask(JSRuntime* rt, JS::WeakCache<void*>& wc) : GCSweepTask(rt), cache(wc) {}
+    SweepWeakCacheTask(JSRuntime* rt, JS::WeakCache<void*>& wc)
+      : GCParallelTask(rt), cache(wc)
+    {}
+
     SweepWeakCacheTask(SweepWeakCacheTask&& other)
-      : GCSweepTask(mozilla::Move(other)), cache(other.cache)
+      : GCParallelTask(mozilla::Move(other)), cache(other.cache)
     {}
 
     void run() override {
         cache.sweep();
     }
 };
 
-#define MAKE_GC_SWEEP_TASK(name, phase)                                       \
-    class name : public GCSweepTask {                                         \
-        void run() override;                                                  \
-      public:                                                                 \
-        explicit name (JSRuntime* rt) : GCSweepTask(rt) {}                    \
-        static const gcstats::Phase StatsPhase = phase;                       \
-    }
-MAKE_GC_SWEEP_TASK(SweepAtomsTask,            gcstats::PHASE_SWEEP_ATOMS);
-MAKE_GC_SWEEP_TASK(SweepCCWrappersTask,       gcstats::PHASE_SWEEP_CC_WRAPPER);
-MAKE_GC_SWEEP_TASK(SweepObjectGroupsTask,     gcstats::PHASE_SWEEP_TYPE_OBJECT);
-MAKE_GC_SWEEP_TASK(SweepRegExpsTask,          gcstats::PHASE_SWEEP_REGEXP);
-MAKE_GC_SWEEP_TASK(SweepMiscTask,             gcstats::PHASE_SWEEP_MISC);
-MAKE_GC_SWEEP_TASK(SweepCompressionTasksTask, gcstats::PHASE_SWEEP_COMPRESSION);
-MAKE_GC_SWEEP_TASK(SweepWeakMapsTask,         gcstats::PHASE_SWEEP_WEAKMAPS);
-MAKE_GC_SWEEP_TASK(SweepUniqueIdsTask,        gcstats::PHASE_SWEEP_UNIQUEIDS);
-#undef MAKE_GC_SWEEP_TASK
-
-/* virtual */ void
-SweepAtomsTask::run()
+static void
+SweepAtoms(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);
+    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();
-    runtime()->unsafeSymbolRegistry().sweep();
-    for (CompartmentsIter comp(runtime(), SkipAtoms); !comp.done(); comp.next())
+    runtime->gc.atomMarking.updateChunkMarkBits(runtime);
+    runtime->sweepAtoms();
+    runtime->unsafeSymbolRegistry().sweep();
+    for (CompartmentsIter comp(runtime, SkipAtoms); !comp.done(); comp.next())
         comp->sweepVarNames();
 }
 
-/* virtual */ void
-SweepCCWrappersTask::run()
-{
-    for (GCCompartmentGroupIter c(runtime()); !c.done(); c.next())
+static void
+SweepCCWrappers(JSRuntime* runtime)
+{
+    for (GCCompartmentGroupIter c(runtime); !c.done(); c.next())
         c->sweepCrossCompartmentWrappers();
 }
 
-/* virtual */ void
-SweepObjectGroupsTask::run()
-{
-    for (GCCompartmentGroupIter c(runtime()); !c.done(); c.next())
-        c->objectGroups.sweep(runtime()->defaultFreeOp());
-}
-
-/* virtual */ void
-SweepRegExpsTask::run()
-{
-    for (GCCompartmentGroupIter c(runtime()); !c.done(); c.next())
+static void
+SweepObjectGroups(JSRuntime* runtime)
+{
+    for (GCCompartmentGroupIter c(runtime); !c.done(); c.next())
+        c->objectGroups.sweep(runtime->defaultFreeOp());
+}
+
+static void
+SweepRegExps(JSRuntime* runtime)
+{
+    for (GCCompartmentGroupIter c(runtime); !c.done(); c.next())
         c->sweepRegExps();
 }
 
-/* virtual */ void
-SweepMiscTask::run()
-{
-    for (GCCompartmentGroupIter c(runtime()); !c.done(); c.next()) {
+static void
+SweepMisc(JSRuntime* runtime)
+{
+    for (GCCompartmentGroupIter c(runtime); !c.done(); c.next()) {
         c->sweepGlobalObject();
         c->sweepTemplateObjects();
         c->sweepSavedStacks();
         c->sweepTemplateLiteralMap();
         c->sweepSelfHostingScriptSource();
         c->sweepNativeIterators();
         c->sweepWatchpoints();
     }
 }
 
-/* virtual */ void
-SweepCompressionTasksTask::run()
+static void
+SweepCompressionTasks(JSRuntime* runtime)
 {
     AutoLockHelperThreadState lock;
 
     // Attach finished compression tasks.
     auto& finished = HelperThreadState().compressionFinishedList(lock);
     for (size_t i = 0; i < finished.length(); i++) {
-        if (finished[i]->runtimeMatches(runtime())) {
+        if (finished[i]->runtimeMatches(runtime)) {
             UniquePtr<SourceCompressionTask> task(Move(finished[i]));
             HelperThreadState().remove(finished, &i);
             task->complete();
         }
     }
 
     // Sweep pending tasks that are holding onto should-be-dead ScriptSources.
     auto& pending = HelperThreadState().compressionPendingList(lock);
     for (size_t i = 0; i < pending.length(); i++) {
         if (pending[i]->shouldCancel())
             HelperThreadState().remove(pending, &i);
     }
 }
 
-/* virtual */ void
-SweepWeakMapsTask::run()
-{
-    for (GCSweepGroupIter zone(runtime()); !zone.done(); zone.next()) {
+static void
+SweepWeakMaps(JSRuntime* runtime)
+{
+    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();
 
@@ -5068,21 +5043,21 @@ SweepWeakMapsTask::run()
         AutoEnterOOMUnsafeRegion oomUnsafe;
         if (!zone->gcWeakKeys().clear())
             oomUnsafe.crash("clearing weak keys in beginSweepingSweepGroup()");
 
         zone->sweepWeakMaps();
     }
 }
 
-/* virtual */ void
-SweepUniqueIdsTask::run()
+static void
+SweepUniqueIds(JSRuntime* runtime)
 {
     FreeOp fop(nullptr);
-    for (GCSweepGroupIter zone(runtime()); !zone.done(); zone.next())
+    for (GCSweepGroupIter zone(runtime); !zone.done(); zone.next())
         zone->sweepUniqueIds(&fop);
 }
 
 void
 GCRuntime::startTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked)
 {
     if (!task.startWithLockHeld(locked)) {
         AutoUnlockHelperThreadState unlock(locked);
@@ -5202,43 +5177,54 @@ PrepareWeakCacheTasks(JSRuntime* rt)
             return true;
         });
         tasks.clear();
     }
 
     return tasks;
 }
 
-template <typename Task>
-class js::gc::AutoRunGCSweepTask
-{
-    Task task_;
-    GCRuntime* gc_;
+class MOZ_RAII js::gc::AutoRunParallelTask : public GCParallelTask
+{
+    using Func = void (*)(JSRuntime*);
+
+    Func func_;
+    gcstats::Phase phase_;
     AutoLockHelperThreadState& lock_;
 
   public:
-    AutoRunGCSweepTask(GCRuntime* gc, AutoLockHelperThreadState& lock)
-      : task_(gc->rt), gc_(gc), lock_(lock)
+    AutoRunParallelTask(JSRuntime* rt, Func func, gcstats::Phase phase,
+                       AutoLockHelperThreadState& lock)
+      : GCParallelTask(rt),
+        func_(func),
+        phase_(phase),
+        lock_(lock)
     {
-        gc_->startTask(task_, Task::StatsPhase, lock_);
-    }
-
-    ~AutoRunGCSweepTask() {
-        gc_->joinTask(task_, Task::StatsPhase, lock_);
+        runtime()->gc.startTask(*this, phase_, lock_);
+    }
+
+    ~AutoRunParallelTask() {
+        runtime()->gc.joinTask(*this, phase_, lock_);
+    }
+
+    void run() override {
+        func_(runtime());
     }
 };
 
 void
-GCRuntime::beginSweepingSweepGroup(AutoLockForExclusiveAccess& lock)
+GCRuntime::beginSweepingSweepGroup()
 {
     /*
      * Begin sweeping the group of zones in currentSweepGroup, performing
      * actions that must be done before yielding to caller.
      */
 
+    using namespace gcstats;
+
     bool sweepingAtoms = false;
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
         /* Set the GC state to sweeping. */
         MOZ_ASSERT(zone->isGCMarking());
         zone->setGCState(Zone::Sweep);
 
         /* Purge the ArenaLists before sweeping. */
         zone->arenas.purge();
@@ -5249,71 +5235,72 @@ GCRuntime::beginSweepingSweepGroup(AutoL
 #ifdef DEBUG
         zone->gcLastSweepGroupIndex = sweepGroupIndex;
 #endif
     }
 
     validateIncrementalMarking();
 
     FreeOp fop(rt);
-    WeakCacheTaskVector sweepCacheTasks = PrepareWeakCacheTasks(rt);
 
     {
-        gcstats::AutoPhase ap(stats(), gcstats::PHASE_FINALIZE_START);
+        AutoPhase ap(stats(), PHASE_FINALIZE_START);
         callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_PREPARE);
         {
-            gcstats::AutoPhase ap2(stats(), gcstats::PHASE_WEAK_ZONES_CALLBACK);
+            AutoPhase ap2(stats(), PHASE_WEAK_ZONES_CALLBACK);
             callWeakPointerZonesCallbacks();
         }
         {
-            gcstats::AutoPhase ap2(stats(), gcstats::PHASE_WEAK_COMPARTMENT_CALLBACK);
+            AutoPhase ap2(stats(), PHASE_WEAK_COMPARTMENT_CALLBACK);
             for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
                 for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next())
                     callWeakPointerCompartmentCallbacks(comp);
             }
         }
         callFinalizeCallbacks(&fop, JSFINALIZE_GROUP_START);
     }
 
     sweepDebuggerOnMainThread(&fop);
 
     {
-        AutoLockHelperThreadState helperLock;
-
-        Maybe<AutoRunGCSweepTask<SweepAtomsTask>> sweepAtoms;
+        AutoLockHelperThreadState lock;
+
+        Maybe<AutoRunParallelTask> sweepAtoms;
         if (sweepingAtoms)
-            sweepAtoms.emplace(this, helperLock);
-
-        gcstats::AutoPhase ap(stats(), gcstats::PHASE_SWEEP_COMPARTMENTS);
-        gcstats::AutoSCC scc(stats(), sweepGroupIndex);
-
-        AutoRunGCSweepTask<SweepCCWrappersTask> sweepCCWrappers(this, helperLock);
-        AutoRunGCSweepTask<SweepObjectGroupsTask> sweepObjectGroups(this, helperLock);
-        AutoRunGCSweepTask<SweepRegExpsTask> sweepRegExps(this, helperLock);
-        AutoRunGCSweepTask<SweepMiscTask> sweepMisc(this, helperLock);
-        AutoRunGCSweepTask<SweepCompressionTasksTask> sweepCompressionTasks(this, helperLock);
-        AutoRunGCSweepTask<SweepWeakMapsTask> sweepWeakMaps(this, helperLock);
-        AutoRunGCSweepTask<SweepUniqueIdsTask> sweepUniqueIds(this, helperLock);
+            sweepAtoms.emplace(rt, SweepAtoms, PHASE_SWEEP_ATOMS, lock);
+
+        AutoPhase ap(stats(), PHASE_SWEEP_COMPARTMENTS);
+        AutoSCC scc(stats(), sweepGroupIndex);
+
+        AutoRunParallelTask sweepCCWrappers(rt, SweepCCWrappers, PHASE_SWEEP_CC_WRAPPER, lock);
+        AutoRunParallelTask sweepObjectGroups(rt, SweepObjectGroups, PHASE_SWEEP_TYPE_OBJECT, lock);
+        AutoRunParallelTask sweepRegExps(rt, SweepRegExps, PHASE_SWEEP_REGEXP, lock);
+        AutoRunParallelTask sweepMisc(rt, SweepMisc, PHASE_SWEEP_MISC, lock);
+        AutoRunParallelTask sweepCompTasks(rt, SweepCompressionTasks, PHASE_SWEEP_COMPRESSION, lock);
+        AutoRunParallelTask sweepWeakMaps(rt, SweepWeakMaps, PHASE_SWEEP_WEAKMAPS, lock);
+        AutoRunParallelTask sweepUniqueIds(rt, SweepUniqueIds, PHASE_SWEEP_UNIQUEIDS, lock);
+
+        WeakCacheTaskVector sweepCacheTasks = PrepareWeakCacheTasks(rt);
         for (auto& task : sweepCacheTasks)
-            startTask(task, gcstats::PHASE_SWEEP_WEAK_CACHES, helperLock);
+            startTask(task, PHASE_SWEEP_WEAK_CACHES, lock);
 
         {
-            AutoUnlockHelperThreadState unlock(helperLock);
+            AutoUnlockHelperThreadState unlock(lock);
             sweepJitDataOnMainThread(&fop);
         }
 
         for (auto& task : sweepCacheTasks)
-            joinTask(task, gcstats::PHASE_SWEEP_WEAK_CACHES, helperLock);
+            joinTask(task, PHASE_SWEEP_WEAK_CACHES, lock);
     }
 
     // Queue all GC things in all zones for sweeping, either on the foreground
     // or on the background thread.
 
     for (GCSweepGroupIter zone(rt); !zone.done(); zone.next()) {
-        gcstats::AutoSCC scc(stats(), sweepGroupIndex);
+        AutoSCC scc(stats(), sweepGroupIndex);
 
         zone->arenas.queueForForegroundSweep(&fop, ForegroundObjectFinalizePhase);
         for (unsigned i = 0; i < ArrayLength(IncrementalFinalizePhases); ++i)
             zone->arenas.queueForForegroundSweep(&fop, IncrementalFinalizePhases[i]);
 
         for (unsigned i = 0; i < ArrayLength(BackgroundFinalizePhases); ++i)
             zone->arenas.queueForBackgroundSweep(&fop, BackgroundFinalizePhases[i]);
 
@@ -5385,17 +5372,17 @@ GCRuntime::beginSweepPhase(JS::gcreason:
 
     releaseObservedTypes = shouldReleaseObservedTypes();
 
     AssertNoWrappersInGrayList(rt);
     DropStringWrappers(rt);
 
     groupZonesForSweeping(reason, lock);
     endMarkingSweepGroup();
-    beginSweepingSweepGroup(lock);
+    beginSweepingSweepGroup();
 }
 
 bool
 ArenaLists::foregroundFinalize(FreeOp* fop, AllocKind thingKind, SliceBudget& sliceBudget,
                                SortedArenaList& sweepList)
 {
     MOZ_ASSERT_IF(IsObjectAllocKind(thingKind), savedObjectArenas(thingKind).isEmpty());
 
@@ -5627,17 +5614,17 @@ GCRuntime::performSweepActions(SliceBudg
         sweepPhaseIndex = 0;
 
         endSweepingSweepGroup();
         getNextSweepGroup();
         if (!currentSweepGroup)
             return Finished;
 
         endMarkingSweepGroup();
-        beginSweepingSweepGroup(lock);
+        beginSweepingSweepGroup();
     }
 }
 
 bool
 GCRuntime::allCCVisibleZonesWereCollected() const
 {
     // Calculate whether the gray marking state is now valid.
     //