diff --git a/js/public/HelperThreadAPI.h b/js/public/HelperThreadAPI.h index c877fbb49b86..bbb5c0517951 100644 --- a/js/public/HelperThreadAPI.h +++ b/js/public/HelperThreadAPI.h @@ -18,22 +18,25 @@ namespace JS { +class HelperThreadTask; + // Argument passed to the task callback to indicate whether we're invoking it // because a new task was added by the JS engine or because we're on a helper // thread that just finished a task and there are other tasks pending. enum class DispatchReason { NewTask, FinishedTask }; /** - * Set callback to dispatch a tasks to an external thread pool. + * Set callback to dispatch a task to an external thread pool. * - * When the task runs it should call JS::RunHelperThreadTask. + * When the task runs it should call JS::RunHelperThreadTask passing |task|. */ -using HelperThreadTaskCallback = void (*)(DispatchReason reason); +using HelperThreadTaskCallback = void (*)(HelperThreadTask* task, + DispatchReason reason); extern JS_PUBLIC_API void SetHelperThreadTaskCallback( HelperThreadTaskCallback callback, size_t threadCount, size_t stackSize); // Function to call from external thread pool to run a helper thread task. -extern JS_PUBLIC_API void RunHelperThreadTask(); +extern JS_PUBLIC_API void RunHelperThreadTask(HelperThreadTask* task); } // namespace JS diff --git a/js/src/vm/HelperThreadState.h b/js/src/vm/HelperThreadState.h index 8bd8b797b458..72d181affd0b 100644 --- a/js/src/vm/HelperThreadState.h +++ b/js/src/vm/HelperThreadState.h @@ -181,9 +181,11 @@ class GlobalHelperThreadState { JS::HelperThreadTaskCallback dispatchTaskCallback = nullptr; friend class AutoHelperTaskQueue; +#ifdef DEBUG // The number of tasks dispatched to the thread pool that have not started // running yet. size_t tasksPending_ = 0; +#endif bool isInitialized_ = false; @@ -417,7 +419,7 @@ class GlobalHelperThreadState { bool submitTask(PromiseHelperTask* task); bool submitTask(GCParallelTask* task, const AutoLockHelperThreadState& locked); - void runOneTask(AutoLockHelperThreadState& lock); + void runOneTask(HelperThreadTask* task, AutoLockHelperThreadState& lock); void runTaskLocked(HelperThreadTask* task, AutoLockHelperThreadState& lock); using Selector = HelperThreadTask* ( diff --git a/js/src/vm/HelperThreadTask.h b/js/src/vm/HelperThreadTask.h index d27e53033ddc..75e66be638e9 100644 --- a/js/src/vm/HelperThreadTask.h +++ b/js/src/vm/HelperThreadTask.h @@ -54,14 +54,19 @@ struct MapTypeToThreadType { static const ThreadType threadType = THREAD_TYPE_COMPRESS; }; -struct HelperThreadTask { - virtual void runHelperThreadTask(AutoLockHelperThreadState& locked) = 0; - virtual ThreadType threadType() = 0; +} // namespace js + +namespace JS { + +class HelperThreadTask { + public: + virtual void runHelperThreadTask(js::AutoLockHelperThreadState& locked) = 0; + virtual js::ThreadType threadType() = 0; virtual ~HelperThreadTask() = default; template bool is() { - return MapTypeToThreadType::threadType == threadType(); + return js::MapTypeToThreadType::threadType == threadType(); } template @@ -71,6 +76,10 @@ struct HelperThreadTask { } }; +} // namespace JS + +namespace js { +using JS::HelperThreadTask; } // namespace js #endif /* vm_HelperThreadTask_h */ diff --git a/js/src/vm/HelperThreads.cpp b/js/src/vm/HelperThreads.cpp index d50c9a134a87..0d43587df7c4 100644 --- a/js/src/vm/HelperThreads.cpp +++ b/js/src/vm/HelperThreads.cpp @@ -439,6 +439,7 @@ void js::CancelOffThreadIonCompile(const CompilationSelector& selector) { jit::IonCompileTask* ionCompileTask = helper->as(); if (IonCompileTaskMatches(selector, ionCompileTask)) { + ionCompileTask->alloc().lifoAlloc()->setReadWrite(); ionCompileTask->mirGen().cancel(); cancelled = true; } @@ -891,14 +892,26 @@ void GlobalHelperThreadState::assertIsLockedByCurrentThread() const { void GlobalHelperThreadState::dispatch(DispatchReason reason, const AutoLockHelperThreadState& lock) { - if (canStartTasks(lock) && tasksPending_ < threadCount) { - // This doesn't guarantee that we don't dispatch more tasks to the external - // pool than necessary if tasks are taking a long time to start, but it does - // limit the number. - tasksPending_++; - - lock.queueTaskToDispatch(reason); + if (helperTasks_.length() >= threadCount) { + return; } + + HelperThreadTask* task = findHighestPriorityTask(lock); + if (!task) { + return; + } + +#ifdef DEBUG + MOZ_ASSERT(tasksPending_ < threadCount); + tasksPending_++; +#endif + + // Add task to list of running tasks immediately. + helperTasks(lock).infallibleEmplaceBack(task); + runningTaskCount[task->threadType()]++; + totalCountRunningTasks++; + + lock.queueTaskToDispatch(task, reason); } void GlobalHelperThreadState::wait( @@ -936,10 +949,11 @@ void GlobalHelperThreadState::waitForAllTasksLocked( AutoLockHelperThreadState& lock) { CancelOffThreadWasmTier2GeneratorLocked(lock); - while (canStartTasks(lock) || tasksPending_ || hasActiveThreads(lock)) { + while (canStartTasks(lock) || hasActiveThreads(lock)) { wait(lock); } + MOZ_ASSERT(tasksPending_ == 0); MOZ_ASSERT(gcParallelWorklist().isEmpty(lock)); MOZ_ASSERT(ionWorklist(lock).empty()); MOZ_ASSERT(wasmWorklist(lock, wasm::CompileMode::Tier1).empty()); @@ -1631,7 +1645,9 @@ void GlobalHelperThreadState::trace(JSTracer* trc) { for (auto* helper : HelperThreadState().helperTasks(lock)) { if (helper->is()) { - helper->as()->trace(trc); + jit::IonCompileTask* ionCompileTask = helper->as(); + ionCompileTask->alloc().lifoAlloc()->setReadWrite(); + ionCompileTask->trace(trc); } } } @@ -1674,7 +1690,8 @@ bool GlobalHelperThreadState::canStartTasks( canStartWasmTier2GeneratorTask(lock); } -void JS::RunHelperThreadTask() { +void JS::RunHelperThreadTask(HelperThreadTask* task) { + MOZ_ASSERT(task); MOZ_ASSERT(CanUseExtraThreads()); AutoLockHelperThreadState lock; @@ -1683,24 +1700,21 @@ void JS::RunHelperThreadTask() { return; } - HelperThreadState().runOneTask(lock); + HelperThreadState().runOneTask(task, lock); } -void GlobalHelperThreadState::runOneTask(AutoLockHelperThreadState& lock) { +void GlobalHelperThreadState::runOneTask(HelperThreadTask* task, + AutoLockHelperThreadState& lock) { +#ifdef DEBUG MOZ_ASSERT(tasksPending_ > 0); tasksPending_--; +#endif - // The selectors may depend on the HelperThreadState not changing between task - // selection and task execution, in particular, on new 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. - HelperThreadTask* task = findHighestPriorityTask(lock); - if (task) { - runTaskLocked(task, lock); - dispatch(DispatchReason::FinishedTask, lock); - } + runTaskLocked(task, lock); notifyAll(lock); + + dispatch(DispatchReason::FinishedTask, lock); } HelperThreadTask* GlobalHelperThreadState::findHighestPriorityTask( @@ -1716,49 +1730,62 @@ HelperThreadTask* GlobalHelperThreadState::findHighestPriorityTask( return nullptr; } -void GlobalHelperThreadState::runTaskLocked(HelperThreadTask* task, - AutoLockHelperThreadState& locked) { - JS::AutoSuppressGCAnalysis nogc; - - HelperThreadState().helperTasks(locked).infallibleEmplaceBack(task); - - ThreadType threadType = task->threadType(); - js::oom::SetThreadType(threadType); - runningTaskCount[threadType]++; - totalCountRunningTasks++; - - task->runHelperThreadTask(locked); - - // Delete task from helperTasks. - HelperThreadState().helperTasks(locked).eraseIfEqual(task); - - totalCountRunningTasks--; - runningTaskCount[threadType]--; - - js::oom::SetThreadType(js::THREAD_TYPE_NONE); -} - -void AutoHelperTaskQueue::queueTaskToDispatch(JS::DispatchReason reason) const { - // This is marked const because it doesn't release the mutex. - - if (reason == JS::DispatchReason::FinishedTask) { - finishedTasksToDispatch++; - return; +#ifdef DEBUG +static bool VectorHasTask( + const Vector& tasks, + HelperThreadTask* task) { + for (HelperThreadTask* t : tasks) { + if (t == task) { + return true; + } } - newTasksToDispatch++; + return false; +} +#endif + +void GlobalHelperThreadState::runTaskLocked(HelperThreadTask* task, + AutoLockHelperThreadState& locked) { + ThreadType threadType = task->threadType(); + + MOZ_ASSERT(VectorHasTask(helperTasks(locked), task)); + MOZ_ASSERT(totalCountRunningTasks != 0); + MOZ_ASSERT(runningTaskCount[threadType] != 0); + + js::oom::SetThreadType(threadType); + + { + JS::AutoSuppressGCAnalysis nogc; + task->runHelperThreadTask(locked); + } + + js::oom::SetThreadType(js::THREAD_TYPE_NONE); + + helperTasks(locked).eraseIfEqual(task); + totalCountRunningTasks--; + runningTaskCount[threadType]--; +} + +void AutoHelperTaskQueue::queueTaskToDispatch(JS::HelperThreadTask* task, + JS::DispatchReason reason) const { + // This is marked const because it doesn't release the mutex. + + AutoEnterOOMUnsafeRegion oomUnsafe; + if (!tasksToDispatch.append(task) || !dispatchReasons.append(reason)) { + oomUnsafe.crash("AutoLockHelperThreadState::queueTaskToDispatch"); + } } void AutoHelperTaskQueue::dispatchQueuedTasks() { + MOZ_ASSERT(tasksToDispatch.length() == dispatchReasons.length()); + // The hazard analysis can't tell that the callback doesn't GC. JS::AutoSuppressGCAnalysis nogc; - for (size_t i = 0; i < newTasksToDispatch; i++) { - HelperThreadState().dispatchTaskCallback(JS::DispatchReason::NewTask); + for (size_t i = 0; i < tasksToDispatch.length(); i++) { + HelperThreadState().dispatchTaskCallback(tasksToDispatch[i], + dispatchReasons[i]); } - for (size_t i = 0; i < finishedTasksToDispatch; i++) { - HelperThreadState().dispatchTaskCallback(JS::DispatchReason::FinishedTask); - } - newTasksToDispatch = 0; - finishedTasksToDispatch = 0; + tasksToDispatch.clear(); + dispatchReasons.clear(); } diff --git a/js/src/vm/HelperThreads.h b/js/src/vm/HelperThreads.h index 3827168f98f9..61307f9dfab1 100644 --- a/js/src/vm/HelperThreads.h +++ b/js/src/vm/HelperThreads.h @@ -73,15 +73,15 @@ extern Mutex gHelperThreadLock MOZ_UNANNOTATED; class AutoHelperTaskQueue { public: ~AutoHelperTaskQueue() { dispatchQueuedTasks(); } - bool hasQueuedTasks() const { - return newTasksToDispatch || finishedTasksToDispatch; - } - void queueTaskToDispatch(JS::DispatchReason reason) const; + bool hasQueuedTasks() const { return !tasksToDispatch.empty(); } + void queueTaskToDispatch(JS::HelperThreadTask* task, + JS::DispatchReason reason) const; void dispatchQueuedTasks(); private: - mutable uint32_t newTasksToDispatch = 0; - mutable uint32_t finishedTasksToDispatch = 0; + // TODO: Convert this to use a linked list. + mutable Vector tasksToDispatch; + mutable Vector dispatchReasons; }; // A lock guard for data protected by the helper thread lock. diff --git a/js/src/vm/InternalThreadPool.cpp b/js/src/vm/InternalThreadPool.cpp index c6cdac1d4447..9a5a26235662 100644 --- a/js/src/vm/InternalThreadPool.cpp +++ b/js/src/vm/InternalThreadPool.cpp @@ -137,7 +137,7 @@ bool InternalThreadPool::ensureThreadCount(size_t threadCount, threads(lock).infallibleEmplaceBack(std::move(thread)); } - return true; + return tasks_.ref().reserve(threadCount); } size_t InternalThreadPool::threadCount(const AutoLockHelperThreadState& lock) { @@ -174,6 +174,11 @@ inline const HelperThreadVector& InternalThreadPool::threads( return threads_.ref(); } +inline HelperTaskVector& InternalThreadPool::tasks( + const AutoLockHelperThreadState& lock) { + return tasks_.ref(); +} + size_t InternalThreadPool::sizeOfIncludingThis( mozilla::MallocSizeOf mallocSizeOf, const AutoLockHelperThreadState& lock) const { @@ -182,21 +187,24 @@ size_t InternalThreadPool::sizeOfIncludingThis( } /* static */ -void InternalThreadPool::DispatchTask(JS::DispatchReason reason) { - Get().dispatchTask(reason); +void InternalThreadPool::DispatchTask(HelperThreadTask* task, + JS::DispatchReason reason) { + Get().dispatchTask(task, reason); } -void InternalThreadPool::dispatchTask(JS::DispatchReason reason) { +void InternalThreadPool::dispatchTask(HelperThreadTask* task, + JS::DispatchReason reason) { // This could now use a separate mutex like TaskController, but continues to // use the helper thread state lock for convenience. AutoLockHelperThreadState lock; - queuedTasks++; + tasks_.ref().infallibleAppend(task); + if (reason == JS::DispatchReason::NewTask) { wakeup.notify_one(); } else { // We're called from a helper thread right before returning to - // HelperThread::threadLoop. There we will check queuedTasks so there's no + // HelperThread::threadLoop. There we will check tasks_ so there's no // need to wake up any threads. MOZ_ASSERT(reason == JS::DispatchReason::FinishedTask); MOZ_ASSERT(!TlsContext.get(), "we should be on a helper thread"); @@ -280,11 +288,16 @@ void HelperThread::threadLoop(InternalThreadPool* pool) { AutoLockHelperThreadState lock; while (!pool->terminating) { - if (pool->queuedTasks != 0) { - pool->queuedTasks--; + HelperTaskVector& tasks = pool->tasks(lock); + if (!tasks.empty()) { + // TODO: Add a test mode that introduces a delay before starting tasks or + // starts them in a different order. + HelperThreadTask** taskp = tasks.begin(); + HelperThreadTask* task = *taskp; + tasks.erase(taskp); AutoUnlockHelperThreadState unlock(lock); - JS::RunHelperThreadTask(); + JS::RunHelperThreadTask(task); continue; } diff --git a/js/src/vm/InternalThreadPool.h b/js/src/vm/InternalThreadPool.h index 1d5c6d1a4316..5c6e07e1cfcb 100644 --- a/js/src/vm/InternalThreadPool.h +++ b/js/src/vm/InternalThreadPool.h @@ -20,16 +20,20 @@ namespace JS { enum class DispatchReason; +class HelperThreadTask; }; namespace js { class AutoLockHelperThreadState; class HelperThread; +using JS::HelperThreadTask; using HelperThreadVector = Vector, 0, SystemAllocPolicy>; +using HelperTaskVector = Vector; + class InternalThreadPool { public: static bool Initialize(size_t threadCount, AutoLockHelperThreadState& lock); @@ -45,15 +49,17 @@ class InternalThreadPool { const AutoLockHelperThreadState& lock) const; private: - static void DispatchTask(JS::DispatchReason reason); + static void DispatchTask(HelperThreadTask* task, JS::DispatchReason reason); - void dispatchTask(JS::DispatchReason reason); + void dispatchTask(HelperThreadTask* task, JS::DispatchReason reason); void shutDown(AutoLockHelperThreadState& lock); HelperThreadVector& threads(const AutoLockHelperThreadState& lock); const HelperThreadVector& threads( const AutoLockHelperThreadState& lock) const; + HelperTaskVector& tasks(const AutoLockHelperThreadState& lock); + void notifyAll(const AutoLockHelperThreadState& lock); void wait(AutoLockHelperThreadState& lock); friend class HelperThread; @@ -62,9 +68,9 @@ class InternalThreadPool { HelperThreadLockData threads_; - js::ConditionVariable wakeup; + HelperThreadLockData tasks_; - HelperThreadLockData queuedTasks; + js::ConditionVariable wakeup; HelperThreadLockData terminating; }; diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index 086f6657160e..e98f9243c7e5 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -1117,16 +1117,20 @@ CycleCollectedJSRuntime* XPCJSContext::CreateRuntime(JSContext* aCx) { } class HelperThreadTaskHandler : public Task { + JS::HelperThreadTask* mTask; + public: - TaskResult Run() override { - JS::RunHelperThreadTask(); - return TaskResult::Complete; - } - explicit HelperThreadTaskHandler() - : Task(Kind::OffMainThreadOnly, EventQueuePriority::Normal) { + explicit HelperThreadTaskHandler(JS::HelperThreadTask* aTask) + : Task(Kind::OffMainThreadOnly, EventQueuePriority::Normal), + mTask(aTask) { // Bug 1703185: Currently all tasks are run at the same priority. } + TaskResult Run() override { + JS::RunHelperThreadTask(mTask); + return TaskResult::Complete; + } + #ifdef MOZ_COLLECTING_RUNNABLE_TELEMETRY bool GetName(nsACString& aName) override { aName.AssignLiteral("HelperThreadTask"); @@ -1138,8 +1142,9 @@ class HelperThreadTaskHandler : public Task { ~HelperThreadTaskHandler() = default; }; -static void DispatchOffThreadTask(JS::DispatchReason) { - TaskController::Get()->AddTask(MakeAndAddRef()); +static void DispatchOffThreadTask(JS::HelperThreadTask* aTask, + JS::DispatchReason) { + TaskController::Get()->AddTask(MakeAndAddRef(aTask)); } static bool CreateSelfHostedSharedMemory(JSContext* aCx,