Bug 1373414 - Backout wasm work stealing on JS helper threads. r=shu

--HG--
extra : rebase_source : 8b1dbd5f5885d41c281da137496ac5e268f4029c
extra : source : 55393d7d4f5d8a0d6258cc3035f418a05ba40aea

js/src/vm/HelperThreads.cpp

@@ -1057,16 +1057,12 @@ GlobalHelperThreadState::maxGCParallelThreads() const
 }
 
 bool
-GlobalHelperThreadState::canStartWasmCompile(const AutoLockHelperThreadState& lock,
-                                             bool assumeThreadAvailable)
+GlobalHelperThreadState::canStartWasmCompile(const AutoLockHelperThreadState& lock)
 {
     // Don't execute an wasm job if an earlier one failed.
     if (wasmWorklist(lock).empty() || numWasmFailedJobs)
         return false;
 
-    if (assumeThreadAvailable)
-        return true;
-
     // Honor the maximum allowed threads to compile wasm jobs at once,
     // to avoid oversaturating the machine.
     if (!checkTaskThreadLimit<wasm::CompileTask*>(maxWasmCompilationThreads()))
@@ -1668,9 +1664,9 @@ HelperThread::ThreadMain(void* arg)
 }
 
 void
-HelperThread::handleWasmWorkload(AutoLockHelperThreadState& locked, bool assumeThreadAvailable)
+HelperThread::handleWasmWorkload(AutoLockHelperThreadState& locked)
 {
-    MOZ_ASSERT(HelperThreadState().canStartWasmCompile(locked, assumeThreadAvailable));
+    MOZ_ASSERT(HelperThreadState().canStartWasmCompile(locked));
     MOZ_ASSERT(idle());
 
     currentTask.emplace(HelperThreadState().wasmWorklist(locked).popCopy());
@@ -1698,33 +1694,6 @@ HelperThread::handleWasmWorkload(AutoLockHelperThreadState& locked, bool assumeT
     currentTask.reset();
 }
 
-bool
-HelperThread::handleWasmIdleWorkload(AutoLockHelperThreadState& locked)
-{
-    // Perform wasm compilation work on a HelperThread that is running
-    // ModuleGenerator instead of blocking while other compilation threads
-    // finish.  This removes a source of deadlocks, as putting all threads to
-    // work guarantees forward progress for compilation.
-
-    // The current thread has already been accounted for, so don't guard on
-    // thread subscription when checking whether we can do work.
-
-    if (HelperThreadState().canStartWasmCompile(locked, /*assumeThreadAvailable=*/ true)) {
-        HelperTaskUnion oldTask = currentTask.value();
-        currentTask.reset();
-        js::oom::ThreadType oldType = (js::oom::ThreadType)js::oom::GetThreadType();
-
-        js::oom::SetThreadType(js::oom::THREAD_TYPE_WASM);
-        handleWasmWorkload(locked, /*assumeThreadAvailable=*/ true);
-
-        js::oom::SetThreadType(oldType);
-        currentTask.emplace(oldTask);
-        return true;
-    }
-
-    return false;
-}
-
 void
 HelperThread::handlePromiseTaskWorkload(AutoLockHelperThreadState& locked)
 {

js/src/vm/HelperThreads.h

@@ -224,8 +224,7 @@ class GlobalHelperThreadState
         return gcParallelWorklist_;
     }
 
-    bool canStartWasmCompile(const AutoLockHelperThreadState& lock,
-                             bool assumeThreadAvailable = false);
+    bool canStartWasmCompile(const AutoLockHelperThreadState& lock);
     bool canStartPromiseTask(const AutoLockHelperThreadState& lock);
     bool canStartIonCompile(const AutoLockHelperThreadState& lock);
     bool canStartIonFreeTask(const AutoLockHelperThreadState& lock);
@@ -388,13 +387,6 @@ struct HelperThread
         return maybeCurrentTaskAs<wasm::CompileTask*>();
     }
 
