[wasm] Reschedule compilation tasks

At the moment, the whole WebAssembly compilation may run in a single
background task. On a low-end device, this can mean that the background
thread is busy for seconds and thereby blocks other tasks, see e.g.
https://crbug.com/914757.

With this CL we re-schedule compilation tasks after every 50ms. These
50ms are an arbitrary number. I don't want to introduce too much
overhead, but since this is in the background we also don't have to
make tasks super short.

Tasks which are going to compile with TurboFan will be posted with
lower priority.

This change requires changes in the CancelableTaskManager. At the
moment it is not possible that a background task posts a new task
which is managed by the same task manager as itself. The problem is
about how to deal with another thread which calls CancelAndWait
concurrently. At the moment, if a new task gets posted after the call
to CancelAndWait, then `CHECK(!canceled_)` in
CancelableTaskManager::Register will fail. If we used a lock to
synchronize the calls to CancelAndWait and Register, then there would
be a deadlock, where the thread which calls CancelAndWait waits for
the task which wants to call Register, but at the same time blocks that
task by holding the lock.

With the change here, posting a task after the call to CancelAndWait
will just immediately cancel the new task. This matches the behavior
you would get if CancelAndWait is called right after calling Register.

Bug: chromium:914757
Change-Id: I6d57aba161db8a915ec0d745658e0c28d25219a8
Reviewed-on: https://chromium-review.googlesource.com/c/1411884
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58898}

src/cancelable-task.cc

@@ -21,19 +21,34 @@ Cancelable::~Cancelable() {
 }
 
 CancelableTaskManager::CancelableTaskManager()
