Implement async AbortAll for the compiler dispatcher

test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler-dispatcher/compiler-dispatcher.h"
 
 #include "include/v8-platform.h"
 #include "src/base/platform/semaphore.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-job.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-tracer.h"
@@ skipping 52 matching lines @@
   static bool old_flag_;
   DISALLOW_COPY_AND_ASSIGN(IgnitionCompilerDispatcherTest);
 };
 
 bool IgnitionCompilerDispatcherTest::old_flag_;
 
 namespace {
 
 class MockPlatform : public v8::Platform {
  public:
-  MockPlatform() : idle_task_(nullptr), time_(0.0), time_step_(0.0), sem_(0) {}
+  MockPlatform() : time_(0.0), time_step_(0.0), idle_task_(nullptr), sem_(0) {}
   ~MockPlatform() override {
-    EXPECT_TRUE(tasks_.empty());
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    EXPECT_TRUE(foreground_tasks_.empty());
+    EXPECT_TRUE(background_tasks_.empty());
     EXPECT_TRUE(idle_task_ == nullptr);
   }
 
   size_t NumberOfAvailableBackgroundThreads() override { return 1; }
 
   void CallOnBackgroundThread(Task* task,
                               ExpectedRuntime expected_runtime) override {
-    tasks_.push_back(task);
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    background_tasks_.push_back(task);
   }
 
   void CallOnForegroundThread(v8::Isolate* isolate, Task* task) override {
-    UNREACHABLE();
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    foreground_tasks_.push_back(task);
   }
 
   void CallDelayedOnForegroundThread(v8::Isolate* isolate, Task* task,
                                      double delay_in_seconds) override {
     UNREACHABLE();
   }
 
   void CallIdleOnForegroundThread(v8::Isolate* isolate,
                                   IdleTask* task) override {
+    base::LockGuard<base::Mutex> lock(&mutex_);
     ASSERT_TRUE(idle_task_ == nullptr);
     idle_task_ = task;
   }
 
   bool IdleTasksEnabled(v8::Isolate* isolate) override { return true; }
 
   double MonotonicallyIncreasingTime() override {
     time_ += time_step_;
     return time_;
   }
 
   void RunIdleTask(double deadline_in_seconds, double time_step) {
-    ASSERT_TRUE(idle_task_ != nullptr);
     time_step_ = time_step;
-    IdleTask* task = idle_task_;
-    idle_task_ = nullptr;
+    IdleTask* task;
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      task = idle_task_;
+      ASSERT_TRUE(idle_task_ != nullptr);
+      idle_task_ = nullptr;
+    }
     task->Run(deadline_in_seconds);
     delete task;
   }
 
-  bool IdleTaskPending() const { return idle_task_; }
+  bool IdleTaskPending() {
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    return idle_task_;
+  }
 
-  bool BackgroundTasksPending() const { return !tasks_.empty(); }
+  bool BackgroundTasksPending() {
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    return !background_tasks_.empty();
+  }
+
+  bool ForegroundTasksPending() {
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    return !foreground_tasks_.empty();
+  }
 
   void RunBackgroundTasksAndBlock(Platform* platform) {
     std::vector<Task*> tasks;
-    tasks.swap(tasks_);
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      tasks.swap(background_tasks_);
+    }
     platform->CallOnBackgroundThread(new TaskWrapper(this, tasks, true),
                                      kShortRunningTask);
     sem_.Wait();
   }
 
   void RunBackgroundTasks(Platform* platform) {
     std::vector<Task*> tasks;
-    tasks.swap(tasks_);
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      tasks.swap(background_tasks_);
+    }
     platform->CallOnBackgroundThread(new TaskWrapper(this, tasks, false),
                                      kShortRunningTask);
   }
 
+  void RunForegroundTasks() {
+    std::vector<Task*> tasks;
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      tasks.swap(foreground_tasks_);
+    }
+    for (auto& task : tasks) {
+      task->Run();
+      delete task;
+    }
+  }
+
   void ClearBackgroundTasks() {
     std::vector<Task*> tasks;
-    tasks.swap(tasks_);
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      tasks.swap(background_tasks_);
+    }
+    for (auto& task : tasks) {
+      delete task;
+    }
+  }
+
+  void ClearForegroundTasks() {
+    std::vector<Task*> tasks;
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      tasks.swap(foreground_tasks_);
+    }
     for (auto& task : tasks) {
       delete task;
     }
   }
 
   void ClearIdleTask() {
+    base::LockGuard<base::Mutex> lock(&mutex_);
     ASSERT_TRUE(idle_task_ != nullptr);
     delete idle_task_;
     idle_task_ = nullptr;
   }
 
  private:
   class TaskWrapper : public Task {
    public:
     TaskWrapper(MockPlatform* platform, const std::vector<Task*>& tasks,
                 bool signal)
         : platform_(platform), tasks_(tasks), signal_(signal) {}
     ~TaskWrapper() = default;
 
     void Run() override {
       for (auto& task : tasks_) {
         task->Run();
         delete task;
       }
       if (signal_) platform_->sem_.Signal();
     }
 
    private:
     MockPlatform* platform_;
     std::vector<Task*> tasks_;
     bool signal_;
 
     DISALLOW_COPY_AND_ASSIGN(TaskWrapper);
   };
 
