Issue #3092 - Refactor WASM compilation handling

js/src/vm/HelperThreads.cpp

@@ -49,10 +49,6 @@ js::CreateHelperThreadsState()
 {
     MOZ_ASSERT(!gHelperThreadState);
     gHelperThreadState = js_new<GlobalHelperThreadState>();
-    return gHelperThreadState != nullptr;
-}
-
-void
 js::DestroyHelperThreadsState()
 {
     MOZ_ASSERT(gHelperThreadState);
@@ -477,14 +473,17 @@ js::CancelOffThreadParses(JSRuntime* rt)
         HelperThreadState().wait(lock, GlobalHelperThreadState::CONSUMER);
     }
 
+                    return gHelperThreadState != nullptr;
+                }
+
+                void
+                js::DestroyHelperThreadsState()
+                {
+                    MOZ_ASSERT(gHelperThreadState);
+                    gHelperThreadState->finish();
+                    js_delete(gHelperThreadState);
+                    gHelperThreadState = nullptr;
     // Clean up any parse tasks which haven't been finished by the main thread.
-    GlobalHelperThreadState::ParseTaskVector& finished = HelperThreadState().parseFinishedList(lock);
-    while (true) {
-        bool found = false;
-        for (size_t i = 0; i < finished.length(); i++) {
-            ParseTask* task = finished[i];
-            if (task->runtimeMatches(rt)) {
-                found = true;
                 AutoUnlockHelperThreadState unlock(lock);
                 HelperThreadState().cancelParseTask(rt->contextFromMainThread(), task->kind, task);
             }
@@ -972,8 +971,7 @@ GlobalHelperThreadState::maxGCParallelThreads() const
 bool
 GlobalHelperThreadState::canStartWasmCompile(const AutoLockHelperThreadState& lock)
 {
-    // Don't execute an wasm job if an earlier one failed.
-    if (wasmWorklist(lock).empty() || numWasmFailedJobs)
+    if (wasmWorklist(lock).empty())
         return false;
 
     // Honor the maximum allowed threads to compile wasm jobs at once,
@@ -1423,13 +1421,13 @@ HelperThread::handleWasmWorkload(AutoLockHelperThreadState& locked)
         success = wasm::CompileFunction(task);
     }
 
-    // On success, try to move work to the finished list.
-    if (success)
-        success = HelperThreadState().wasmFinishedList(locked).append(task);
-
-    // On failure, note the failure for harvesting by the parent.
+    // Append the task to the finished queue owned by its module generator.
     if (!success)
-        HelperThreadState().noteWasmFailure(locked);
+        task->setFailed();
+
+    AutoEnterOOMUnsafeRegion oomUnsafe;
+    if (!task->finishedList()->append(task))
+        oomUnsafe.crash("HelperThread::handleWasmWorkload");
 
     // Notify the main thread in case it's waiting.
     HelperThreadState().notifyAll(GlobalHelperThreadState::CONSUMER, locked);

js/src/vm/HelperThreads.h

@@ -89,8 +89,8 @@ class GlobalHelperThreadState
     wasm::IonCompileTaskPtrVector wasmWorklist_, wasmFinishedList_;
 
   public:
-    // For now, only allow a single parallel wasm compilation to happen at a
-    // time. This avoids race conditions on wasmWorklist/wasmFinishedList/etc.
+    // Helper-thread initiated wasm compilations are serialized to avoid the
+    // deadlock scenario described in WasmGenerator.cpp.
     mozilla::Atomic<bool> wasmCompilationInProgress;
 
   private:

js/src/wasm/WasmGenerator.cpp

@@ -54,6 +54,7 @@ ModuleGenerator::ModuleGenerator(ImportVector&& imports)
     lastPatchedCallsite_(0),
     startOfUnpatchedCallsites_(0),
     parallel_(false),
+    parallelCompilationInProgressOnHelperThread_(false),
     outstanding_(0),
     activeFuncDef_(nullptr),
     startedFuncDefs_(false),
@@ -71,18 +72,21 @@ ModuleGenerator::~ModuleGenerator()
             AutoLockHelperThreadState lock;
             while (true) {
                 IonCompileTaskPtrVector& worklist = HelperThreadState().wasmWorklist(lock);
-                MOZ_ASSERT(outstanding_ >= worklist.length());
-                outstanding_ -= worklist.length();
-                worklist.clear();
+                for (size_t i = worklist.length(); i > 0;) {
+                    if (worklist[i - 1]->finishedList() == &finishedTasks_) {
+                        HelperThreadState().remove(worklist, &i);
+                        MOZ_ASSERT(outstanding_ > 0);
+                        outstanding_--;
+                    } else {
+                        i--;
+                    }
+                }
 
-                IonCompileTaskPtrVector& finished = HelperThreadState().wasmFinishedList(lock);
-                MOZ_ASSERT(outstanding_ >= finished.length());
-                outstanding_ -= finished.length();
-                finished.clear();
-
-                uint32_t numFailed = HelperThreadState().harvestFailedWasmJobs(lock);
-                MOZ_ASSERT(outstanding_ >= numFailed);
-                outstanding_ -= numFailed;
+                for (size_t i = finishedTasks_.length(); i > 0;) {
+                    HelperThreadState().remove(finishedTasks_, &i);
+                    MOZ_ASSERT(outstanding_ > 0);
+                    outstanding_--;
+                }
 
                 if (!outstanding_)
                     break;
@@ -91,8 +95,10 @@ ModuleGenerator::~ModuleGenerator()
             }
         }
 
-        MOZ_ASSERT(HelperThreadState().wasmCompilationInProgress);
-        HelperThreadState().wasmCompilationInProgress = false;
+        if (parallelCompilationInProgressOnHelperThread_) {
+            MOZ_ASSERT(HelperThreadState().wasmCompilationInProgress);
+            HelperThreadState().wasmCompilationInProgress = false;
+        }
     } else {
         MOZ_ASSERT(!outstanding_);
     }
@@ -208,12 +214,9 @@ ModuleGenerator::finishOutstandingTask()
         while (true) {
             MOZ_ASSERT(outstanding_ > 0);
 
-            if (HelperThreadState().wasmFailed(lock))
-                return false;
-
-            if (!HelperThreadState().wasmFinishedList(lock).empty()) {
+            if (!finishedTasks_.empty()) {
                 outstanding_--;
-                task = HelperThreadState().wasmFinishedList(lock).popCopy();
+                task = finishedTasks_.popCopy();
                 break;
             }
 
@@ -221,6 +224,9 @@ ModuleGenerator::finishOutstandingTask()
         }
     }
 
+    if (task->failed())
+        return false;
+
     return finishTask(task);
 }
 
