Bug 1502940 - Tidy up sweep actions that implement GC zeal modes r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 30 Oct 2018 10:32:10 +0000
changeset 443473 131bc0e561970979dec138a2d65d5b7b4d3d995a
parent 443472 0d19b902fd7cf57dd5c00862489f26ca1ae99af6
child 443474 f2aee8014db6194ee85ec6043263389698a4b1d4
push id109383
push userjcoppeard@mozilla.com
push dateTue, 30 Oct 2018 10:42:32 +0000
treeherdermozilla-inbound@131bc0e56197 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1502940
milestone65.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 1502940 - Tidy up sweep actions that implement GC zeal modes r=sfink
js/src/gc/GC.cpp
js/src/gc/GCRuntime.h
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -6110,38 +6110,29 @@ GCRuntime::beginSweepingSweepGroup(FreeO
 
     sweepCache = nullptr;
     safeToYield = true;
 
     return Finished;
 }
 
 #ifdef JS_GC_ZEAL
-
 bool
 GCRuntime::shouldYieldForZeal(ZealMode mode)
 {
-    return useZeal && isIncremental && hasZealMode(mode);
-}
-
-IncrementalProgress
-GCRuntime::maybeYieldForSweepingZeal(FreeOp* fop, SliceBudget& budget)
-{
-    /*
-     * Check whether we need to yield for GC zeal. We always yield when running
-     * in incremental multi-slice zeal mode so RunDebugGC can reset the slice
-     * budget.
-     */
-    if (initialState != State::Sweep && shouldYieldForZeal(ZealMode::IncrementalMultipleSlices)) {
-        return NotFinished;
-    }
-
-    return Finished;
-}
-
+    bool yield = useZeal && isIncremental && hasZealMode(mode);
+
+    // Only yield on the first sweep slice for this mode.
+    bool firstSweepSlice = initialState != State::Sweep;
+    if (mode == ZealMode::IncrementalMultipleSlices && !firstSweepSlice) {
+        yield = false;
+    }
+
+    return yield;
+}
 #endif
 
 IncrementalProgress
 GCRuntime::endSweepingSweepGroup(FreeOp* fop, SliceBudget& budget)
 {
     {
         gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::FINALIZE_END);
         FreeOp fop(rt);
@@ -6684,64 +6675,72 @@ class SweepActionCall final : public Swe
   public:
     explicit SweepActionCall(Method m) : method(m) {}
     IncrementalProgress run(GCRuntime* gc, Args... args) override {
         return (gc->*method)(args...);
     }
     void assertFinished() const override { }
 };
 
-#ifdef JS_GC_ZEAL
 // Implementation of the SweepAction interface that yields in a specified zeal
-// mode and then calls another action.
+// mode.
 template <typename... Args>
 class SweepActionMaybeYield final : public SweepAction<GCRuntime*, Args...>
 {
-    using Action = SweepAction<GCRuntime*, Args...>;
-
     ZealMode mode;
-    UniquePtr<Action> action;
-    bool triggered;
+    bool isYielding;
 
   public:
-    SweepActionMaybeYield(UniquePtr<Action> action, ZealMode mode)
-      : mode(mode), action(std::move(action)), triggered(false) {}
+    explicit SweepActionMaybeYield(ZealMode mode)
+      : mode(mode), isYielding(false) {}
 
     IncrementalProgress run(GCRuntime* gc, Args... args) override {
-        if (!triggered && gc->shouldYieldForZeal(mode)) {
-            triggered = true;
+#ifdef JS_GC_ZEAL
+        if (!isYielding && gc->shouldYieldForZeal(mode)) {
+            isYielding = true;
             return NotFinished;
         }
 
-        triggered = false;
-        return action->run(gc, args...);
+        isYielding = false;
+#endif
+        return Finished;
     }
 
     void assertFinished() const override {
-        MOZ_ASSERT(!triggered);
-    }
+        MOZ_ASSERT(!isYielding);
+    }
+
+    // These actions should be skipped if GC zeal is not configured.
+#ifndef JS_GC_ZEAL
+    bool shouldSkip() override {
+        return true;
+    }
+#endif
 };