-    /*
-     * Perform wasm compilation work on behalf of a thread that is running a
-     * wasm ModuleGenerator and would otherwise block waiting for other
-     * compilation threads. Return true if work was performed, otherwise false.
-     */
-    bool handleWasmIdleWorkload(AutoLockHelperThreadState& locked);
-
     /* Any source being parsed/emitted on this thread. */
     ParseTask* parseTask() {
         return maybeCurrentTaskAs<ParseTask*>();
@@ -429,7 +421,7 @@ struct HelperThread
         return nullptr;
     }
 
-    void handleWasmWorkload(AutoLockHelperThreadState& locked, bool assumeThreadAvailable = false);
+    void handleWasmWorkload(AutoLockHelperThreadState& locked);
     void handlePromiseTaskWorkload(AutoLockHelperThreadState& locked);
     void handleIonWorkload(AutoLockHelperThreadState& locked);
     void handleIonFreeWorkload(AutoLockHelperThreadState& locked);

js/src/wasm/WasmGenerator.cpp

@@ -61,7 +61,6 @@ ModuleGenerator::ModuleGenerator(UniqueChars* error)
     outstanding_(0),
     currentTask_(nullptr),
     batchedBytecode_(0),
-    maybeHelperThread_(CurrentHelperThread()),
     activeFuncDef_(nullptr),
     startedFuncDefs_(false),
     finishedFuncDefs_(false),
@@ -298,17 +297,7 @@ ModuleGenerator::finishOutstandingTask()
                 break;
             }
 
-            // If we're on a helper thread then attempt to do some work.  This
-            // avoids a source of deadlocks, as putting all threads to work
-            // guarantees forward progress for compilation.
-
-            if (!maybeHelperThread_ || !maybeHelperThread_->handleWasmIdleWorkload(lock)) {
-                // Other threads are working on the remaining task(s) and will
-                // notify us when that work is complete.
-                MOZ_ASSERT(outstanding_ > 0);
-                MOZ_ASSERT_IF(maybeHelperThread_, HelperThreadState().wasmWorklist(lock).empty());
-                HelperThreadState().wait(lock, GlobalHelperThreadState::CONSUMER);
-            }
+            HelperThreadState().wait(lock, GlobalHelperThreadState::CONSUMER);
         }
     }
 
@@ -883,17 +872,18 @@ ModuleGenerator::startFuncDefs()
     MOZ_ASSERT(!finishedFuncDefs_);
 
     // The wasmCompilationInProgress atomic ensures that there is only one
-    // parallel compilation in progress at a time.  This restriction can be
-    // lifted, but it will be somewhat desirable to keep it when tiered
-    // compilation is introduced (and tier-2 code for one module can be compiled
-    // in parallel with the tier-1 code for the next module).
-    //
-    // Deadlocks are avoided by guaranteeing the forward progress of
-    // compilation, in particular, by having the ModuleGenerator thread (in the
-    // case of asm.js and future tiered compilation) make progress on
-    // compilation and not simply wait for other helper threads to finish.
+    // parallel compilation in progress at a time. In the special case of
+    // asm.js, where the ModuleGenerator itself can be on a helper thread, this
+    // avoids the possibility of deadlock since at most 1 helper thread will be
+    // blocking on other helper threads and there are always >1 helper threads.
+    // With wasm, this restriction could be relaxed by moving the worklist state
+    // out of HelperThreadState since each independent compilation needs its own
+    // worklist pair. Alternatively, the deadlock could be avoided by having the
+    // ModuleGenerator thread make progress (on compile tasks) instead of
+    // blocking.
 
     GlobalHelperThreadState& threads = HelperThreadState();
+    MOZ_ASSERT(threads.threadCount > 1);
 
     uint32_t numTasks;
     if (CanUseExtraThreads() &&

js/src/wasm/WasmGenerator.h

@@ -244,7 +244,6 @@ class MOZ_STACK_CLASS ModuleGenerator
     UniqueFuncBytesVector           freeFuncBytes_;
     CompileTask*                    currentTask_;
     uint32_t                        batchedBytecode_;
-    HelperThread*                   maybeHelperThread_;
 
     // Assertions
     DebugOnly<FunctionGenerator*>   activeFuncDef_;