-  IdleTask* idle_task_;
   double time_;
   double time_step_;
 
-  std::vector<Task*> tasks_;
+  // Protects all *_tasks_.
+  base::Mutex mutex_;
+
+  IdleTask* idle_task_;
+  std::vector<Task*> background_tasks_;
+  std::vector<Task*> foreground_tasks_;
+
   base::Semaphore sem_;
 
   DISALLOW_COPY_AND_ASSIGN(MockPlatform);
 };
 
 }  // namespace
 
 TEST_F(CompilerDispatcherTest, Construct) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
@@ skipping 263 matching lines @@
   ASSERT_FALSE(dispatcher.FinishNow(shared));
 
   ASSERT_FALSE(dispatcher.IsEnqueued(shared));
   ASSERT_FALSE(shared->is_compiled());
   ASSERT_TRUE(i_isolate()->has_pending_exception());
 
   i_isolate()->clear_pending_exception();
   platform.ClearIdleTask();
 }
 
+TEST_F(IgnitionCompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) {
+  MockPlatform platform;
+  CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
+
+  const char script[] =
+      "function g() { var y = 1; function f11(x) { return x * y }; return f11; "
+      "} g();";
+  Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), script));
+  Handle<SharedFunctionInfo> shared(f->shared(), i_isolate());
+
+  ASSERT_FALSE(platform.IdleTaskPending());
+  ASSERT_TRUE(dispatcher.Enqueue(shared));
+  ASSERT_TRUE(platform.IdleTaskPending());
+
+  ASSERT_EQ(dispatcher.jobs_.size(), 1u);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kInitial);
+
+  // Make compiling super expensive, and advance job as much as possible on the
+  // foreground thread.
+  dispatcher.tracer_->RecordCompile(50000.0, 1);
+  platform.RunIdleTask(10.0, 0.0);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kReadyToCompile);
+
+  ASSERT_TRUE(dispatcher.IsEnqueued(shared));
+  ASSERT_FALSE(shared->is_compiled());
+  ASSERT_FALSE(platform.IdleTaskPending());
+  ASSERT_TRUE(platform.BackgroundTasksPending());
+
+  // The background task hasn't yet started, so we can just cancel it.
+  dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kDontBlock);
+  ASSERT_FALSE(platform.ForegroundTasksPending());
+
+  ASSERT_FALSE(dispatcher.IsEnqueued(shared));
+  ASSERT_FALSE(shared->is_compiled());
+
+  platform.RunBackgroundTasksAndBlock(V8::GetCurrentPlatform());
+
+  if (platform.IdleTaskPending()) platform.ClearIdleTask();
+  ASSERT_FALSE(platform.BackgroundTasksPending());
+  ASSERT_FALSE(platform.ForegroundTasksPending());
+}
+
+TEST_F(IgnitionCompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) {
+  MockPlatform platform;
+  CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
+
+  const char script1[] =
+      "function g() { var y = 1; function f11(x) { return x * y }; return f11; "
+      "} g();";
+  Handle<JSFunction> f1 = Handle<JSFunction>::cast(RunJS(isolate(), script1));
+  Handle<SharedFunctionInfo> shared1(f1->shared(), i_isolate());
+
+  const char script2[] =
+      "function g() { var y = 1; function f12(x) { return x * y }; return f12; "
+      "} g();";
+  Handle<JSFunction> f2 = Handle<JSFunction>::cast(RunJS(isolate(), script2));
+  Handle<SharedFunctionInfo> shared2(f2->shared(), i_isolate());
+
+  ASSERT_FALSE(platform.IdleTaskPending());
+  ASSERT_TRUE(dispatcher.Enqueue(shared1));
+  ASSERT_TRUE(platform.IdleTaskPending());
+
+  ASSERT_EQ(dispatcher.jobs_.size(), 1u);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kInitial);
+
+  // Make compiling super expensive, and advance job as much as possible on the
+  // foreground thread.
+  dispatcher.tracer_->RecordCompile(50000.0, 1);
+  platform.RunIdleTask(10.0, 0.0);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kReadyToCompile);
+
+  ASSERT_TRUE(dispatcher.IsEnqueued(shared1));
+  ASSERT_FALSE(shared1->is_compiled());
+  ASSERT_FALSE(platform.IdleTaskPending());
+  ASSERT_TRUE(platform.BackgroundTasksPending());
+
+  // Kick off background tasks, racing with the AbortAll() call.

[marja, 2017/01/04 12:54:43]

Instead of making your tests racy, you could use condition variables to ensure
that things happen in the order you want to. The task would wait on a condition
variable, and then you'd call your func.

[jochen (gone - plz use gerrit), 2017/01/04 13:18:31]

ah, the comment is just wrong, it's no longer a race

