Bug 1398140 - Remove Ion helper thread pausing mechanism. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 11 Sep 2017 16:53:38 +0200
changeset 429568 ab8c75e0d4220a3218c79e963a3ffc455ba05419
parent 429567 1ef55343a4e988b27633479a8a8f8d9dac2a233b
child 429569 2c284ff4354aa075579a85d4dd13ca1bf7d41f86
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1398140
milestone57.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 1398140 - Remove Ion helper thread pausing mechanism. r=luke
js/src/jit/MIRGenerator.h
js/src/jit/MIRGraph.cpp
js/src/vm/HelperThreads.cpp
js/src/vm/HelperThreads.h
js/src/vm/TraceLoggingTypes.h
--- a/js/src/jit/MIRGenerator.h
+++ b/js/src/jit/MIRGenerator.h
@@ -113,31 +113,22 @@ class MIRGenerator
         return safeForMinorGC_;
     }
     void setNotSafeForMinorGC() {
         safeForMinorGC_ = false;
     }
 
     // Whether the active thread is trying to cancel this build.
     bool shouldCancel(const char* why) {
-        maybePause();
         return cancelBuild_;
     }
     void cancel() {
         cancelBuild_ = true;
     }
 
-    void maybePause() {
-        if (pauseBuild_ && *pauseBuild_)
-            PauseCurrentHelperThread();
-    }
-    void setPauseFlag(mozilla::Atomic<bool, mozilla::Relaxed>* pauseBuild) {
-        pauseBuild_ = pauseBuild;
-    }
-
     bool compilingWasm() const {
         return info_->compilingWasm();
     }
 
     uint32_t wasmMaxStackArgBytes() const {
         MOZ_ASSERT(compilingWasm());
         return wasmMaxStackArgBytes_;
     }
@@ -186,17 +177,16 @@ class MIRGenerator
 
   protected:
     const CompileInfo* info_;
     const OptimizationInfo* optimizationInfo_;
     TempAllocator* alloc_;
     MIRGraph* graph_;
     AbortReasonOr<Ok> offThreadStatus_;
     ObjectGroupVector abortedPreliminaryGroups_;
-    mozilla::Atomic<bool, mozilla::Relaxed>* pauseBuild_;
     mozilla::Atomic<bool, mozilla::Relaxed> cancelBuild_;
 
     uint32_t wasmMaxStackArgBytes_;
     bool needsOverrecursedCheck_;
     bool needsStaticStackAlignment_;
     bool usesSimd_;
     bool cachedUsesSimd_;
 
