[complier] Enable parallel eager inner function compilation with compiler dispatcher.

src/compiler-dispatcher/compiler-dispatcher.cc

Index: src/compiler-dispatcher/compiler-dispatcher.cc
diff --git a/src/compiler-dispatcher/compiler-dispatcher.cc b/src/compiler-dispatcher/compiler-dispatcher.cc
index 38ebf8c15681ab0132117780221c827669fba03b..dbb7e4a86e0138db4959c3db3bd189fb7ebe392e 100644
--- a/src/compiler-dispatcher/compiler-dispatcher.cc
+++ b/src/compiler-dispatcher/compiler-dispatcher.cc
@@ -8,6 +8,7 @@
 #include "include/v8.h"
 #include "src/base/platform/time.h"
 #include "src/cancelable-task.h"
+#include "src/compilation-info.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-job.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-tracer.h"
 #include "src/flags.h"
@@ -36,7 +37,11 @@ bool DoNextStepOnMainThread(Isolate* isolate, CompilerDispatcherJob* job,
       job->FinalizeParsingOnMainThread();
       break;
 
-    case CompileJobStatus::kReadyToAnalyse:
+    case CompileJobStatus::kReadyToAnalyze:
+      job->AnalyzeOnMainThread();
+      break;
+
+    case CompileJobStatus::kAnalyzed:
       job->PrepareToCompileOnMainThread();
       break;
 
@@ -226,7 +231,7 @@ CompilerDispatcher::~CompilerDispatcher() {
   task_manager_->CancelAndWait();
 }
 
-bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function) {
+bool CompilerDispatcher::CanEnqueue(Handle<SharedFunctionInfo> function) {
   if (!IsEnabled()) return false;
 
   if (memory_pressure_level_.Value() != MemoryPressureLevel::kNone) {
@@ -245,12 +250,17 @@ bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function) {
     return false;
   }
 
+  return true;
+}
+
+bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function) {
+  if (!CanEnqueue(function)) return false;
   if (IsEnqueued(function)) return true;
 
   if (trace_compiler_dispatcher_) {
     PrintF("CompilerDispatcher: enqueuing ");
     function->ShortPrint();
-    PrintF("\n");
+    PrintF(" for parse and compile\n");
   }
 
   std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
@@ -277,11 +287,47 @@ bool CompilerDispatcher::EnqueueAndStep(Handle<SharedFunctionInfo> function) {
   return true;
 }
 
-bool CompilerDispatcher::IsEnabled() const {
-  v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
-  return FLAG_compiler_dispatcher && platform_->IdleTasksEnabled(v8_isolate);
+bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function,
+                                 FunctionLiteral* literal,
+                                 AstValueFactory* ast_value_factory) {

[marja, 2017/01/12 16:07:33]

So now we might access this AstValueFactory from a background thread, right?

But the main thread can still add values to it?

That doesn't sound safe. Why is that OK?

Or is the intention that nobody adds values into AstValueFactory after this
point? If that's the case, can you add a debug mechanism for "sealing" the
AstValueFactory (so that all funcs which try to add values will DCHECK if we try
to call them from here on)?

Doesn't compilation also potentially add values (e.g., by accessing the string
constants which might not have been accessed before)?

[rmcilroy, 2017/01/13 11:28:02]

Yes this is a valid concern. Right now it isn't a problem because we only access
the ast_value_factory in the BytecodeGenerator constructor, which happens on the
main thread, however we would like to move this off-thread so we should figure
out a solution. My thought is that since we only access STRING_CONSTANTS, we
could pre-allocate those during isolate initialization, and pre-populate each
ast-value-factory with these constants when they are created, that way we
wouldn't even need to pass the ast-value-factory here, we could instead get the
constants directly from the isolate. I'll have a look into this as a seperate
CL.

[rmcilroy, 2017/01/16 14:18:25]

Removed need for ast-value-factory, does this work for you Marja?

+  if (!CanEnqueue(function)) return false;
+  if (IsEnqueued(function)) return true;
+
+  if (trace_compiler_dispatcher_) {
+    PrintF("CompilerDispatcher: enqueuing ");
+    function->ShortPrint();
+    PrintF(" for compile\n");
+  }
+
+  std::unique_ptr<CompilerDispatcherJob> job(
+      new CompilerDispatcherJob(isolate_, tracer_.get(), function, literal,
+                                ast_value_factory, max_stack_size_));
+  std::pair<int, int> key(Script::cast(function->script())->id(),
+                          function->function_literal_id());
+  jobs_.insert(std::make_pair(key, std::move(job)));
+  ScheduleIdleTaskIfNeeded();
+  return true;
 }
 