+  dispatcher.block_for_testing_.SetValue(true);
+  platform.RunBackgroundTasks(V8::GetCurrentPlatform());
+
+  // Busy loop until the background task started running.
+  while (dispatcher.block_for_testing_.Value()) {

[marja, 2017/01/04 12:54:43]

Here too you can wait on a condition variable.

[jochen (gone - plz use gerrit), 2017/01/04 13:18:31]

then I'd need even more members just for testing...

[marja, 2017/01/04 13:28:36]

Ah, right, because you can't mock the task and just test the dispatcher. And the
task just calls the real compilation interface (instead of calling something on
the mock platform), so can't mock there either.

+  }
+  dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kDontBlock);
+  ASSERT_TRUE(platform.ForegroundTasksPending());
+
+  // We can't schedule new tasks while we're aborting.
+  ASSERT_FALSE(dispatcher.Enqueue(shared2));
+
+  // Run the first AbortTask. Since the background job is still pending, it
+  // can't do anything.
+  platform.RunForegroundTasks();
+  {
+    base::LockGuard<base::Mutex> lock(&dispatcher.mutex_);
+    ASSERT_TRUE(dispatcher.abort_);
+  }
+
+  // Release background task.
+  dispatcher.semaphore_for_testing_.Signal();
+
+  // Busy loop until the background task scheduled another AbortTask task.

[marja, 2017/01/04 12:54:43]

Ditto

+  while (!platform.ForegroundTasksPending()) {
+  }
+
+  platform.RunForegroundTasks();
+  ASSERT_TRUE(dispatcher.jobs_.empty());
+  {
+    base::LockGuard<base::Mutex> lock(&dispatcher.mutex_);
+    ASSERT_FALSE(dispatcher.abort_);
+  }
+
+  ASSERT_TRUE(platform.IdleTaskPending());
+  platform.RunIdleTask(5.0, 1.0);
+  ASSERT_FALSE(platform.BackgroundTasksPending());
+  ASSERT_FALSE(platform.ForegroundTasksPending());
+
+  // Now it's possible to enqueue new functions again.
+  ASSERT_TRUE(dispatcher.Enqueue(shared2));
+  ASSERT_TRUE(platform.IdleTaskPending());
+  ASSERT_FALSE(platform.BackgroundTasksPending());
+  ASSERT_FALSE(platform.ForegroundTasksPending());
+  platform.ClearIdleTask();
+}
+
+TEST_F(IgnitionCompilerDispatcherTest, FinishNowDuringAbortAll) {
+  MockPlatform platform;
+  CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
+
+  const char script[] =
+      "function g() { var y = 1; function f13(x) { return x * y }; return f13; "
+      "} g();";
+  Handle<JSFunction> f = Handle<JSFunction>::cast(RunJS(isolate(), script));
+  Handle<SharedFunctionInfo> shared(f->shared(), i_isolate());
+
+  ASSERT_FALSE(platform.IdleTaskPending());
+  ASSERT_TRUE(dispatcher.Enqueue(shared));
+  ASSERT_TRUE(platform.IdleTaskPending());
+
+  ASSERT_EQ(dispatcher.jobs_.size(), 1u);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kInitial);
+
+  // Make compiling super expensive, and advance job as much as possible on the
+  // foreground thread.
+  dispatcher.tracer_->RecordCompile(50000.0, 1);
+  platform.RunIdleTask(10.0, 0.0);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kReadyToCompile);
+
+  ASSERT_TRUE(dispatcher.IsEnqueued(shared));
+  ASSERT_FALSE(shared->is_compiled());
+  ASSERT_FALSE(platform.IdleTaskPending());
+  ASSERT_TRUE(platform.BackgroundTasksPending());
+
+  // Kick off background tasks, racing with the AbortAll() call.

[marja, 2017/01/04 12:54:43]

Ditto

+  dispatcher.block_for_testing_.SetValue(true);
+  platform.RunBackgroundTasks(V8::GetCurrentPlatform());
+
+  // Busy loop until the background task started running.
+  while (dispatcher.block_for_testing_.Value()) {
+  }
+  dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kDontBlock);
+  ASSERT_TRUE(platform.ForegroundTasksPending());
+
+  // Run the first AbortTask. Since the background job is still pending, it
+  // can't do anything.
+  platform.RunForegroundTasks();
+  {
+    base::LockGuard<base::Mutex> lock(&dispatcher.mutex_);
+    ASSERT_TRUE(dispatcher.abort_);
+  }
+
+  // While the background thread holds on to a job, it is still enqueud.
+  ASSERT_TRUE(dispatcher.IsEnqueued(shared));
+
+  // Release background task.
+  dispatcher.semaphore_for_testing_.Signal();
+
+  // Force the compilation to finish, even while aborting.
+  ASSERT_TRUE(dispatcher.FinishNow(shared));
+  ASSERT_TRUE(dispatcher.jobs_.empty());
+  {
+    base::LockGuard<base::Mutex> lock(&dispatcher.mutex_);
+    ASSERT_FALSE(dispatcher.abort_);
+  }
+
+  ASSERT_TRUE(platform.ForegroundTasksPending());
+  ASSERT_TRUE(platform.IdleTaskPending());
+  ASSERT_FALSE(platform.BackgroundTasksPending());
+  platform.ClearForegroundTasks();
+  platform.ClearIdleTask();
+}
+
 }  // namespace internal
 }  // namespace v8