--- a/js/src/jit/MIRGraph.cpp
+++ b/js/src/jit/MIRGraph.cpp
@@ -23,17 +23,16 @@ MIRGenerator::MIRGenerator(CompileCompar
   : compartment(compartment),
     runtime(compartment ? compartment->runtime() : nullptr),
     info_(info),
     optimizationInfo_(optimizationInfo),
     alloc_(alloc),
     graph_(graph),
     offThreadStatus_(Ok()),
     abortedPreliminaryGroups_(*alloc_),
-    pauseBuild_(nullptr),
     cancelBuild_(false),
     wasmMaxStackArgBytes_(0),
     needsOverrecursedCheck_(false),
     needsStaticStackAlignment_(false),
     usesSimd_(false),
     cachedUsesSimd_(false),
     modifiesFrameArguments_(false),
     instrumentedProfiling_(false),
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -278,31 +278,24 @@ CancelOffThreadIonCompileLocked(const Co
             HelperThreadState().remove(worklist, &i);
         }
     }
 
     /* Wait for in progress entries to finish up. */
     bool cancelled;
     do {
         cancelled = false;
-        bool unpaused = false;
         for (auto& helper : *HelperThreadState().threads) {
             if (helper.ionBuilder() &&
                 IonBuilderMatches(selector, helper.ionBuilder()))
             {
                 helper.ionBuilder()->cancel();
-                if (helper.pause) {
-                    helper.pause = false;
-                    unpaused = true;
-                }
                 cancelled = true;
             }
         }
-        if (unpaused)
-            HelperThreadState().notifyAll(GlobalHelperThreadState::PAUSE, lock);
         if (cancelled)
             HelperThreadState().wait(lock, GlobalHelperThreadState::CONSUMER);
     } while (cancelled);
 
     /* Cancel code generation for any completed entries. */
     GlobalHelperThreadState::IonBuilderVector& finished = HelperThreadState().ionFinishedList(lock);
     for (size_t i = 0; i < finished.length(); i++) {
         jit::IonBuilder* builder = finished[i];
@@ -1155,22 +1148,16 @@ size_t
 GlobalHelperThreadState::maxIonCompilationThreads() const
 {
     if (IsHelperThreadSimulatingOOM(js::THREAD_TYPE_ION))
         return 1;
     return threadCount;
 }
 
 size_t
-GlobalHelperThreadState::maxUnpausedIonCompilationThreads() const
-{
-    return 1;
-}
-
-size_t
 GlobalHelperThreadState::maxWasmCompilationThreads() const
 {
     if (IsHelperThreadSimulatingOOM(js::THREAD_TYPE_WASM))
         return 1;
     return cpuCount;
 }
 
 size_t
@@ -1271,16 +1258,18 @@ bool
 GlobalHelperThreadState::canStartPromiseHelperTask(const AutoLockHelperThreadState& lock)
 {
     return !promiseHelperTasks(lock).empty();
 }
 
 static bool
 IonBuilderHasHigherPriority(jit::IonBuilder* first, jit::IonBuilder* second)
 {
+    // Return true if priority(first) > priority(second).
+    //
     // This method can return whatever it wants, though it really ought to be a
     // total order. The ordering is allowed to race (change on the fly), however.
 
     // A lower optimization level indicates a higher priority.
     if (first->optimizationInfo().level() != second->optimizationInfo().level())
         return first->optimizationInfo().level() < second->optimizationInfo().level();
 
     // A script without an IonScript has precedence on one with.
@@ -1301,109 +1290,33 @@ GlobalHelperThreadState::canStartIonComp
 
 bool
 GlobalHelperThreadState::canStartIonFreeTask(const AutoLockHelperThreadState& lock)
 {
     return !ionFreeList(lock).empty();
 }
 
 jit::IonBuilder*
-GlobalHelperThreadState::highestPriorityPendingIonCompile(const AutoLockHelperThreadState& lock,
-                                                          bool remove /* = false */)
+GlobalHelperThreadState::highestPriorityPendingIonCompile(const AutoLockHelperThreadState& lock)
 {
     auto& worklist = ionWorklist(lock);
-    if (worklist.empty()) {
-        MOZ_ASSERT(!remove);
-        return nullptr;
-    }
+    MOZ_ASSERT(!worklist.empty());
 
     // Get the highest priority IonBuilder which has not started compilation yet.
     size_t index = 0;
     for (size_t i = 1; i < worklist.length(); i++) {
         if (IonBuilderHasHigherPriority(worklist[i], worklist[index]))
             index = i;
     }
+
     jit::IonBuilder* builder = worklist[index];
-    if (remove)
-        worklist.erase(&worklist[index]);
+    worklist.erase(&worklist[index]);
     return builder;
 }
 
-HelperThread*
-GlobalHelperThreadState::lowestPriorityUnpausedIonCompileAtThreshold(
-    const AutoLockHelperThreadState& lock)
-{
-    // Get the lowest priority IonBuilder which has started compilation and
-    // isn't paused, unless there are still fewer than the maximum number of
-    // such builders permitted.
-    size_t numBuilderThreads = 0;
-    HelperThread* thread = nullptr;
-    for (auto& thisThread : *threads) {
-        if (thisThread.ionBuilder() && !thisThread.pause) {
-            numBuilderThreads++;
-            if (!thread ||
-                IonBuilderHasHigherPriority(thread->ionBuilder(), thisThread.ionBuilder()))
-            {
-                thread = &thisThread;
-            }
-        }
-    }
-    if (numBuilderThreads < maxUnpausedIonCompilationThreads())
-        return nullptr;
-    return thread;
-}
-
-HelperThread*
-GlobalHelperThreadState::highestPriorityPausedIonCompile(const AutoLockHelperThreadState& lock)
-{
-    // Get the highest priority IonBuilder which has started compilation but
-    // which was subsequently paused.
-    HelperThread* thread = nullptr;
-    for (auto& thisThread : *threads) {
-        if (thisThread.pause) {
-            // Currently, only threads with IonBuilders can be paused.
-            MOZ_ASSERT(thisThread.ionBuilder());
-            if (!thread ||
-                IonBuilderHasHigherPriority(thisThread.ionBuilder(), thread->ionBuilder()))
-            {
-                thread = &thisThread;
-            }
-        }
-    }
-    return thread;
-}
-
-bool
-GlobalHelperThreadState::pendingIonCompileHasSufficientPriority(
-    const AutoLockHelperThreadState& lock)
-{
-    // Can't compile anything if there are no scripts to compile.
-    if (!canStartIonCompile(lock))
-        return false;
-
-    // Count the number of threads currently compiling scripts, and look for
-    // the thread with the lowest priority.
-    HelperThread* lowestPriorityThread = lowestPriorityUnpausedIonCompileAtThreshold(lock);
-
-    // If the number of threads building scripts is less than the maximum, the
-    // compilation can start immediately.
-    if (!lowestPriorityThread)
-        return true;
-
-    // If there is a builder in the worklist with higher priority than some
-    // builder currently being compiled, then that current compilation can be
-    // paused, so allow the compilation.
-    if (IonBuilderHasHigherPriority(highestPriorityPendingIonCompile(lock),
-                                    lowestPriorityThread->ionBuilder()))
-        return true;
-
-    // Compilation will have to wait until one of the active compilations finishes.
-    return false;
-}
-
 bool
 GlobalHelperThreadState::canStartParseTask(const AutoLockHelperThreadState& lock)
 {
     // Parse tasks that end up compiling asm.js in turn may use Wasm compilation
     // threads to generate machine code.  We have no way (at present) to know
     // ahead of time whether a parse task is going to parse asm.js content or
     // not, so we just assume that all parse tasks are master tasks.
     return !parseWorklist(lock).empty() &&
@@ -1936,32 +1849,19 @@ HelperThread::handlePromiseHelperTaskWor
 void
 HelperThread::handleIonWorkload(AutoLockHelperThreadState& locked)
 {
     MOZ_ASSERT(HelperThreadState().canStartIonCompile(locked));
     MOZ_ASSERT(idle());
 
     // Find the IonBuilder in the worklist with the highest priority, and
     // remove it from the worklist.
-    jit::IonBuilder* builder =
-        HelperThreadState().highestPriorityPendingIonCompile(locked, /* remove = */ true);
-
-    // If there are now too many threads with active IonBuilders, indicate to
-    // the one with the lowest priority that it should pause. Note that due to
-    // builder priorities changing since pendingIonCompileHasSufficientPriority
-    // was called, the builder we are pausing may actually be higher priority
-    // than the one we are about to start. Oh well.
-    HelperThread* other = HelperThreadState().lowestPriorityUnpausedIonCompileAtThreshold(locked);
-    if (other) {
-        MOZ_ASSERT(other->ionBuilder() && !other->pause);
-        other->pause = true;
-    }
+    jit::IonBuilder* builder = HelperThreadState().highestPriorityPendingIonCompile(locked);
 
     currentTask.emplace(builder);
-    builder->setPauseFlag(&pause);
 
     JSRuntime* rt = builder->script()->compartment()->runtimeFromAnyThread();
 
     {
         AutoUnlockHelperThreadState unlock(locked);
 
         TraceLoggerThread* logger = TraceLoggerForCurrentThread();
         TraceLoggerEvent event(TraceLogger_AnnotateScripts, builder->script());
@@ -1987,42 +1887,19 @@ HelperThread::handleIonWorkload(AutoLock
     // cancels in progress Ion compilations before destroying its target
     // context, and after we reset the current task we are no longer considered
     // to be Ion compiling.
     JSContext* target = builder->script()->zoneFromAnyThread()->group()->ownerContext().context();
     if (target)
         target->requestInterrupt(JSContext::RequestInterruptCanWait);
 
     currentTask.reset();
-    pause = false;
 
     // Notify the active thread in case it is waiting for the compilation to finish.
     HelperThreadState().notifyAll(GlobalHelperThreadState::CONSUMER, locked);
-
-    // When finishing Ion compilation jobs, we can start unpausing compilation
-    // threads that were paused to restrict the number of active compilations.
-    // Only unpause one at a time, to make sure we don't exceed the restriction.
-    // Since threads are currently only paused for Ion compilations, this
-    // strategy will eventually unpause all paused threads, regardless of how
-    // many there are, since each thread we unpause will eventually finish and
-    // end up back here.
-    if (HelperThread* other = HelperThreadState().highestPriorityPausedIonCompile(locked)) {
-        MOZ_ASSERT(other->ionBuilder() && other->pause);
-
-        // Only unpause the other thread if there isn't a higher priority
-        // builder which this thread or another can start on.
-        jit::IonBuilder* builder = HelperThreadState().highestPriorityPendingIonCompile(locked);
-        if (!builder || IonBuilderHasHigherPriority(other->ionBuilder(), builder)) {
-            other->pause = false;
-
-            // Notify all paused threads, to make sure the one we just
-            // unpaused wakes up.
-            HelperThreadState().notifyAll(GlobalHelperThreadState::PAUSE, locked);
-        }
-    }
 }
 
 void
 HelperThread::handleIonFreeWorkload(AutoLockHelperThreadState& locked)
 {
     MOZ_ASSERT(idle());
     MOZ_ASSERT(HelperThreadState().canStartIonFreeTask(locked));
 
@@ -2043,30 +1920,16 @@ js::CurrentHelperThread()
     auto threadId = ThisThread::GetId();
     for (auto& thisThread : *HelperThreadState().threads) {
         if (thisThread.thread.isSome() && threadId == thisThread.thread->get_id())
             return &thisThread;
     }
     return nullptr;
 }
 
-void
-js::PauseCurrentHelperThread()
-{
-    TraceLoggerThread* logger = TraceLoggerForCurrentThread();
-    AutoTraceLog logPaused(logger, TraceLogger_IonCompilationPaused);
-
-    HelperThread* thread = CurrentHelperThread();
-    MOZ_ASSERT(thread);
-
-    AutoLockHelperThreadState lock;
-    while (thread->pause)
-        HelperThreadState().wait(lock, GlobalHelperThreadState::PAUSE);
-}
-
 bool
 JSContext::addPendingCompileError(js::CompileError** error)
 {
     auto errorPtr = make_unique<js::CompileError>();
     if (!errorPtr)
         return false;
     if (!helperThread()->parseTask()->errors.append(errorPtr.get())) {
         ReportOutOfMemory(this);
@@ -2340,17 +2203,17 @@ HelperThread::threadLoop()
             // tasks not being added (because of the lifo structure of the work
             // lists).  Unlocking the HelperThreadState between task selection
             // and execution is not well-defined.
 
             if (HelperThreadState().canStartGCParallelTask(lock)) {
                 task = js::THREAD_TYPE_GCPARALLEL;
             } else if (HelperThreadState().canStartGCHelperTask(lock)) {
                 task = js::THREAD_TYPE_GCHELPER;
-            } else if (HelperThreadState().pendingIonCompileHasSufficientPriority(lock)) {
+            } else if (HelperThreadState().canStartIonCompile(lock)) {
                 task = js::THREAD_TYPE_ION;
             } else if (HelperThreadState().canStartWasmCompile(lock, wasm::CompileMode::Tier1)) {
                 task = js::THREAD_TYPE_WASM;
                 tier = wasm::CompileMode::Tier1;
             } else if (HelperThreadState().canStartPromiseHelperTask(lock)) {
                 task = js::THREAD_TYPE_PROMISE_TASK;
             } else if (HelperThreadState().canStartParseTask(lock)) {
                 task = js::THREAD_TYPE_PARSE;
--- a/js/src/vm/HelperThreads.h
+++ b/js/src/vm/HelperThreads.h
@@ -138,17 +138,16 @@ class GlobalHelperThreadState
 
     // GC tasks needing to be done in parallel.
     GCParallelTaskVector gcParallelWorklist_;
 
     ParseTask* removeFinishedParseTask(ParseTaskKind kind, void* token);
 
   public:
     size_t maxIonCompilationThreads() const;
-    size_t maxUnpausedIonCompilationThreads() const;
     size_t maxWasmCompilationThreads() const;
     size_t maxWasmTier2GeneratorThreads() const;
     size_t maxParseThreads() const;
     size_t maxCompressionThreads() const;
     size_t maxGCHelperThreads() const;
     size_t maxGCParallelThreads() const;
 
     GlobalHelperThreadState();
@@ -168,20 +167,16 @@ class GlobalHelperThreadState
         // progress, ie, a work item has been completed by a helper thread and
         // the thread that created the work item can now consume it.
         CONSUMER,
 
         // For notifying helper threads doing the work that they may be able to
         // make progress, ie, a work item has been enqueued and an idle helper
         // thread may pick up up the work item and perform it.
         PRODUCER,
-
-        // For notifying threads doing work which are paused that they may be
-        // able to resume making progress.
-        PAUSE
     };
 
     void wait(AutoLockHelperThreadState& locked, CondVar which,
               mozilla::TimeDuration timeout = mozilla::TimeDuration::Forever());
     void notifyAll(CondVar which, const AutoLockHelperThreadState&);
     void notifyOne(CondVar which, const AutoLockHelperThreadState&);
 
     // Helper method for removing items from the vectors below while iterating over them.
@@ -275,26 +270,17 @@ class GlobalHelperThreadState
 
     // Used by a major GC to signal processing enqueued compression tasks.
     void startHandlingCompressionTasks(const AutoLockHelperThreadState&);
 
   private:
     void scheduleCompressionTasks(const AutoLockHelperThreadState&);
 
   public:
-    // Unlike the public methods above, the value returned by this method can
-    // change over time, even if the helper thread state lock is held
-    // throughout.
-    bool pendingIonCompileHasSufficientPriority(const AutoLockHelperThreadState& lock);
-
-    jit::IonBuilder* highestPriorityPendingIonCompile(const AutoLockHelperThreadState& lock,
-                                                      bool remove = false);
-    HelperThread* lowestPriorityUnpausedIonCompileAtThreshold(
-        const AutoLockHelperThreadState& lock);
-    HelperThread* highestPriorityPausedIonCompile(const AutoLockHelperThreadState& lock);
+    jit::IonBuilder* highestPriorityPendingIonCompile(const AutoLockHelperThreadState& lock);
 
     template <
         typename F,
         typename = typename mozilla::EnableIf<
             // Matches when the type is a function or lambda with the signature `bool(ParseTask*)`
             mozilla::IsSame<bool, decltype((*(F*)nullptr)((ParseTask*)nullptr))>::value
         >::Type
     >
@@ -330,23 +316,21 @@ class GlobalHelperThreadState
      * Lock protecting all mutable shared state accessed by helper threads, and
      * used by all condition variables.
      */
     js::Mutex helperLock;
 
     /* Condvars for threads waiting/notifying each other. */
     js::ConditionVariable consumerWakeup;
     js::ConditionVariable producerWakeup;
-    js::ConditionVariable pauseWakeup;
 
     js::ConditionVariable& whichWakeup(CondVar which) {
         switch (which) {
           case CONSUMER: return consumerWakeup;
           case PRODUCER: return producerWakeup;
-          case PAUSE: return pauseWakeup;
           default: MOZ_CRASH("Invalid CondVar in |whichWakeup|");
         }
     }
 };
 
 static inline GlobalHelperThreadState&
 HelperThreadState()
 {
@@ -371,23 +355,16 @@ struct HelperThread
     mozilla::Maybe<Thread> thread;
 
     /*
      * Indicate to a thread that it should terminate itself. This is only read
      * or written with the helper thread state lock held.
      */
     bool terminate;
 
-    /*
-     * Indicate to a thread that it should pause execution. This is only
-     * written with the helper thread state lock held, but may be read from
-     * without the lock held.
-     */
-    mozilla::Atomic<bool, mozilla::Relaxed> pause;
-
     /* The current task being executed by this thread, if any. */
     mozilla::Maybe<HelperTaskUnion> currentTask;
 
     bool idle() const {
         return currentTask.isNothing();
     }
 
     /* Any builder currently being compiled by Ion on this thread. */
@@ -467,20 +444,16 @@ EnsureHelperThreadsInitialized();
 // --thread-count=N option.
 void
 SetFakeCPUCount(size_t count);
 
 // Get the current helper thread, or null.
 HelperThread*
 CurrentHelperThread();
 
-// Pause the current thread until it's pause flag is unset.
-void
-PauseCurrentHelperThread();
-
 // Enqueues a wasm compilation task.
 bool
 StartOffThreadWasmCompile(wasm::CompileTask* task, wasm::CompileMode mode);
 
 namespace wasm {
 
 // Called on a helper thread after StartOffThreadWasmCompile.
 void
--- a/js/src/vm/TraceLoggingTypes.h
+++ b/js/src/vm/TraceLoggingTypes.h
@@ -18,17 +18,16 @@
     _(Engine)                                         \
     _(GC)                                             \
     _(GCAllocation)                                   \
     _(GCSweeping)                                     \
     _(Interpreter)                                    \
     _(InlinedScripts)                                 \
     _(IonAnalysis)                                    \
     _(IonCompilation)                                 \
-    _(IonCompilationPaused)                           \
     _(IonLinking)                                     \
     _(IonMonkey)                                      \
     _(IrregexpCompile)                                \
     _(IrregexpExecute)                                \
     _(MinorGC)                                        \
     _(Frontend)                                       \
     _(ParsingFull)                                    \
     _(ParsingSyntax)                                  \