+bool CompilerDispatcher::EnqueueAndStep(Handle<SharedFunctionInfo> function,
+                                        FunctionLiteral* literal,
+                                        AstValueFactory* ast_value_factory) {
+  if (!Enqueue(function, literal, ast_value_factory)) return false;
+
+  if (trace_compiler_dispatcher_) {
+    PrintF("CompilerDispatcher: stepping ");
+    function->ShortPrint();
+    PrintF("\n");
+  }
+  JobMap::const_iterator job = GetJobFor(function);
+  DoNextStepOnMainThread(isolate_, job->second.get(),
+                         ExceptionHandling::kSwallow);
+  ConsiderJobForBackgroundProcessing(job->second.get());
+  return true;
+}
+
+bool CompilerDispatcher::IsEnabled() const { return FLAG_compiler_dispatcher; }
+
 bool CompilerDispatcher::IsEnqueued(Handle<SharedFunctionInfo> function) const {
   return GetJobFor(function) != jobs_.end();
 }
@@ -302,6 +348,16 @@ void CompilerDispatcher::WaitForJobIfRunningOnBackground(
   DCHECK(running_background_jobs_.find(job) == running_background_jobs_.end());
 }
 
+bool CompilerDispatcher::FinishNow(CompilerDispatcherJob* job) {
+  WaitForJobIfRunningOnBackground(job);
+  while (!IsFinished(job)) {
+    DoNextStepOnMainThread(isolate_, job, ExceptionHandling::kThrow);
+  }
+  bool result = job->status() != CompileJobStatus::kFailed;
+  job->ResetOnMainThread();
+  return result;
+}
+
 bool CompilerDispatcher::FinishNow(Handle<SharedFunctionInfo> function) {
   JobMap::const_iterator job = GetJobFor(function);
   CHECK(job != jobs_.end());
@@ -312,12 +368,7 @@ bool CompilerDispatcher::FinishNow(Handle<SharedFunctionInfo> function) {
     PrintF(" now\n");
   }
 
-  WaitForJobIfRunningOnBackground(job->second.get());
-  while (!IsFinished(job->second.get())) {
-    DoNextStepOnMainThread(isolate_, job->second.get(),
-                           ExceptionHandling::kThrow);
-  }
-  bool result = job->second->status() != CompileJobStatus::kFailed;
+  bool result = FinishNow(job->second.get());
 
   if (trace_compiler_dispatcher_) {
     PrintF("CompilerDispatcher: finished working on ");
@@ -326,7 +377,6 @@ bool CompilerDispatcher::FinishNow(Handle<SharedFunctionInfo> function) {
     tracer_->DumpStatistics();
   }
 
-  job->second->ResetOnMainThread();
   jobs_.erase(job);
   if (jobs_.empty()) {
     base::LockGuard<base::Mutex> lock(&mutex_);
@@ -335,6 +385,30 @@ bool CompilerDispatcher::FinishNow(Handle<SharedFunctionInfo> function) {
   return result;
 }
 
+bool CompilerDispatcher::FinishAllNow() {
+  if (trace_compiler_dispatcher_) {
+    PrintF("CompilerDispatcher: finishing all jobs now\n");
+  }
+
+  bool result = true;
+  for (auto& it : jobs_) {
+    result &= FinishNow(it.second.get());

[jochen (gone - plz use gerrit), 2017/01/12 13:13:36]

does that work? FinishNow() should delete it from jobs_, at which point we can't
advance it anymore?

[rmcilroy, 2017/01/12 13:59:47]

This is calling the new FinishNowwith a Job argument, which doesn't delete it
from jobs_

+  }
+
+  if (trace_compiler_dispatcher_) {
+    PrintF("CompilerDispatcher: finished all jobs\n");
+  }
+
+  jobs_.clear();

[jochen (gone - plz use gerrit), 2017/01/12 13:13:36]

jobs_ should already be empty here, no?

[rmcilroy, 2017/01/12 13:59:47]

As above, the jobs are not deleted by the FinishNow being called here.

+  {
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    DCHECK(pending_background_jobs_.empty());
+    DCHECK(running_background_jobs_.empty());
+    abort_ = false;
+  }
+  return result;
+}
+
 void CompilerDispatcher::AbortAll(BlockingBehavior blocking) {
   bool background_tasks_running =
       task_manager_->TryAbortAll() == CancelableTaskManager::kTaskRunning;