From 10438c20290cae51dcfecd71a7ce3919d1c1e1c1 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Tue, 16 Apr 2024 06:17:54 +0300 Subject: [PATCH] Backed out 4 changesets (bug 1888429) for causing xpc failures @ tools/profiler/tests/xpcshell/test_enterjit_osr_enabling.js CLOSED TREE Backed out changeset 4c5efaca4057 (bug 1888429) Backed out changeset 8b30cb54d5d9 (bug 1888429) Backed out changeset c82b988fab50 (bug 1888429) Backed out changeset b64446651983 (bug 1888429) --- js/public/HelperThreadAPI.h | 13 +- js/src/gc/GCParallelTask.cpp | 20 ++- js/src/gc/ParallelMarking.cpp | 16 +-- js/src/vm/HelperThreadState.h | 84 +++++------ js/src/vm/HelperThreadTask.h | 17 +-- js/src/vm/HelperThreads.cpp | 286 ++++++++++++++++++-------------------- js/src/vm/InternalThreadPool.cpp | 94 ++++--------- js/src/vm/InternalThreadPool.h | 21 ++- js/xpconnect/src/XPCJSContext.cpp | 18 +-- 9 files changed, 244 insertions(+), 325 deletions(-) diff --git a/js/public/HelperThreadAPI.h b/js/public/HelperThreadAPI.h index 646da69925e0..c877fbb49b86 100644 --- a/js/public/HelperThreadAPI.h +++ b/js/public/HelperThreadAPI.h @@ -18,19 +18,22 @@ 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 task to an external thread pool. + * Set callback to dispatch a tasks to an external thread pool. * - * When the task runs it should call JS::RunHelperThreadTask passing |task|. + * When the task runs it should call JS::RunHelperThreadTask. */ -using HelperThreadTaskCallback = void (*)(HelperThreadTask* task); +using HelperThreadTaskCallback = void (*)(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(HelperThreadTask* task); +extern JS_PUBLIC_API void RunHelperThreadTask(); } // namespace JS diff --git a/js/src/gc/GCParallelTask.cpp b/js/src/gc/GCParallelTask.cpp index 377579bd6970..27cb39df3692 100644 --- a/js/src/gc/GCParallelTask.cpp +++ b/js/src/gc/GCParallelTask.cpp @@ -100,7 +100,19 @@ void js::GCParallelTask::joinWithLockHeld(AutoLockHelperThreadState& lock, return; } - joinNonIdleTask(deadline, lock); + if (isNotYetRunning(lock) && deadline.isNothing()) { + // If the task was dispatched but has not yet started then cancel the task + // and run it from the main thread. This stops us from blocking here when + // the helper threads are busy with other tasks. + MOZ_ASSERT(isInList()); + MOZ_ASSERT_IF(isDispatched(lock), gc->dispatchedParallelTasks != 0); + + remove(); + runFromMainThread(lock); + } else { + // Otherwise wait for the task to complete. + joinNonIdleTask(deadline, lock); + } if (isIdle(lock)) { recordDuration(); @@ -115,9 +127,9 @@ void GCParallelTask::recordDuration() { void js::GCParallelTask::joinNonIdleTask(Maybe deadline, AutoLockHelperThreadState& lock) { - while (!isFinished(lock)) { - MOZ_ASSERT(!isIdle(lock)); + MOZ_ASSERT(!isIdle(lock)); + while (!isFinished(lock)) { TimeDuration timeout = TimeDuration::Forever(); if (deadline) { TimeStamp now = TimeStamp::Now(); @@ -136,9 +148,7 @@ void js::GCParallelTask::joinNonIdleTask(Maybe deadline, } void js::GCParallelTask::runFromMainThread(AutoLockHelperThreadState& lock) { - MOZ_ASSERT(isIdle(lock)); MOZ_ASSERT(js::CurrentThreadCanAccessRuntime(gc->rt)); - runTask(gc->rt->gcContext(), lock); setIdle(lock); } diff --git a/js/src/gc/ParallelMarking.cpp b/js/src/gc/ParallelMarking.cpp index 2d77b12e4744..ea94d465cdb0 100644 --- a/js/src/gc/ParallelMarking.cpp +++ b/js/src/gc/ParallelMarking.cpp @@ -81,25 +81,19 @@ bool ParallelMarker::markOneColor(MarkColor color, SliceBudget& sliceBudget) { AutoLockHelperThreadState lock; + // There should always be enough parallel tasks to run our marking work. + MOZ_RELEASE_ASSERT(gc->maxParallelThreads >= workerCount()); + MOZ_ASSERT(activeTasks == 0); for (size_t i = 0; i < workerCount(); i++) { ParallelMarkTask& task = *tasks[i]; if (task.hasWork()) { incActiveTasks(&task, lock); } - } - - // There should always be enough parallel tasks to run our marking work. - MOZ_RELEASE_ASSERT(gc->maxParallelThreads >= workerCount()); - - // Run the parallel tasks, using the main thread for the first one. - for (size_t i = 1; i < workerCount(); i++) { - ParallelMarkTask& task = *tasks[i]; gc->startTask(task, lock); } - tasks[0]->runFromMainThread(lock); - tasks[0]->recordDuration(); // Record stats as if it used a helper thread. - for (size_t i = 1; i < workerCount(); i++) { + + for (size_t i = 0; i < workerCount(); i++) { gc->joinTask(*tasks[i], lock); } diff --git a/js/src/vm/HelperThreadState.h b/js/src/vm/HelperThreadState.h index 0096ccf47bb5..a43efef7af02 100644 --- a/js/src/vm/HelperThreadState.h +++ b/js/src/vm/HelperThreadState.h @@ -32,8 +32,8 @@ #include "js/AllocPolicy.h" // SystemAllocPolicy #include "js/CompileOptions.h" // JS::ReadOnlyCompileOptions #include "js/experimental/JSStencil.h" // JS::InstantiationStorage -#include "js/HelperThreadAPI.h" // JS::HelperThreadTaskCallback -#include "js/MemoryMetrics.h" // JS::GlobalStats +#include "js/HelperThreadAPI.h" // JS::HelperThreadTaskCallback, JS::DispatchReason +#include "js/MemoryMetrics.h" // JS::GlobalStats #include "js/ProfilingStack.h" // JS::RegisterThreadCallback, JS::UnregisterThreadCallback #include "js/RootingAPI.h" // JS::Handle #include "js/UniquePtr.h" // UniquePtr @@ -79,8 +79,6 @@ typedef Vector } // namespace wasm -using HelperThreadTaskVector = Vector; - // Per-process state for off thread work items. class GlobalHelperThreadState { friend class AutoLockHelperThreadState; @@ -174,6 +172,8 @@ class GlobalHelperThreadState { // GCRuntime before being dispatched to the helper thread system. GCParallelTaskList gcParallelWorklist_; + using HelperThreadTaskVector = + Vector; // Vector of running HelperThreadTask. // This is used to get the HelperThreadTask that are currently running. HelperThreadTaskVector helperTasks_; @@ -183,15 +183,9 @@ class GlobalHelperThreadState { // pool is used. JS::HelperThreadTaskCallback dispatchTaskCallback = nullptr; - // Condition variable for notifiying the main thread that a helper task has - // completed some work. - js::ConditionVariable consumerWakeup; - -#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; @@ -253,7 +247,7 @@ class GlobalHelperThreadState { // Helper method for removing items from the vectors below while iterating // over them. template - static void remove(T& vector, size_t* index) { + void remove(T& vector, size_t* index) { // Self-moving is undefined behavior. if (*index != vector.length() - 1) { vector[*index] = std::move(vector.back()); @@ -327,7 +321,6 @@ class GlobalHelperThreadState { return compressionFinishedList_; } - private: GCParallelTaskList& gcParallelWorklist() { return gcParallelWorklist_; } HelperThreadTaskVector& helperTasks(const AutoLockHelperThreadState&) { @@ -372,55 +365,46 @@ class GlobalHelperThreadState { HelperThreadTask* maybeGetGCParallelTask( const AutoLockHelperThreadState& lock); - jit::IonCompileTask* highestPriorityPendingIonCompile( - const AutoLockHelperThreadState& lock, bool checkExecutionStatus); - - bool checkTaskThreadLimit(ThreadType threadType, size_t maxThreads, - bool isMaster, - const AutoLockHelperThreadState& lock) const; - bool checkTaskThreadLimit(ThreadType threadType, size_t maxThreads, - const AutoLockHelperThreadState& lock) const { - return checkTaskThreadLimit(threadType, maxThreads, /* isMaster */ false, - lock); - } - - bool hasActiveThreads(const AutoLockHelperThreadState&); - bool canStartTasks(const AutoLockHelperThreadState& locked); + enum class ScheduleCompressionTask { GC, API }; - public: // Used by a major GC to signal processing enqueued compression tasks. - enum class ScheduleCompressionTask { GC, API }; void startHandlingCompressionTasks(ScheduleCompressionTask schedule, JSRuntime* maybeRuntime, const AutoLockHelperThreadState& lock); - void runPendingSourceCompressions(JSRuntime* runtime, - AutoLockHelperThreadState& lock); + jit::IonCompileTask* highestPriorityPendingIonCompile( + const AutoLockHelperThreadState& lock, bool checkExecutionStatus); + public: void trace(JSTracer* trc); + bool hasActiveThreads(const AutoLockHelperThreadState&); + bool canStartTasks(const AutoLockHelperThreadState& locked); void waitForAllTasks(); void waitForAllTasksLocked(AutoLockHelperThreadState&); -#ifdef DEBUG - bool hasOffThreadIonCompile(Zone* zone, AutoLockHelperThreadState& lock); -#endif + bool checkTaskThreadLimit(ThreadType threadType, size_t maxThreads, + bool isMaster, + const AutoLockHelperThreadState& lock) const; + bool checkTaskThreadLimit(ThreadType threadType, size_t maxThreads, + const AutoLockHelperThreadState& lock) const { + return checkTaskThreadLimit(threadType, maxThreads, /* isMaster */ false, + lock); + } - void cancelOffThreadIonCompile(const CompilationSelector& selector); - void cancelOffThreadWasmTier2Generator(AutoLockHelperThreadState& lock); + void triggerFreeUnusedMemory(); - bool hasAnyDelazifyTask(JSRuntime* rt, AutoLockHelperThreadState& lock); - void cancelPendingDelazifyTask(JSRuntime* rt, - AutoLockHelperThreadState& lock); - void waitUntilCancelledDelazifyTasks(JSRuntime* rt, - AutoLockHelperThreadState& lock); - void waitUntilEmptyFreeDelazifyTaskVector(AutoLockHelperThreadState& lock); + private: + // Condition variable for notifiying the main thread that a helper task has + // completed some work. + js::ConditionVariable consumerWakeup; - void cancelOffThreadCompressions(JSRuntime* runtime, - AutoLockHelperThreadState& lock); + void dispatch(JS::DispatchReason reason, + const AutoLockHelperThreadState& locked); - void triggerFreeUnusedMemory(); + void runTask(HelperThreadTask* task, AutoLockHelperThreadState& lock); + public: bool submitTask(wasm::UniqueTier2GeneratorTask task); bool submitTask(wasm::CompileTask* task, wasm::CompileMode mode); bool submitTask(UniquePtr&& task, @@ -435,19 +419,15 @@ class GlobalHelperThreadState { bool submitTask(PromiseHelperTask* task); bool submitTask(GCParallelTask* task, const AutoLockHelperThreadState& locked); - - void runOneTask(HelperThreadTask* task, AutoLockHelperThreadState& lock); - void dispatch(const AutoLockHelperThreadState& locked); - - private: - HelperThreadTask* findHighestPriorityTask( - const AutoLockHelperThreadState& locked); - + void runOneTask(AutoLockHelperThreadState& lock); void runTaskLocked(HelperThreadTask* task, AutoLockHelperThreadState& lock); using Selector = HelperThreadTask* ( GlobalHelperThreadState::*)(const AutoLockHelperThreadState&); static const Selector selectors[]; + + HelperThreadTask* findHighestPriorityTask( + const AutoLockHelperThreadState& locked); }; static inline bool IsHelperThreadStateInitialized() { diff --git a/js/src/vm/HelperThreadTask.h b/js/src/vm/HelperThreadTask.h index 75e66be638e9..d27e53033ddc 100644 --- a/js/src/vm/HelperThreadTask.h +++ b/js/src/vm/HelperThreadTask.h @@ -54,19 +54,14 @@ struct MapTypeToThreadType { static const ThreadType threadType = THREAD_TYPE_COMPRESS; }; -} // namespace js - -namespace JS { - -class HelperThreadTask { - public: - virtual void runHelperThreadTask(js::AutoLockHelperThreadState& locked) = 0; - virtual js::ThreadType threadType() = 0; +struct HelperThreadTask { + virtual void runHelperThreadTask(AutoLockHelperThreadState& locked) = 0; + virtual ThreadType threadType() = 0; virtual ~HelperThreadTask() = default; template bool is() { - return js::MapTypeToThreadType::threadType == threadType(); + return MapTypeToThreadType::threadType == threadType(); } template @@ -76,10 +71,6 @@ class 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 5de1a9a4e780..da8231c1dc73 100644 --- a/js/src/vm/HelperThreads.cpp +++ b/js/src/vm/HelperThreads.cpp @@ -39,6 +39,8 @@ using mozilla::TimeDuration; using mozilla::TimeStamp; using mozilla::Utf8Unit; +using JS::DispatchReason; + namespace js { Mutex gHelperThreadLock(mutexid::GlobalHelperThreadState); @@ -151,7 +153,7 @@ bool GlobalHelperThreadState::submitTask(wasm::CompileTask* task, return false; } - dispatch(lock); + dispatch(DispatchReason::NewTask, lock); return true; } @@ -179,7 +181,7 @@ bool GlobalHelperThreadState::submitTask(wasm::UniqueTier2GeneratorTask task) { } (void)task.release(); - dispatch(lock); + dispatch(DispatchReason::NewTask, lock); return true; } @@ -189,19 +191,14 @@ static void CancelOffThreadWasmTier2GeneratorLocked( return; } - HelperThreadState().cancelOffThreadWasmTier2Generator(lock); -} - -void GlobalHelperThreadState::cancelOffThreadWasmTier2Generator( - AutoLockHelperThreadState& lock) { // Remove pending tasks from the tier2 generator worklist and cancel and // delete them. { wasm::Tier2GeneratorTaskPtrVector& worklist = - wasmTier2GeneratorWorklist(lock); + HelperThreadState().wasmTier2GeneratorWorklist(lock); for (size_t i = 0; i < worklist.length(); i++) { wasm::Tier2GeneratorTask* task = worklist[i]; - remove(worklist, &i); + HelperThreadState().remove(worklist, &i); js_delete(task); } } @@ -213,7 +210,7 @@ void GlobalHelperThreadState::cancelOffThreadWasmTier2Generator( // If there is a running Tier2 generator task, shut it down in a predictable // way. The task will be deleted by the normal deletion logic. - for (auto* helper : helperTasks(lock)) { + for (auto* helper : HelperThreadState().helperTasks(lock)) { if (helper->is()) { // Set a flag that causes compilation to shortcut itself. helper->as()->cancel(); @@ -222,9 +219,11 @@ void GlobalHelperThreadState::cancelOffThreadWasmTier2Generator( // where the shutdown code is trying to shut down helper threads and the // ongoing tier2 compilation is trying to finish, which requires it to // have access to helper threads. - uint32_t oldFinishedCount = wasmTier2GeneratorsFinished(lock); - while (wasmTier2GeneratorsFinished(lock) == oldFinishedCount) { - wait(lock); + uint32_t oldFinishedCount = + HelperThreadState().wasmTier2GeneratorsFinished(lock); + while (HelperThreadState().wasmTier2GeneratorsFinished(lock) == + oldFinishedCount) { + HelperThreadState().wait(lock); } // At most one of these tasks. @@ -255,7 +254,7 @@ bool GlobalHelperThreadState::submitTask( // unwanted mutations. task->alloc().lifoAlloc()->setReadOnly(); - dispatch(locked); + dispatch(DispatchReason::NewTask, locked); return true; } @@ -313,7 +312,7 @@ bool GlobalHelperThreadState::submitTask( return false; } - dispatch(locked); + dispatch(DispatchReason::NewTask, locked); return true; } @@ -402,11 +401,6 @@ void js::CancelOffThreadIonCompile(const CompilationSelector& selector) { return; } - HelperThreadState().cancelOffThreadIonCompile(selector); -} - -void GlobalHelperThreadState::cancelOffThreadIonCompile( - const CompilationSelector& selector) { jit::JitRuntime* jitRuntime = GetSelectorRuntime(selector)->jitRuntime(); MOZ_ASSERT(jitRuntime); @@ -414,12 +408,13 @@ void GlobalHelperThreadState::cancelOffThreadIonCompile( { AutoLockHelperThreadState lock; - if (!isInitialized(lock)) { + if (!HelperThreadState().isInitialized(lock)) { return; } /* Cancel any pending entries for which processing hasn't started. */ - GlobalHelperThreadState::IonCompileTaskVector& worklist = ionWorklist(lock); + GlobalHelperThreadState::IonCompileTaskVector& worklist = + HelperThreadState().ionWorklist(lock); for (size_t i = 0; i < worklist.length(); i++) { jit::IonCompileTask* task = worklist[i]; if (IonCompileTaskMatches(selector, task)) { @@ -429,7 +424,7 @@ void GlobalHelperThreadState::cancelOffThreadIonCompile( worklist[i]->alloc().lifoAlloc()->setReadWrite(); FinishOffThreadIonCompile(task, lock); - remove(worklist, &i); + HelperThreadState().remove(worklist, &i); } } @@ -437,40 +432,40 @@ void GlobalHelperThreadState::cancelOffThreadIonCompile( bool cancelled; do { cancelled = false; - for (auto* helper : helperTasks(lock)) { + for (auto* helper : HelperThreadState().helperTasks(lock)) { if (!helper->is()) { continue; } jit::IonCompileTask* ionCompileTask = helper->as(); if (IonCompileTaskMatches(selector, ionCompileTask)) { - ionCompileTask->alloc().lifoAlloc()->setReadWrite(); ionCompileTask->mirGen().cancel(); cancelled = true; } } if (cancelled) { - wait(lock); + HelperThreadState().wait(lock); } } while (cancelled); /* Cancel code generation for any completed entries. */ GlobalHelperThreadState::IonCompileTaskVector& finished = - ionFinishedList(lock); + HelperThreadState().ionFinishedList(lock); for (size_t i = 0; i < finished.length(); i++) { jit::IonCompileTask* task = finished[i]; if (IonCompileTaskMatches(selector, task)) { JSRuntime* rt = task->script()->runtimeFromAnyThread(); - jitRuntime->numFinishedOffThreadTasksRef(lock)--; + rt->jitRuntime()->numFinishedOffThreadTasksRef(lock)--; jit::FinishOffThreadTask(rt, freeTask, task); - remove(finished, &i); + HelperThreadState().remove(finished, &i); } } } /* Cancel lazy linking for pending tasks (attached to the ionScript). */ JSRuntime* runtime = GetSelectorRuntime(selector); - jit::IonCompileTask* task = jitRuntime->ionLazyLinkList(runtime).getFirst(); + jit::IonCompileTask* task = + runtime->jitRuntime()->ionLazyLinkList(runtime).getFirst(); while (task) { jit::IonCompileTask* next = task->getNext(); if (IonCompileTaskMatches(selector, task)) { @@ -492,27 +487,29 @@ bool js::HasOffThreadIonCompile(Zone* zone) { return false; } - return HelperThreadState().hasOffThreadIonCompile(zone, lock); -} - -bool GlobalHelperThreadState::hasOffThreadIonCompile( - Zone* zone, AutoLockHelperThreadState& lock) { - for (jit::IonCompileTask* task : ionWorklist(lock)) { + GlobalHelperThreadState::IonCompileTaskVector& worklist = + HelperThreadState().ionWorklist(lock); + for (size_t i = 0; i < worklist.length(); i++) { + jit::IonCompileTask* task = worklist[i]; if (task->script()->zoneFromAnyThread() == zone) { return true; } } - for (auto* helper : helperTasks(lock)) { - if (helper->is()) { - JSScript* script = helper->as()->script(); - if (script->zoneFromAnyThread() == zone) { - return true; - } + for (auto* helper : HelperThreadState().helperTasks(lock)) { + if (!helper->is()) { + continue; + } + JSScript* script = helper->as()->script(); + if (script->zoneFromAnyThread() == zone) { + return true; } } - for (jit::IonCompileTask* task : ionFinishedList(lock)) { + GlobalHelperThreadState::IonCompileTaskVector& finished = + HelperThreadState().ionFinishedList(lock); + for (size_t i = 0; i < finished.length(); i++) { + jit::IonCompileTask* task = finished[i]; if (task->script()->zoneFromAnyThread() == zone) { return true; } @@ -520,10 +517,13 @@ bool GlobalHelperThreadState::hasOffThreadIonCompile( JSRuntime* rt = zone->runtimeFromMainThread(); if (rt->hasJitRuntime()) { - for (jit::IonCompileTask* task : rt->jitRuntime()->ionLazyLinkList(rt)) { + jit::IonCompileTask* task = + rt->jitRuntime()->ionLazyLinkList(rt).getFirst(); + while (task) { if (task->script()->zone() == zone) { return true; } + task = task->getNext(); } } @@ -641,9 +641,9 @@ void FreeDelazifyTask::runHelperThreadTask(AutoLockHelperThreadState& locked) { js_delete(this); } -void GlobalHelperThreadState::cancelPendingDelazifyTask( - JSRuntime* rt, AutoLockHelperThreadState& lock) { - auto& delazifyList = delazifyWorklist(lock); +static void CancelPendingDelazifyTask(JSRuntime* rt, + AutoLockHelperThreadState& lock) { + auto& delazifyList = HelperThreadState().delazifyWorklist(lock); auto end = delazifyList.end(); for (auto iter = delazifyList.begin(); iter != end;) { @@ -656,17 +656,21 @@ void GlobalHelperThreadState::cancelPendingDelazifyTask( } } -void GlobalHelperThreadState::waitUntilCancelledDelazifyTasks( - JSRuntime* rt, AutoLockHelperThreadState& lock) { +static void WaitUntilCancelledDelazifyTasks(JSRuntime* rt, + AutoLockHelperThreadState& lock) { + if (!HelperThreadState().isInitialized(lock)) { + return; + } + while (true) { - cancelPendingDelazifyTask(rt, lock); + CancelPendingDelazifyTask(rt, lock); // If running tasks are delazifying any functions, then we have to wait // until they complete to remove them from the pending list. DelazifyTask // are inserting themself back to be processed once more after delazifying a // function. bool inProgress = false; - for (auto* helper : helperTasks(lock)) { + for (auto* helper : HelperThreadState().helperTasks(lock)) { if (helper->is() && helper->as()->runtimeMatchesOrNoRuntime(rt)) { inProgress = true; @@ -677,17 +681,30 @@ void GlobalHelperThreadState::waitUntilCancelledDelazifyTasks( break; } - wait(lock); + HelperThreadState().wait(lock); } - MOZ_ASSERT(!hasAnyDelazifyTask(rt, lock)); +#ifdef DEBUG + for (DelazifyTask* task : HelperThreadState().delazifyWorklist(lock)) { + MOZ_ASSERT(!task->runtimeMatchesOrNoRuntime(rt)); + } + for (auto* helper : HelperThreadState().helperTasks(lock)) { + MOZ_ASSERT_IF(helper->is(), + !helper->as()->runtimeMatchesOrNoRuntime(rt)); + } +#endif } -void GlobalHelperThreadState::waitUntilEmptyFreeDelazifyTaskVector( +static void WaitUntilEmptyFreeDelazifyTaskVector( AutoLockHelperThreadState& lock) { + if (!HelperThreadState().isInitialized(lock)) { + return; + } + while (true) { bool inProgress = false; - if (!freeDelazifyTaskVector(lock).empty()) { + auto& freeList = HelperThreadState().freeDelazifyTaskVector(lock); + if (!freeList.empty()) { inProgress = true; } @@ -695,7 +712,7 @@ void GlobalHelperThreadState::waitUntilEmptyFreeDelazifyTaskVector( // until they complete to remove them from the pending list. DelazifyTask // are inserting themself back to be processed once more after delazifying a // function. - for (auto* helper : helperTasks(lock)) { + for (auto* helper : HelperThreadState().helperTasks(lock)) { if (helper->is()) { inProgress = true; break; @@ -705,35 +722,31 @@ void GlobalHelperThreadState::waitUntilEmptyFreeDelazifyTaskVector( break; } - wait(lock); + HelperThreadState().wait(lock); } } void js::CancelOffThreadDelazify(JSRuntime* runtime) { AutoLockHelperThreadState lock; - if (!HelperThreadState().isInitialized(lock)) { - return; - } - // Cancel all Delazify tasks from the given runtime, and wait if tasks are // from the given runtime are being executed. - HelperThreadState().waitUntilCancelledDelazifyTasks(runtime, lock); + WaitUntilCancelledDelazifyTasks(runtime, lock); // Empty the free list of delazify task, in case one of the delazify task // ended and therefore did not returned to the pending list of delazify tasks. - HelperThreadState().waitUntilEmptyFreeDelazifyTaskVector(lock); + WaitUntilEmptyFreeDelazifyTaskVector(lock); } -bool GlobalHelperThreadState::hasAnyDelazifyTask( - JSRuntime* rt, AutoLockHelperThreadState& lock) { - for (auto task : delazifyWorklist(lock)) { +static bool HasAnyDelazifyTask(JSRuntime* rt, AutoLockHelperThreadState& lock) { + auto& delazifyList = HelperThreadState().delazifyWorklist(lock); + for (auto task : delazifyList) { if (task->runtimeMatchesOrNoRuntime(rt)) { return true; } } - for (auto* helper : helperTasks(lock)) { + for (auto* helper : HelperThreadState().helperTasks(lock)) { if (helper->is() && helper->as()->runtimeMatchesOrNoRuntime(rt)) { return true; @@ -750,7 +763,7 @@ void js::WaitForAllDelazifyTasks(JSRuntime* rt) { } while (true) { - if (!HelperThreadState().hasAnyDelazifyTask(rt, lock)) { + if (!HasAnyDelazifyTask(rt, lock)) { break; } @@ -761,7 +774,7 @@ void js::WaitForAllDelazifyTasks(JSRuntime* rt) { void GlobalHelperThreadState::submitTask( DelazifyTask* task, const AutoLockHelperThreadState& locked) { delazifyWorklist(locked).insertBack(task); - dispatch(locked); + dispatch(DispatchReason::NewTask, locked); } bool GlobalHelperThreadState::submitTask( @@ -769,7 +782,7 @@ bool GlobalHelperThreadState::submitTask( if (!freeDelazifyTaskVector(locked).append(std::move(task))) { return false; } - dispatch(locked); + dispatch(DispatchReason::NewTask, locked); return true; } @@ -876,30 +889,19 @@ void GlobalHelperThreadState::assertIsLockedByCurrentThread() const { } #endif // DEBUG -void GlobalHelperThreadState::dispatch(const AutoLockHelperThreadState& lock) { - if (helperTasks_.length() >= threadCount) { - return; - } +void GlobalHelperThreadState::dispatch( + DispatchReason reason, const AutoLockHelperThreadState& locked) { + if (canStartTasks(locked) && 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_++; - 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++; - - // The hazard analysis can't tell that the callback doesn't GC. - JS::AutoSuppressGCAnalysis nogc; + // The hazard analysis can't tell that the callback doesn't GC. + JS::AutoSuppressGCAnalysis nogc; - dispatchTaskCallback(task); + dispatchTaskCallback(reason); + } } void GlobalHelperThreadState::wait( @@ -936,11 +938,10 @@ void GlobalHelperThreadState::waitForAllTasksLocked( AutoLockHelperThreadState& lock) { CancelOffThreadWasmTier2GeneratorLocked(lock); - while (canStartTasks(lock) || hasActiveThreads(lock)) { + while (canStartTasks(lock) || tasksPending_ || 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()); @@ -1409,14 +1410,14 @@ bool GlobalHelperThreadState::submitTask( return false; } - dispatch(locked); + dispatch(DispatchReason::NewTask, locked); return true; } bool GlobalHelperThreadState::submitTask( GCParallelTask* task, const AutoLockHelperThreadState& locked) { gcParallelWorklist().insertBack(task, locked); - dispatch(locked); + dispatch(DispatchReason::NewTask, locked); return true; } @@ -1470,20 +1471,18 @@ void js::CancelOffThreadCompressions(JSRuntime* runtime) { } AutoLockHelperThreadState lock; - HelperThreadState().cancelOffThreadCompressions(runtime, lock); -} -void GlobalHelperThreadState::cancelOffThreadCompressions( - JSRuntime* runtime, AutoLockHelperThreadState& lock) { // Cancel all pending compression tasks. - ClearCompressionTaskList(compressionPendingList(lock), runtime); - ClearCompressionTaskList(compressionWorklist(lock), runtime); + ClearCompressionTaskList(HelperThreadState().compressionPendingList(lock), + runtime); + ClearCompressionTaskList(HelperThreadState().compressionWorklist(lock), + runtime); // Cancel all in-process compression tasks and wait for them to join so we // clean up the finished tasks. while (true) { bool inProgress = false; - for (auto* helper : helperTasks(lock)) { + for (auto* helper : HelperThreadState().helperTasks(lock)) { if (!helper->is()) { continue; } @@ -1497,11 +1496,12 @@ void GlobalHelperThreadState::cancelOffThreadCompressions( break; } - wait(lock); + HelperThreadState().wait(lock); } // Clean up finished tasks. - ClearCompressionTaskList(compressionFinishedList(lock), runtime); + ClearCompressionTaskList(HelperThreadState().compressionFinishedList(lock), + runtime); } void js::AttachFinishedCompressions(JSRuntime* runtime, @@ -1531,21 +1531,17 @@ void js::RunPendingSourceCompressions(JSRuntime* runtime) { } AutoLockHelperThreadState lock; - HelperThreadState().runPendingSourceCompressions(runtime, lock); -} -void GlobalHelperThreadState::runPendingSourceCompressions( - JSRuntime* runtime, AutoLockHelperThreadState& lock) { - startHandlingCompressionTasks( + HelperThreadState().startHandlingCompressionTasks( GlobalHelperThreadState::ScheduleCompressionTask::API, nullptr, lock); // Wait until all tasks have started compression. - while (!compressionWorklist(lock).empty()) { - wait(lock); + while (!HelperThreadState().compressionWorklist(lock).empty()) { + HelperThreadState().wait(lock); } // Wait for all in-process compression tasks to complete. - waitForAllTasksLocked(lock); + HelperThreadState().waitForAllTasksLocked(lock); AttachFinishedCompressions(runtime, lock); } @@ -1598,7 +1594,7 @@ bool GlobalHelperThreadState::submitTask(PromiseHelperTask* task) { return false; } - dispatch(lock); + dispatch(DispatchReason::NewTask, lock); return true; } @@ -1631,15 +1627,15 @@ void GlobalHelperThreadState::trace(JSTracer* trc) { task->trace(trc); } - for (auto* helper : helperTasks(lock)) { + for (auto* helper : HelperThreadState().helperTasks(lock)) { if (helper->is()) { - jit::IonCompileTask* ionCompileTask = helper->as(); - ionCompileTask->alloc().lifoAlloc()->setReadWrite(); - ionCompileTask->trace(trc); + helper->as()->trace(trc); } } } + // The lazy link list is only accessed on the main thread, so trace it after + // releasing the lock. JSRuntime* rt = trc->runtime(); if (auto* jitRuntime = rt->jitRuntime()) { jit::IonCompileTask* task = jitRuntime->ionLazyLinkList(rt).getFirst(); @@ -1676,8 +1672,7 @@ bool GlobalHelperThreadState::canStartTasks( canStartWasmTier2GeneratorTask(lock); } -void JS::RunHelperThreadTask(HelperThreadTask* task) { - MOZ_ASSERT(task); +void JS::RunHelperThreadTask() { MOZ_ASSERT(CanUseExtraThreads()); AutoLockHelperThreadState lock; @@ -1686,18 +1681,22 @@ void JS::RunHelperThreadTask(HelperThreadTask* task) { return; } - HelperThreadState().runOneTask(task, lock); - HelperThreadState().dispatch(lock); + HelperThreadState().runOneTask(lock); } -void GlobalHelperThreadState::runOneTask(HelperThreadTask* task, - AutoLockHelperThreadState& lock) { -#ifdef DEBUG +void GlobalHelperThreadState::runOneTask(AutoLockHelperThreadState& lock) { MOZ_ASSERT(tasksPending_ > 0); tasksPending_--; -#endif - runTaskLocked(task, lock); + // 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); + } notifyAll(lock); } @@ -1715,37 +1714,24 @@ HelperThreadTask* GlobalHelperThreadState::findHighestPriorityTask( return nullptr; } -#ifdef DEBUG -static bool VectorHasTask(const HelperThreadTaskVector& tasks, - HelperThreadTask* task) { - for (HelperThreadTask* t : tasks) { - if (t == task) { - return true; - } - } - - return false; -} -#endif - void GlobalHelperThreadState::runTaskLocked(HelperThreadTask* task, AutoLockHelperThreadState& locked) { - ThreadType threadType = task->threadType(); + JS::AutoSuppressGCAnalysis nogc; - MOZ_ASSERT(VectorHasTask(helperTasks(locked), task)); - MOZ_ASSERT(totalCountRunningTasks != 0); - MOZ_ASSERT(runningTaskCount[threadType] != 0); + HelperThreadState().helperTasks(locked).infallibleEmplaceBack(task); + ThreadType threadType = task->threadType(); js::oom::SetThreadType(threadType); + runningTaskCount[threadType]++; + totalCountRunningTasks++; - { - JS::AutoSuppressGCAnalysis nogc; - task->runHelperThreadTask(locked); - } + task->runHelperThreadTask(locked); - js::oom::SetThreadType(js::THREAD_TYPE_NONE); + // Delete task from helperTasks. + HelperThreadState().helperTasks(locked).eraseIfEqual(task); - helperTasks(locked).eraseIfEqual(task); totalCountRunningTasks--; runningTaskCount[threadType]--; + + js::oom::SetThreadType(js::THREAD_TYPE_NONE); } diff --git a/js/src/vm/InternalThreadPool.cpp b/js/src/vm/InternalThreadPool.cpp index 7c5f6c45ae4c..483e99525461 100644 --- a/js/src/vm/InternalThreadPool.cpp +++ b/js/src/vm/InternalThreadPool.cpp @@ -55,10 +55,6 @@ namespace js { class HelperThread { Thread thread; - ConditionVariable wakeup; - - HelperThreadLockData nextTask; - /* * The profiling thread for this helper thread, which can be used to push * and pop label frames. @@ -68,9 +64,7 @@ class HelperThread { ProfilingStack* profilingStack = nullptr; public: - const uint32_t id; - - explicit HelperThread(uint32_t id); + HelperThread(); [[nodiscard]] bool init(InternalThreadPool* pool); ThreadId threadId() { return thread.get_id(); } @@ -83,9 +77,6 @@ class HelperThread { void ensureRegisteredWithProfiler(); void unregisterWithProfilerIfNeeded(); - void dispatchTask(HelperThreadTask* task); - void notify(); - private: struct AutoProfilerLabel { AutoProfilerLabel(HelperThread* helperThread, const char* label, @@ -131,9 +122,6 @@ bool InternalThreadPool::Initialize(size_t threadCount, bool InternalThreadPool::ensureThreadCount(size_t threadCount, AutoLockHelperThreadState& lock) { - // Ensure space in freeThreadSet. - MOZ_RELEASE_ASSERT(threadCount <= sizeof(uint32_t) * CHAR_BIT); - MOZ_ASSERT(threads(lock).length() < threadCount); if (!threads(lock).reserve(threadCount)) { @@ -141,20 +129,12 @@ bool InternalThreadPool::ensureThreadCount(size_t threadCount, } while (threads(lock).length() < threadCount) { - uint32_t id = threads(lock).length(); - - auto thread = js::MakeUnique(id); + auto thread = js::MakeUnique(); if (!thread || !thread->init(this)) { return false; } threads(lock).infallibleEmplaceBack(std::move(thread)); - - setThreadFree(id); - } - - for (size_t i = 0; i < threads(lock).length(); i++) { - MOZ_ASSERT(threads(lock)[i]->id == i); } return true; @@ -177,9 +157,7 @@ void InternalThreadPool::shutDown(AutoLockHelperThreadState& lock) { MOZ_ASSERT(!terminating); terminating = true; - for (auto& thread : threads(lock)) { - thread->notify(); - } + notifyAll(lock); for (auto& thread : threads(lock)) { AutoUnlockHelperThreadState unlock(lock); @@ -204,36 +182,34 @@ size_t InternalThreadPool::sizeOfIncludingThis( } /* static */ -void InternalThreadPool::DispatchTask(HelperThreadTask* task) { - Get().dispatchOrQueueTask(task); +void InternalThreadPool::DispatchTask(JS::DispatchReason reason) { + Get().dispatchTask(reason); } -void InternalThreadPool::dispatchOrQueueTask(HelperThreadTask* task) { +void InternalThreadPool::dispatchTask(JS::DispatchReason reason) { gHelperThreadLock.assertOwnedByCurrentThread(); - MOZ_ASSERT(!terminating); - MOZ_ASSERT(freeThreadSet != 0); - - uint32_t id = mozilla::CountTrailingZeroes32(freeThreadSet); - clearThreadFree(id); - - HelperThread* thread = threads_.ref()[id].get(); - thread->dispatchTask(task); + queuedTasks++; + 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 + // need to wake up any threads. + MOZ_ASSERT(reason == JS::DispatchReason::FinishedTask); + MOZ_ASSERT(!TlsContext.get(), "we should be on a helper thread"); + } } -void InternalThreadPool::setThreadFree(uint32_t threadId) { - uint32_t idMask = 1 << threadId; - MOZ_ASSERT((freeThreadSet & idMask) == 0); - freeThreadSet |= idMask; +void InternalThreadPool::notifyAll(const AutoLockHelperThreadState& lock) { + wakeup.notify_all(); } -void InternalThreadPool::clearThreadFree(uint32_t threadId) { - uint32_t idMask = 1 << threadId; - MOZ_ASSERT((freeThreadSet & idMask) != 0); - freeThreadSet &= ~idMask; +void InternalThreadPool::wait(AutoLockHelperThreadState& lock) { + wakeup.wait_for(lock, mozilla::TimeDuration::Forever()); } -HelperThread::HelperThread(uint32_t id) - : thread(Thread::Options().setStackSize(HELPER_STACK_SIZE)), id(id) {} +HelperThread::HelperThread() + : thread(Thread::Options().setStackSize(HELPER_STACK_SIZE)) {} bool HelperThread::init(InternalThreadPool* pool) { return thread.init(HelperThread::ThreadMain, pool, this); @@ -295,35 +271,19 @@ HelperThread::AutoProfilerLabel::~AutoProfilerLabel() { } } -void HelperThread::dispatchTask(HelperThreadTask* task) { - MOZ_ASSERT(!nextTask); - nextTask = task; - notify(); -} - -void HelperThread::notify() { wakeup.notify_one(); } - void HelperThread::threadLoop(InternalThreadPool* pool) { MOZ_ASSERT(CanUseExtraThreads()); AutoLockHelperThreadState lock; while (!pool->terminating) { - if (!nextTask) { - AUTO_PROFILER_LABEL("HelperThread::threadLoop::wait", IDLE); - wakeup.wait(lock); + if (pool->queuedTasks != 0) { + pool->queuedTasks--; + HelperThreadState().runOneTask(lock); continue; } - // JS::RunHelperThreadTask calls runOneTask and then dispatch. Here we split - // this up so we can mark the current thread as free in between and allow - // dispatch to pick this thread for the next task. - - HelperThreadState().runOneTask(nextTask, lock); - - nextTask = nullptr; - pool->setThreadFree(id); - - HelperThreadState().dispatch(lock); + AUTO_PROFILER_LABEL("HelperThread::threadLoop::wait", IDLE); + pool->wait(lock); } } diff --git a/js/src/vm/InternalThreadPool.h b/js/src/vm/InternalThreadPool.h index e241ce08edff..1d5c6d1a4316 100644 --- a/js/src/vm/InternalThreadPool.h +++ b/js/src/vm/InternalThreadPool.h @@ -19,20 +19,17 @@ #include "threading/ProtectedData.h" namespace JS { -class HelperThreadTask; +enum class DispatchReason; }; 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); @@ -48,26 +45,28 @@ class InternalThreadPool { const AutoLockHelperThreadState& lock) const; private: - static void DispatchTask(HelperThreadTask* task); + static void DispatchTask(JS::DispatchReason reason); - void dispatchOrQueueTask(HelperThreadTask* task); - void maybeDispatchQueuedTask(); + void dispatchTask(JS::DispatchReason reason); void shutDown(AutoLockHelperThreadState& lock); HelperThreadVector& threads(const AutoLockHelperThreadState& lock); const HelperThreadVector& threads( const AutoLockHelperThreadState& lock) const; - void setThreadFree(uint32_t threadId); - void clearThreadFree(uint32_t threadId); - + void notifyAll(const AutoLockHelperThreadState& lock); + void wait(AutoLockHelperThreadState& lock); friend class HelperThread; static InternalThreadPool* Instance; HelperThreadLockData threads_; + + js::ConditionVariable wakeup; + + HelperThreadLockData queuedTasks; + HelperThreadLockData terminating; - HelperThreadLockData freeThreadSet; }; } // namespace js diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index 43efd02fe652..8f3621f9c5d6 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -1108,19 +1108,15 @@ CycleCollectedJSRuntime* XPCJSContext::CreateRuntime(JSContext* aCx) { } class HelperThreadTaskHandler : public Task { - JS::HelperThreadTask* mTask; - public: - 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); + JS::RunHelperThreadTask(); return TaskResult::Complete; } + explicit HelperThreadTaskHandler() + : Task(Kind::OffMainThreadOnly, EventQueuePriority::Normal) { + // Bug 1703185: Currently all tasks are run at the same priority. + } #ifdef MOZ_COLLECTING_RUNNABLE_TELEMETRY bool GetName(nsACString& aName) override { @@ -1133,8 +1129,8 @@ class HelperThreadTaskHandler : public Task { ~HelperThreadTaskHandler() = default; }; -static void DispatchOffThreadTask(JS::HelperThreadTask* aTask) { - TaskController::Get()->AddTask(MakeAndAddRef(aTask)); +static void DispatchOffThreadTask(JS::DispatchReason) { + TaskController::Get()->AddTask(MakeAndAddRef()); } static bool CreateSelfHostedSharedMemory(JSContext* aCx, -- 2.11.4.GIT