[Compiler] Process multiple jobs in background compiler dispatcher task.

Spinning up a new background task is expensive, and many times an existing
task will finish it's work before a new task starts work on a job, so enable
the existing tasks to do more than one background job.

BUG=v8:5203

Change-Id: Ibbef317c8bb3921c36a096fed88d244716be9c42
Reviewed-on: https://chromium-review.googlesource.com/441706
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43593}

src/compiler-dispatcher/compiler-dispatcher.cc

@@ -224,7 +224,7 @@ CompilerDispatcher::CompilerDispatcher(Isolate* isolate, Platform* platform,
       memory_pressure_level_(MemoryPressureLevel::kNone),
       abort_(false),
       idle_task_scheduled_(false),
-      num_scheduled_background_tasks_(0),
+      num_background_tasks_(0),
       main_thread_blocking_on_job_(nullptr),
       block_for_testing_(false),
       semaphore_for_testing_(0) {
@@ -567,10 +567,10 @@ void CompilerDispatcher::ScheduleMoreBackgroundTasksIfNeeded() {
     base::LockGuard<base::Mutex> lock(&mutex_);
     if (pending_background_jobs_.empty()) return;
     if (platform_->NumberOfAvailableBackgroundThreads() <=
-        num_scheduled_background_tasks_) {
+        num_background_tasks_) {
       return;
     }
-    ++num_scheduled_background_tasks_;
+    ++num_background_tasks_;
   }
   platform_->CallOnBackgroundThread(
       new BackgroundTask(isolate_, task_manager_.get(), this),
@@ -578,38 +578,47 @@ void CompilerDispatcher::ScheduleMoreBackgroundTasksIfNeeded() {
 }
 
 void CompilerDispatcher::DoBackgroundWork() {
-  CompilerDispatcherJob* job = nullptr;
-  {
-    base::LockGuard<base::Mutex> lock(&mutex_);
-    --num_scheduled_background_tasks_;
-    if (!pending_background_jobs_.empty()) {
-      auto it = pending_background_jobs_.begin();
-      job = *it;
-      pending_background_jobs_.erase(it);
-      running_background_jobs_.insert(job);
+  for (;;) {
+    CompilerDispatcherJob* job = nullptr;
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      if (!pending_background_jobs_.empty()) {
+        auto it = pending_background_jobs_.begin();
+        job = *it;
+        pending_background_jobs_.erase(it);
+        running_background_jobs_.insert(job);
+      }
     }
-  }
-  if (job == nullptr) return;
+    if (job == nullptr) break;
 
-  if (V8_UNLIKELY(block_for_testing_.Value())) {
-    block_for_testing_.SetValue(false);
-    semaphore_for_testing_.Wait();
-  }
+    if (V8_UNLIKELY(block_for_testing_.Value())) {
+      block_for_testing_.SetValue(false);
+      semaphore_for_testing_.Wait();
+    }
 
-  if (trace_compiler_dispatcher_) {
-    PrintF("CompilerDispatcher: doing background work\n");
-  }
+    if (trace_compiler_dispatcher_) {
+      PrintF("CompilerDispatcher: doing background work\n");
+    }
 
-  DoNextStepOnBackgroundThread(job);
+    DoNextStepOnBackgroundThread(job);
+    // Unconditionally schedule an idle task, as all background steps have to be
+    // followed by a main thread step.
+    ScheduleIdleTaskFromAnyThread();
 
-  ScheduleMoreBackgroundTasksIfNeeded();
-  // Unconditionally schedule an idle task, as all background steps have to be
-  // followed by a main thread step.
-  ScheduleIdleTaskFromAnyThread();
+    {
+      base::LockGuard<base::Mutex> lock(&mutex_);
+      running_background_jobs_.erase(job);
+
+      if (main_thread_blocking_on_job_ == job) {
+        main_thread_blocking_on_job_ = nullptr;
+        main_thread_blocking_signal_.NotifyOne();
+      }
+    }
+  }
 
   {
     base::LockGuard<base::Mutex> lock(&mutex_);
-    running_background_jobs_.erase(job);
+    --num_background_tasks_;
 
     if (running_background_jobs_.empty() && abort_) {
       // This is the last background job that finished. The abort task
@@ -617,11 +626,6 @@ void CompilerDispatcher::DoBackgroundWork() {
       // one to be on the safe side.
       ScheduleAbortTask();
     }
-
-    if (main_thread_blocking_on_job_ == job) {
-      main_thread_blocking_on_job_ = nullptr;
-      main_thread_blocking_signal_.NotifyOne();
-    }
   }
   // Don't touch |this| anymore after this point, as it might have been
   // deleted.

src/compiler-dispatcher/compiler-dispatcher.h

@@ -127,6 +127,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
   FRIEND_TEST(CompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask);
   FRIEND_TEST(CompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask);
   FRIEND_TEST(CompilerDispatcherTest, FinishNowDuringAbortAll);
+  FRIEND_TEST(CompilerDispatcherTest, CompileMultipleOnBackgroundThread);
 
   typedef std::multimap<std::pair<int, int>,
                         std::unique_ptr<CompilerDispatcherJob>>
@@ -173,8 +174,8 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
 
   bool idle_task_scheduled_;
 
-  // Number of currently scheduled BackgroundTask objects.
-  size_t num_scheduled_background_tasks_;
+  // Number of scheduled or running BackgroundTask objects.
+  size_t num_background_tasks_;
 
   // The set of CompilerDispatcherJobs that can be advanced on any thread.
   std::unordered_set<CompilerDispatcherJob*> pending_background_jobs_;

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

@@ -1102,5 +1102,68 @@ TEST_F(CompilerDispatcherTest, EnqueueAndStepTwice) {
   platform.ClearBackgroundTasks();
 }
 
+TEST_F(CompilerDispatcherTest, CompileMultipleOnBackgroundThread) {
+  MockPlatform platform;
+  CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
+
+  const char script1[] =
+      "function g() { var y = 1; function f19(x) { return x * y }; "
+      "return f19; } 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 f20(x) { return x * y }; "
+      "return f20; } 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(dispatcher.Enqueue(shared2));
+  ASSERT_TRUE(platform.IdleTaskPending());
+
+  ASSERT_EQ(dispatcher.jobs_.size(), 2u);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kInitial);
+  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_EQ(dispatcher.jobs_.size(), 2u);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kReadyToCompile);
+  ASSERT_TRUE((++dispatcher.jobs_.begin())->second->status() ==
+              CompileJobStatus::kReadyToCompile);
+
+  ASSERT_TRUE(dispatcher.IsEnqueued(shared1));
+  ASSERT_TRUE(dispatcher.IsEnqueued(shared2));
+  ASSERT_FALSE(shared1->is_compiled());
+  ASSERT_FALSE(shared2->is_compiled());
+  ASSERT_FALSE(platform.IdleTaskPending());
+  ASSERT_TRUE(platform.BackgroundTasksPending());
+
+  platform.RunBackgroundTasksAndBlock(V8::GetCurrentPlatform());
+
+  ASSERT_TRUE(platform.IdleTaskPending());
+  ASSERT_FALSE(platform.BackgroundTasksPending());
+  ASSERT_EQ(dispatcher.jobs_.size(), 2u);
+  ASSERT_TRUE(dispatcher.jobs_.begin()->second->status() ==
+              CompileJobStatus::kCompiled);
+  ASSERT_TRUE((++dispatcher.jobs_.begin())->second->status() ==
+              CompileJobStatus::kCompiled);
+
+  // Now grant a lot of idle time and freeze time.
+  platform.RunIdleTask(1000.0, 0.0);
+
+  ASSERT_FALSE(dispatcher.IsEnqueued(shared1));
+  ASSERT_FALSE(dispatcher.IsEnqueued(shared2));
+  ASSERT_TRUE(shared1->is_compiled());
+  ASSERT_TRUE(shared2->is_compiled());
+  ASSERT_FALSE(platform.IdleTaskPending());
+}
+
 }  // namespace internal
 }  // namespace v8