@@ -365,6 +371,9 @@ ModuleGenerator::patchFarJumps(const TrapExitOffsetArray& trapExits)
 bool
 ModuleGenerator::finishTask(IonCompileTask* task)
 {
+    if (task->failed())
+        return false;
+
     const FuncBytes& func = task->func();
     FuncCompileResults& results = task->results();
 
@@ -856,31 +865,25 @@ ModuleGenerator::startFuncDefs()
     MOZ_ASSERT(!startedFuncDefs_);
     MOZ_ASSERT(!finishedFuncDefs_);
 
-    // The wasmCompilationInProgress atomic ensures that there is only one
-    // 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.
+    // Helper-thread initiated wasm compilations stay serialized so that we do
+    // not end up with multiple helper threads blocking on other helper threads.
+    // Main-thread compilations can still overlap because they drain their own
+    // finished-task queue and do not steal tasks from other generators.
 
     GlobalHelperThreadState& threads = HelperThreadState();
     MOZ_ASSERT(threads.threadCount > 1);
 
     uint32_t numTasks;
-    if (CanUseExtraThreads() && threads.wasmCompilationInProgress.compareExchange(false, true)) {
+    if (CanUseExtraThreads() && (!CurrentHelperThread() ||
+        threads.wasmCompilationInProgress.compareExchange(false, true))) {
 #ifdef DEBUG
         {
             AutoLockHelperThreadState lock;
-            MOZ_ASSERT(!HelperThreadState().wasmFailed(lock));
             MOZ_ASSERT(HelperThreadState().wasmWorklist(lock).empty());
-            MOZ_ASSERT(HelperThreadState().wasmFinishedList(lock).empty());
         }
 #endif
         parallel_ = true;
+        parallelCompilationInProgressOnHelperThread_ = !!CurrentHelperThread();
         numTasks = 2 * threads.maxWasmCompilationThreads();
     } else {
         numTasks = 1;
@@ -891,6 +894,9 @@ ModuleGenerator::startFuncDefs()
     for (size_t i = 0; i < numTasks; i++)
         tasks_.infallibleEmplaceBack(*shared_, COMPILATION_LIFO_DEFAULT_CHUNK_SIZE);
 
+    for (auto& task : tasks_)
+        task.setFinishedList(&finishedTasks_);
+
     if (!freeTasks_.reserve(numTasks))
         return false;
     for (size_t i = 0; i < numTasks; i++)

js/src/wasm/WasmGenerator.h

@@ -104,9 +104,11 @@ class MOZ_STACK_CLASS ModuleGenerator
 
     // Parallel compilation
     bool                            parallel_;
+    bool                            parallelCompilationInProgressOnHelperThread_;
     uint32_t                        outstanding_;
     IonCompileTaskVector            tasks_;
     IonCompileTaskPtrVector         freeTasks_;
+    IonCompileTaskPtrVector         finishedTasks_;
 
     // Assertions
     DebugOnly<FunctionGenerator*>   activeFuncDef_;

js/src/wasm/WasmIonCompile.h

@@ -19,6 +19,7 @@
 #define wasm_ion_compile_h
 
 #include "jit/MacroAssembler.h"
+#include "vm/HelperThreads.h"
 #include "wasm/WasmTypes.h"
 
 namespace js {
@@ -106,6 +107,8 @@ class IonCompileTask
     UniqueFuncBytes            func_;
     CompileMode                mode_;
     Maybe<FuncCompileResults>  results_;
+    IonCompileTaskPtrVector*   finishedList_;
+    bool                       failed_;
 
     IonCompileTask(const IonCompileTask&) = delete;
     IonCompileTask& operator=(const IonCompileTask&) = delete;
@@ -113,7 +116,26 @@ class IonCompileTask
   public:
     IonCompileTask(const ModuleGeneratorData& mg, size_t defaultChunkSize)
       : mg_(mg), lifo_(defaultChunkSize), func_(nullptr), mode_(CompileMode::None)
+      , finishedList_(nullptr)
+      , failed_(false)
     {}
+
+    void setFinishedList(IonCompileTaskPtrVector* finishedList) {
+        finishedList_ = finishedList;
+    }
+
+    IonCompileTaskPtrVector* finishedList() const {
+        MOZ_ASSERT(finishedList_);
+        return finishedList_;
+    }
+
+    void setFailed() {
+        failed_ = true;
+    }
+
+    bool failed() const {
+        return failed_;
+    }
     LifoAlloc& lifo() {
         return lifo_;
     }
@@ -143,6 +165,7 @@ class IonCompileTask
         results_.reset();
         lifo_.releaseAll();
         mode_ = CompileMode::None;
+        failed_ = false;
     }
 };