Bug 1888429 - Part 1: Pass task to run to helper thread dispatch callback r=jandem,mccr8

This changes the helper thread system to pick the task to dispatch up front and
pass it though to TaskController via the callback.

One issue that came up was that the memory containing IonCompileTasks is
protected until the task starts running, but we may now trace these before that
happens.

Differential Revision: https://phabricator.services.mozilla.com/D206762
This commit is contained in:
Jon Coppeard 2024-05-24 16:50:05 +00:00
parent cac448c325
commit 4c0c91961b
8 changed files with 158 additions and 93 deletions

View File

@ -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

View File

@ -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* (

View File

@ -54,14 +54,19 @@ struct MapTypeToThreadType<SourceCompressionTask> {
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 <typename T>
bool is() {
return MapTypeToThreadType<T>::threadType == threadType();
return js::MapTypeToThreadType<T>::threadType == threadType();
}
template <typename T>
@ -71,6 +76,10 @@ struct HelperThreadTask {
}
};
} // namespace JS
namespace js {
using JS::HelperThreadTask;
} // namespace js
#endif /* vm_HelperThreadTask_h */

View File

@ -439,6 +439,7 @@ void js::CancelOffThreadIonCompile(const CompilationSelector& selector) {
jit::IonCompileTask* ionCompileTask = helper->as<jit::IonCompileTask>();
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<jit::IonCompileTask>()) {
helper->as<jit::IonCompileTask>()->trace(trc);
jit::IonCompileTask* ionCompileTask = helper->as<jit::IonCompileTask>();
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<HelperThreadTask*, 0, SystemAllocPolicy>& 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();
}

View File

@ -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<JS::HelperThreadTask*, 1, SystemAllocPolicy> tasksToDispatch;
mutable Vector<JS::DispatchReason, 1, SystemAllocPolicy> dispatchReasons;
};
// A lock guard for data protected by the helper thread lock.

View File

@ -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;
}

View File

@ -20,16 +20,20 @@
namespace JS {
enum class DispatchReason;
class HelperThreadTask;
};
namespace js {
class AutoLockHelperThreadState;
class HelperThread;
using JS::HelperThreadTask;
using HelperThreadVector =
Vector<UniquePtr<HelperThread>, 0, SystemAllocPolicy>;
using HelperTaskVector = Vector<HelperThreadTask*, 0, SystemAllocPolicy>;
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<HelperThreadVector> threads_;
js::ConditionVariable wakeup;
HelperThreadLockData<HelperTaskVector> tasks_;
HelperThreadLockData<size_t> queuedTasks;
js::ConditionVariable wakeup;
HelperThreadLockData<bool> terminating;
};

View File

@ -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<HelperThreadTaskHandler>());
static void DispatchOffThreadTask(JS::HelperThreadTask* aTask,
JS::DispatchReason) {
TaskController::Get()->AddTask(MakeAndAddRef<HelperThreadTaskHandler>(aTask));
}
static bool CreateSelfHostedSharedMemory(JSContext* aCx,