diff --git a/js/src/vm/HelperThreads.cpp b/js/src/vm/HelperThreads.cpp index 5880e3f50..d4b4df2e2 100644 --- a/js/src/vm/HelperThreads.cpp +++ b/js/src/vm/HelperThreads.cpp @@ -20,7 +20,6 @@ #include "vm/SharedImmutableStringsCache.h" #include "vm/Time.h" #include "vm/TraceLogging.h" -#include "wasm/WasmGenerator.h" #include "jscntxtinlines.h" #include "jscompartmentinlines.h" @@ -474,8 +473,7 @@ js::CancelOffThreadParses(JSRuntime* rt) } // Clean up any parse tasks which haven't been finished by the main thread. - GlobalHelperThreadState::ParseTaskVector& finished = - HelperThreadState().parseFinishedList(lock); + GlobalHelperThreadState::ParseTaskVector& finished = HelperThreadState().parseFinishedList(lock); while (true) { bool found = false; for (size_t i = 0; i < finished.length(); i++) { @@ -483,8 +481,7 @@ js::CancelOffThreadParses(JSRuntime* rt) if (task->runtimeMatches(rt)) { found = true; AutoUnlockHelperThreadState unlock(lock); - HelperThreadState().cancelParseTask(rt->contextFromMainThread(), - task->kind, task); + HelperThreadState().cancelParseTask(rt->contextFromMainThread(), task->kind, task); } } if (!found) @@ -964,7 +961,8 @@ GlobalHelperThreadState::maxGCParallelThreads() const bool GlobalHelperThreadState::canStartWasmCompile(const AutoLockHelperThreadState& lock) { - if (wasmWorklist(lock).empty()) + // Don't execute an wasm job if an earlier one failed. + if (wasmWorklist(lock).empty() || numWasmFailedJobs) return false; // Honor the maximum allowed threads to compile wasm jobs at once, @@ -1416,13 +1414,15 @@ HelperThread::handleWasmWorkload(AutoLockHelperThreadState& locked) success = wasm::CompileFunction(task, &error); } - // Append the task to the finished queue owned by its module generator. - if (!success) - task->setFailed(); + // On success, try to move work to the finished list. + if (success) + success = HelperThreadState().wasmFinishedList(locked).append(task); - AutoEnterOOMUnsafeRegion oomUnsafe; - if (!task->finishedList()->append(task)) - oomUnsafe.crash("HelperThread::handleWasmWorkload"); + // On failure, note the failure for harvesting by the parent. + if (!success) { + HelperThreadState().noteWasmFailure(locked); + HelperThreadState().setWasmError(locked, Move(error)); + } // Notify the main thread in case it's waiting. HelperThreadState().notifyAll(GlobalHelperThreadState::CONSUMER, locked); diff --git a/js/src/vm/HelperThreads.h b/js/src/vm/HelperThreads.h index afaf21e4e..1cbd01a65 100644 --- a/js/src/vm/HelperThreads.h +++ b/js/src/vm/HelperThreads.h @@ -90,8 +90,8 @@ class GlobalHelperThreadState wasm::CompileTaskPtrVector wasmWorklist_, wasmFinishedList_; public: - // Helper-thread initiated wasm compilations are serialized to avoid the - // deadlock scenario described in WasmGenerator.cpp. + // For now, only allow a single parallel wasm compilation to happen at a + // time. This avoids race conditions on wasmWorklist/wasmFinishedList/etc. mozilla::Atomic wasmCompilationInProgress; private: diff --git a/js/src/wasm/WasmGenerator.cpp b/js/src/wasm/WasmGenerator.cpp index 04c636d1f..ad30903e5 100644 --- a/js/src/wasm/WasmGenerator.cpp +++ b/js/src/wasm/WasmGenerator.cpp @@ -55,7 +55,6 @@ ModuleGenerator::ModuleGenerator(UniqueChars* error) lastPatchedCallsite_(0), startOfUnpatchedCallsites_(0), parallel_(false), - parallelCompilationInProgressOnHelperThread_(false), outstanding_(0), currentTask_(nullptr), batchedBytecode_(0), @@ -75,21 +74,18 @@ ModuleGenerator::~ModuleGenerator() AutoLockHelperThreadState lock; while (true) { CompileTaskPtrVector& worklist = HelperThreadState().wasmWorklist(lock); - 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--; - } - } + MOZ_ASSERT(outstanding_ >= worklist.length()); + outstanding_ -= worklist.length(); + worklist.clear(); - for (size_t i = finishedTasks_.length(); i > 0;) { - HelperThreadState().remove(finishedTasks_, &i); - MOZ_ASSERT(outstanding_ > 0); - outstanding_--; - } + CompileTaskPtrVector& 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; if (!outstanding_) break; @@ -98,10 +94,8 @@ ModuleGenerator::~ModuleGenerator() } } - if (parallelCompilationInProgressOnHelperThread_) { - MOZ_ASSERT(HelperThreadState().wasmCompilationInProgress); - HelperThreadState().wasmCompilationInProgress = false; - } + MOZ_ASSERT(HelperThreadState().wasmCompilationInProgress); + HelperThreadState().wasmCompilationInProgress = false; } else { MOZ_ASSERT(!outstanding_); } @@ -258,9 +252,17 @@ ModuleGenerator::finishOutstandingTask() while (true) { MOZ_ASSERT(outstanding_ > 0); - if (!finishedTasks_.empty()) { + if (HelperThreadState().wasmFailed(lock)) { + if (error_) { + MOZ_ASSERT(!*error_, "Should have stopped earlier"); + *error_ = Move(HelperThreadState().harvestWasmError(lock)); + } + return false; + } + + if (!HelperThreadState().wasmFinishedList(lock).empty()) { outstanding_--; - task = finishedTasks_.popCopy(); + task = HelperThreadState().wasmFinishedList(lock).popCopy(); break; } @@ -268,9 +270,6 @@ ModuleGenerator::finishOutstandingTask() } } - if (task->failed()) - return false; - return finishTask(task); } @@ -436,9 +435,6 @@ ModuleGenerator::patchFarJumps(const TrapExitOffsetArray& trapExits, const Offse bool ModuleGenerator::finishTask(CompileTask* task) { - if (task->failed()) - return false; - masm_.haltingAlign(CodeAlignment); // Before merging in the new function's code, if calls in a prior function @@ -869,25 +865,34 @@ ModuleGenerator::startFuncDefs() MOZ_ASSERT(!startedFuncDefs_); MOZ_ASSERT(!finishedFuncDefs_); - // 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. + // 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. GlobalHelperThreadState& threads = HelperThreadState(); MOZ_ASSERT(threads.threadCount > 1); uint32_t numTasks; - if (CanUseExtraThreads() && (!CurrentHelperThread() || - threads.wasmCompilationInProgress.compareExchange(false, true))) { + if (CanUseExtraThreads() && + threads.cpuCount > 1 && + 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; @@ -898,9 +903,6 @@ ModuleGenerator::startFuncDefs() for (size_t i = 0; i < numTasks; i++) tasks_.infallibleEmplaceBack(*env_, compileMode_, 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++) diff --git a/js/src/wasm/WasmGenerator.h b/js/src/wasm/WasmGenerator.h index cc592abc8..88d44ddc0 100644 --- a/js/src/wasm/WasmGenerator.h +++ b/js/src/wasm/WasmGenerator.h @@ -142,8 +142,6 @@ class CompileTask Maybe masm_; FuncCompileUnitVector units_; bool debugEnabled_; - CompileTaskPtrVector* finishedList_; - bool failed_; CompileTask(const CompileTask&) = delete; CompileTask& operator=(const CompileTask&) = delete; @@ -158,28 +156,10 @@ class CompileTask CompileTask(const ModuleEnvironment& env, CompileMode mode, size_t defaultChunkSize) : env_(env), mode_(mode), - lifo_(defaultChunkSize), - finishedList_(nullptr), - failed_(false) + lifo_(defaultChunkSize) { init(); } - void setFinishedList(CompileTaskPtrVector* finishedList) { - finishedList_ = finishedList; - } - - CompileTaskPtrVector* finishedList() const { - MOZ_ASSERT(finishedList_); - return finishedList_; - } - - void setFailed() { - failed_ = true; - } - - bool failed() const { - return failed_; - } LifoAlloc& lifo() { return lifo_; } @@ -214,7 +194,6 @@ class CompileTask masm_.reset(); alloc_.reset(); lifo_.releaseAll(); - failed_ = false; init(); return true; @@ -259,11 +238,9 @@ class MOZ_STACK_CLASS ModuleGenerator // Parallel compilation bool parallel_; - bool parallelCompilationInProgressOnHelperThread_; uint32_t outstanding_; CompileTaskVector tasks_; CompileTaskPtrVector freeTasks_; - CompileTaskPtrVector finishedTasks_; UniqueFuncBytesVector freeFuncBytes_; CompileTask* currentTask_; uint32_t batchedBytecode_;