-    : task_id_counter_(0), canceled_(false) {}
+    : task_id_counter_(kInvalidTaskId), canceled_(false) {}
+
+CancelableTaskManager::~CancelableTaskManager() {
+  // It is required that {CancelAndWait} is called before the manager object is
+  // destroyed. This guarantees that all tasks managed by this
+  // {CancelableTaskManager} are either canceled or finished their execution
+  // when the {CancelableTaskManager} dies.
+  CHECK(canceled_);
+}
 
 CancelableTaskManager::Id CancelableTaskManager::Register(Cancelable* task) {
   base::MutexGuard guard(&mutex_);
+  if (canceled_) {
+    // The CancelableTaskManager has already been canceled. Therefore we mark
+    // the new task immediately as canceled so that it does not get executed.
+    task->Cancel();
+    return kInvalidTaskId;
+  }
   CancelableTaskManager::Id id = ++task_id_counter_;
   // Id overflows are not supported.
-  CHECK_NE(0, id);
+  CHECK_NE(kInvalidTaskId, id);
   CHECK(!canceled_);
   cancelable_tasks_[id] = task;
   return id;
 }
 
 void CancelableTaskManager::RemoveFinishedTask(CancelableTaskManager::Id id) {
+  CHECK_NE(kInvalidTaskId, id);
   base::MutexGuard guard(&mutex_);
   size_t removed = cancelable_tasks_.erase(id);
   USE(removed);
@@ -42,6 +57,7 @@ void CancelableTaskManager::RemoveFinishedTask(CancelableTaskManager::Id id) {
 }
 
 TryAbortResult CancelableTaskManager::TryAbort(CancelableTaskManager::Id id) {
+  CHECK_NE(kInvalidTaskId, id);
   base::MutexGuard guard(&mutex_);
   auto entry = cancelable_tasks_.find(id);
   if (entry != cancelable_tasks_.end()) {

src/cancelable-task.h

@@ -35,9 +35,15 @@ class V8_EXPORT_PRIVATE CancelableTaskManager {
 
   CancelableTaskManager();
 
+  ~CancelableTaskManager();
+
   // Registers a new cancelable {task}. Returns the unique {id} of the task that
   // can be used to try to abort a task by calling {Abort}.
-  // Must not be called after CancelAndWait.
+  // If {Register} is called after {CancelAndWait}, then the task will be will
+  // be aborted immediately.
+  // {Register} should only be called by the thread which owns the
+  // {CancelableTaskManager}, or by a task which is managed by the
+  // {CancelableTaskManager}.
   Id Register(Cancelable* task);
 
   // Try to abort running a task identified by {id}.
@@ -62,6 +68,8 @@ class V8_EXPORT_PRIVATE CancelableTaskManager {
   bool canceled() const { return canceled_; }
 
  private:
+  static constexpr Id kInvalidTaskId = 0;
+
   // Only called by {Cancelable} destructor. The task is done with executing,
   // but needs to be removed.
   void RemoveFinishedTask(Id id);

src/wasm/module-compiler.cc

@@ -86,8 +86,10 @@ class CompilationStateImpl {
 
   void OnFinishedUnit(ExecutionTier, WasmCode*);
 
+  void ReportDetectedFeatures(const WasmFeatures& detected);
   void OnBackgroundTaskStopped(const WasmFeatures& detected);
   void PublishDetectedFeatures(Isolate* isolate, const WasmFeatures& detected);
+  void RestartBackgroundCompileTask();
   void RestartBackgroundTasks(size_t max = std::numeric_limits<size_t>::max());
   // Only one foreground thread (finisher) is allowed to run at a time.
   // {SetFinisherIsRunning} returns whether the flag changed its state.
@@ -774,12 +776,18 @@ class BackgroundCompileTask : public CancelableTask {
     CompilationEnv env = native_module_->CreateCompilationEnv();
     auto* compilation_state = Impl(native_module_->compilation_state());
     WasmFeatures detected_features = kNoWasmFeatures;
+    double deadline = MonotonicallyIncreasingTimeInMs() + 50.0;
     while (!compilation_state->failed()) {
       if (!FetchAndExecuteCompilationUnit(&env, native_module_,
                                           compilation_state, &detected_features,
                                           counters_)) {
         break;
       }
+      if (deadline < MonotonicallyIncreasingTimeInMs()) {
+        compilation_state->ReportDetectedFeatures(detected_features);
+        compilation_state->RestartBackgroundCompileTask();
+        return;
+      }
     }
     compilation_state->OnBackgroundTaskStopped(detected_features);
   }
@@ -1715,6 +1723,31 @@ void CompilationStateImpl::OnFinishedUnit(ExecutionTier tier, WasmCode* code) {
   }
 }
 
+void CompilationStateImpl::RestartBackgroundCompileTask() {
+  auto task = base::make_unique<BackgroundCompileTask>(
+      &background_task_manager_, native_module_, isolate_->counters());
+
+  // If --wasm-num-compilation-tasks=0 is passed, do only spawn foreground
+  // tasks. This is used to make timing deterministic.
+  if (FLAG_wasm_num_compilation_tasks == 0) {
+    foreground_task_runner_->PostTask(std::move(task));
+    return;
+  }
+
+  if (baseline_compilation_finished()) {
+    V8::GetCurrentPlatform()->CallLowPriorityTaskOnWorkerThread(
+        std::move(task));
+  } else {
+    V8::GetCurrentPlatform()->CallOnWorkerThread(std::move(task));
+  }
+}
+
+void CompilationStateImpl::ReportDetectedFeatures(
+    const WasmFeatures& detected) {
+  base::MutexGuard guard(&mutex_);
+  UnionFeaturesInto(&detected_features_, detected);
+}
+
 void CompilationStateImpl::OnBackgroundTaskStopped(
     const WasmFeatures& detected) {
   base::MutexGuard guard(&mutex_);
@@ -1750,16 +1783,7 @@ void CompilationStateImpl::RestartBackgroundTasks(size_t max) {
   }
 
   for (; num_restart > 0; --num_restart) {
-    auto task = base::make_unique<BackgroundCompileTask>(
-        &background_task_manager_, native_module_, isolate_->counters());
-
-    // If --wasm-num-compilation-tasks=0 is passed, do only spawn foreground
-    // tasks. This is used to make timing deterministic.
-    if (FLAG_wasm_num_compilation_tasks > 0) {
-      V8::GetCurrentPlatform()->CallOnWorkerThread(std::move(task));
-    } else {
-      foreground_task_runner_->PostTask(std::move(task));
-    }
+    RestartBackgroundCompileTask();
   }
 }
 

test/unittests/cancelable-tasks-unittest.cc

@@ -221,6 +221,7 @@ TEST_F(CancelableTaskManagerTest, RemoveUnmanagedId) {
 
 TEST_F(CancelableTaskManagerTest, EmptyTryAbortAll) {
   EXPECT_EQ(TryAbortResult::kTaskRemoved, TryAbortAll());
+  CancelAndWait();
 }
 
 TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksNotRunTryAbortAll) {
@@ -236,6 +237,7 @@ TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksNotRunTryAbortAll) {
   runner2.Join();
   EXPECT_EQ(0u, result1);
   EXPECT_EQ(0u, result2);
+  CancelAndWait();
 }
 
 TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksStartedTryAbortAll) {
@@ -258,6 +260,7 @@ TEST_F(CancelableTaskManagerTest, ThreadedMultipleTasksStartedTryAbortAll) {
   runner2.Join();
   EXPECT_EQ(1u, result1);
   EXPECT_EQ(0u, result2);
+  CancelAndWait();
 }
 
 }  // namespace internal