-#endif
 
 // Implementation of the SweepAction interface that calls a list of actions in
 // sequence.
 template <typename... Args>
 class SweepActionSequence final : public SweepAction<Args...>
 {
     using Action = SweepAction<Args...>;
     using ActionVector = Vector<UniquePtr<Action>, 0, SystemAllocPolicy>;
     using Iter = IncrementalIter<ContainerIter<ActionVector>>;
 
     ActionVector actions;
     typename Iter::State iterState;
 
   public:
     bool init(UniquePtr<Action>* acts, size_t count) {
         for (size_t i = 0; i < count; i++) {
-            if (!actions.emplaceBack(std::move(acts[i]))) {
+            auto& action = acts[i];
+            if (action->shouldSkip()) {
+                continue;
+            }
+            if (!actions.emplaceBack(std::move(action))) {
                 return false;
             }
         }
         return true;
     }
 
     IncrementalProgress run(Args... args) override {
         for (Iter iter(iterState, actions); !iter.done(); iter.next()) {
@@ -6860,24 +6859,24 @@ class RemoveLastTemplateParameter<Target
 };
 
 template <typename... Args>
 static UniquePtr<SweepAction<GCRuntime*, Args...>>
 Call(IncrementalProgress (GCRuntime::*method)(Args...)) {
    return MakeUnique<SweepActionCall<Args...>>(method);
 }
 
-template <typename... Args>
-static UniquePtr<SweepAction<GCRuntime*, Args...>>
-MaybeYield(ZealMode zealMode, UniquePtr<SweepAction<GCRuntime*, Args...>> action) {
-#ifdef JS_GC_ZEAL
-    return js::MakeUnique<SweepActionMaybeYield<Args...>>(std::move(action), zealMode);
-#else
-    return action;
-#endif
+static UniquePtr<SweepAction<GCRuntime*, FreeOp*, SliceBudget&>>
+MaybeYield(ZealMode zealMode) {
+    return js::MakeUnique<SweepActionMaybeYield<FreeOp*, SliceBudget&>>(zealMode);
+}
+
+static UniquePtr<SweepAction<GCRuntime*, FreeOp*, SliceBudget&, Zone*>>
+MaybeYieldInZoneLoop(ZealMode zealMode) {
+    return js::MakeUnique<SweepActionMaybeYield<FreeOp*, SliceBudget&, Zone*>>(zealMode);
 }
 
 template <typename... Args, typename... Rest>
 static UniquePtr<SweepAction<Args...>>
 Sequence(UniquePtr<SweepAction<Args...>> first, Rest... rest)
 {
     UniquePtr<SweepAction<Args...>> actions[] = { std::move(first), std::move(rest)... };
     auto seq = MakeUnique<SweepActionSequence<Args...>>();
@@ -6934,35 +6933,33 @@ GCRuntime::initSweepActions()
     using namespace sweepaction;
     using sweepaction::Call;
 
     sweepActions.ref() =
         RepeatForSweepGroup(rt,
             Sequence(
                 Call(&GCRuntime::endMarkingSweepGroup),
                 Call(&GCRuntime::beginSweepingSweepGroup),
-#ifdef JS_GC_ZEAL
-                Call(&GCRuntime::maybeYieldForSweepingZeal),
-#endif
-                MaybeYield(ZealMode::YieldBeforeSweepingAtoms,
-                           Call(&GCRuntime::sweepAtomsTable)),
-                MaybeYield(ZealMode::YieldBeforeSweepingCaches,
-                           Call(&GCRuntime::sweepWeakCaches)),
+                MaybeYield(ZealMode::IncrementalMultipleSlices),
+                MaybeYield(ZealMode::YieldBeforeSweepingAtoms),
+                Call(&GCRuntime::sweepAtomsTable),
+                MaybeYield(ZealMode::YieldBeforeSweepingCaches),
+                Call(&GCRuntime::sweepWeakCaches),
                 ForEachZoneInSweepGroup(rt,
                     Sequence(
-                        MaybeYield(ZealMode::YieldBeforeSweepingTypes,
-                                   Call(&GCRuntime::sweepTypeInformation)),
-                        MaybeYield(ZealMode::YieldBeforeSweepingObjects,
-                                   ForEachAllocKind(ForegroundObjectFinalizePhase.kinds,
-                                                    Call(&GCRuntime::finalizeAllocKind))),
-                        MaybeYield(ZealMode::YieldBeforeSweepingNonObjects,
-                                   ForEachAllocKind(ForegroundNonObjectFinalizePhase.kinds,
-                                                    Call(&GCRuntime::finalizeAllocKind))),
-                        MaybeYield(ZealMode::YieldBeforeSweepingShapeTrees,
-                                   Call(&GCRuntime::sweepShapeTree)),
+                        MaybeYieldInZoneLoop(ZealMode::YieldBeforeSweepingTypes),
+                        Call(&GCRuntime::sweepTypeInformation),
+                        MaybeYieldInZoneLoop(ZealMode::YieldBeforeSweepingObjects),
+                        ForEachAllocKind(ForegroundObjectFinalizePhase.kinds,
+                                         Call(&GCRuntime::finalizeAllocKind)),
+                        MaybeYieldInZoneLoop(ZealMode::YieldBeforeSweepingNonObjects),
+                        ForEachAllocKind(ForegroundNonObjectFinalizePhase.kinds,
+                                         Call(&GCRuntime::finalizeAllocKind)),
+                        MaybeYieldInZoneLoop(ZealMode::YieldBeforeSweepingShapeTrees),
+                        Call(&GCRuntime::sweepShapeTree),
                         Call(&GCRuntime::releaseSweptEmptyArenas))),
                 Call(&GCRuntime::endSweepingSweepGroup)));
 
     return sweepActions != nullptr;
 }
 
 IncrementalProgress
 GCRuntime::performSweepActions(SliceBudget& budget)
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -59,16 +59,17 @@ enum IncrementalProgress
 // types are not deduced but come ultimately from the type of a function pointer
 // passed to SweepFunc.
 template <typename... Args>
 struct SweepAction
 {
     virtual ~SweepAction() {}
     virtual IncrementalProgress run(Args... args) = 0;
     virtual void assertFinished() const = 0;
+    virtual bool shouldSkip() { return false; }
 };
 
 class ChunkPool
 {
     Chunk* head_;
     size_t count_;
 
   public:
@@ -626,19 +627,16 @@ class GCRuntime
 
     void beginSweepPhase(JS::gcreason::Reason reason, AutoGCSession& session);
     void groupZonesForSweeping(JS::gcreason::Reason reason);
     MOZ_MUST_USE bool findInterZoneEdges();
     void getNextSweepGroup();
     IncrementalProgress endMarkingSweepGroup(FreeOp* fop, SliceBudget& budget);
     void markIncomingCrossCompartmentPointers(MarkColor color);
     IncrementalProgress beginSweepingSweepGroup(FreeOp* fop, SliceBudget& budget);
-#ifdef JS_GC_ZEAL
-    IncrementalProgress maybeYieldForSweepingZeal(FreeOp* fop, SliceBudget& budget);
-#endif
     bool shouldReleaseObservedTypes();
     void sweepDebuggerOnMainThread(FreeOp* fop);
     void sweepJitDataOnMainThread(FreeOp* fop);
     IncrementalProgress endSweepingSweepGroup(FreeOp* fop, SliceBudget& budget);
     IncrementalProgress performSweepActions(SliceBudget& sliceBudget);
     IncrementalProgress sweepTypeInformation(FreeOp* fop, SliceBudget& budget, Zone* zone);
     IncrementalProgress releaseSweptEmptyArenas(FreeOp* fop, SliceBudget& budget, Zone* zone);
     void startSweepingAtomsTable();