[compiler-dispatcher] Move stepping logic to job

Instead of having the "next step" state machine logic in the compiler
dispatcher, move it to the compiler dispatcher job. This is a
prerequisite to having more generic jobs with slightly different state
sets, such as an optimizing compile job.

Bug: v8:6537
Change-Id: Ib7319222f26dbc0e2afc95573b06d813825afde3
Reviewed-on: https://chromium-review.googlesource.com/554751
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46396}

src/compiler-dispatcher/compiler-dispatcher-job.cc

@@ -179,6 +179,50 @@ bool CompilerDispatcherJob::IsAssociatedWith(
   return *shared_ == *shared;
 }
 
+void CompilerDispatcherJob::StepNextOnMainThread() {
+  switch (status()) {
+    case CompileJobStatus::kInitial:
+      return PrepareToParseOnMainThread();
+
+    case CompileJobStatus::kReadyToParse:
+      return Parse();
+
+    case CompileJobStatus::kParsed:
+      return FinalizeParsingOnMainThread();
+
+    case CompileJobStatus::kReadyToAnalyze:
+      return AnalyzeOnMainThread();
+
+    case CompileJobStatus::kAnalyzed:
+      return PrepareToCompileOnMainThread();
+
+    case CompileJobStatus::kReadyToCompile:
+      return Compile();
+
+    case CompileJobStatus::kCompiled:
+      return FinalizeCompilingOnMainThread();
+
+    case CompileJobStatus::kFailed:
+    case CompileJobStatus::kDone:
+      return;
+  }
+  UNREACHABLE();
+}
+
+void CompilerDispatcherJob::StepNextOnBackgroundThread() {
+  DCHECK(CanStepNextOnAnyThread());
+  switch (status()) {
+    case CompileJobStatus::kReadyToParse:
+      return Parse();
+
+    case CompileJobStatus::kReadyToCompile:
+      return Compile();
+
+    default:
+      UNREACHABLE();
+  }
+}
+
 void CompilerDispatcherJob::PrepareToParseOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kInitial);
@@ -315,7 +359,7 @@ void CompilerDispatcherJob::Parse() {
   }
 }
 
-bool CompilerDispatcherJob::FinalizeParsingOnMainThread() {
+void CompilerDispatcherJob::FinalizeParsingOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kParsed);
   COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kFinalizeParsing);
@@ -367,11 +411,9 @@ bool CompilerDispatcherJob::FinalizeParsingOnMainThread() {
     character_stream_.reset();
   }
   parse_info_->set_deferred_handles(scope.Detach());
-
-  return status_ != CompileJobStatus::kFailed;
 }
 
-bool CompilerDispatcherJob::AnalyzeOnMainThread() {
+void CompilerDispatcherJob::AnalyzeOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kReadyToAnalyze);
   COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kAnalyze);
@@ -393,11 +435,9 @@ bool CompilerDispatcherJob::AnalyzeOnMainThread() {
     }
   }
   compile_info_->set_deferred_handles(scope.Detach());
-
-  return status_ != CompileJobStatus::kFailed;
 }
 
-bool CompilerDispatcherJob::PrepareToCompileOnMainThread() {
+void CompilerDispatcherJob::PrepareToCompileOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kAnalyzed);
   COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kPrepareToCompile);
@@ -407,12 +447,11 @@ bool CompilerDispatcherJob::PrepareToCompileOnMainThread() {
   if (!compile_job_.get()) {
     if (!isolate_->has_pending_exception()) isolate_->StackOverflow();
     status_ = CompileJobStatus::kFailed;
-    return false;
+    return;
   }
 
   CHECK(compile_job_->can_execute_on_background_thread());
   status_ = CompileJobStatus::kReadyToCompile;
-  return true;
 }
 
 void CompilerDispatcherJob::Compile() {
@@ -437,7 +476,7 @@ void CompilerDispatcherJob::Compile() {
   status_ = CompileJobStatus::kCompiled;
 }
 
-bool CompilerDispatcherJob::FinalizeCompilingOnMainThread() {
+void CompilerDispatcherJob::FinalizeCompilingOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kCompiled);
   COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kFinalizeCompiling);
@@ -452,7 +491,7 @@ bool CompilerDispatcherJob::FinalizeCompilingOnMainThread() {
         !Compiler::FinalizeCompilationJob(compile_job_.release())) {
       if (!isolate_->has_pending_exception()) isolate_->StackOverflow();
       status_ = CompileJobStatus::kFailed;
-      return false;
+      return;
     }
   }
 
@@ -462,7 +501,6 @@ bool CompilerDispatcherJob::FinalizeCompilingOnMainThread() {
   parse_info_.reset();
 
   status_ = CompileJobStatus::kDone;
-  return true;
 }
 
 void CompilerDispatcherJob::ResetOnMainThread() {

src/compiler-dispatcher/compiler-dispatcher-job.h

@@ -76,8 +76,6 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
                         size_t max_stack_size);
   ~CompilerDispatcherJob();
 
-  CompileJobStatus status() const { return status_; }
-
   bool has_context() const { return !context_.is_null(); }
   Context* context() { return *context_; }
 
@@ -87,31 +85,26 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
   // function.
   bool IsAssociatedWith(Handle<SharedFunctionInfo> shared) const;
 
-  // Transition from kInitial to kReadyToParse.
-  void PrepareToParseOnMainThread();
-
-  // Transition from kReadyToParse to kParsed (or kDone if there is
-  // finish_callback).
-  void Parse();
-
-  // Transition from kParsed to kReadyToAnalyze (or kFailed). Returns false
-  // when transitioning to kFailed. In that case, an exception is pending.
-  bool FinalizeParsingOnMainThread();
+  bool IsFinished() {
+    return status() == CompileJobStatus::kDone ||
+           status() == CompileJobStatus::kFailed;
+  }
 
-  // Transition from kReadyToAnalyze to kAnalyzed (or kFailed). Returns
-  // false when transitioning to kFailed. In that case, an exception is pending.
-  bool AnalyzeOnMainThread();
+  bool IsFailed() { return status() == CompileJobStatus::kFailed; }
 
-  // Transition from kAnalyzed to kReadyToCompile (or kFailed). Returns
-  // false when transitioning to kFailed. In that case, an exception is pending.
-  bool PrepareToCompileOnMainThread();
+  // Return true if the next step can be run on any thread, that is when both
+  // StepNextOnMainThread and StepNextOnBackgroundThread could be used for the
+  // next step.
+  bool CanStepNextOnAnyThread() {
+    return status() == CompileJobStatus::kReadyToParse ||
+           status() == CompileJobStatus::kReadyToCompile;
+  }
 
-  // Transition from kReadyToCompile to kCompiled.
-  void Compile();
+  // Step the job forward by one state on the main thread.
+  void StepNextOnMainThread();
 
-  // Transition from kCompiled to kDone (or kFailed). Returns false when
-  // transitioning to kFailed. In that case, an exception is pending.
-  bool FinalizeCompilingOnMainThread();
+  // Step the job forward by one state on a background thread.
+  void StepNextOnBackgroundThread();
 
   // Transition from any state to kInitial and free all resources.
   void ResetOnMainThread();
@@ -124,7 +117,10 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
   void ShortPrint();
 
  private:
-  FRIEND_TEST(CompilerDispatcherJobTest, ScopeChain);
+  friend class CompilerDispatcherTest;
+  friend class CompilerDispatcherJobTest;
+
+  CompileJobStatus status() const { return status_; }
 
   CompileJobStatus status_;
   Isolate* isolate_;
@@ -152,6 +148,28 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
 
   bool trace_compiler_dispatcher_jobs_;
 
+  // Transition from kInitial to kReadyToParse.
+  void PrepareToParseOnMainThread();
+
+  // Transition from kReadyToParse to kParsed (or kDone if there is
+  // finish_callback).
+  void Parse();
+
+  // Transition from kParsed to kReadyToAnalyze (or kFailed).
+  void FinalizeParsingOnMainThread();
+
+  // Transition from kReadyToAnalyze to kAnalyzed (or kFailed).
+  void AnalyzeOnMainThread();
+
+  // Transition from kAnalyzed to kReadyToCompile (or kFailed).
+  void PrepareToCompileOnMainThread();
+
+  // Transition from kReadyToCompile to kCompiled.
+  void Compile();
+
+  // Transition from kCompiled to kDone (or kFailed).
+  void FinalizeCompilingOnMainThread();
+
   DISALLOW_COPY_AND_ASSIGN(CompilerDispatcherJob);
 };
 

src/compiler-dispatcher/compiler-dispatcher.cc

@@ -21,16 +21,6 @@ namespace {
 
 enum class ExceptionHandling { kSwallow, kThrow };
 
-bool IsFinished(CompilerDispatcherJob* job) {
-  return job->status() == CompileJobStatus::kDone ||
-         job->status() == CompileJobStatus::kFailed;
-}
-
-bool CanRunOnAnyThread(CompilerDispatcherJob* job) {
-  return job->status() == CompileJobStatus::kReadyToParse ||
-         job->status() == CompileJobStatus::kReadyToCompile;
-}
-
 bool DoNextStepOnMainThread(Isolate* isolate, CompilerDispatcherJob* job,
                             ExceptionHandling exception_handling) {
   DCHECK(ThreadId::Current().Equals(isolate->thread_id()));
@@ -42,69 +32,23 @@ bool DoNextStepOnMainThread(Isolate* isolate, CompilerDispatcherJob* job,
   if (job->has_context()) {
     isolate->set_context(job->context());
   } else {
-    DCHECK(CanRunOnAnyThread(job));
+    DCHECK(job->CanStepNextOnAnyThread());
   }
 
-  switch (job->status()) {
-    case CompileJobStatus::kInitial:
-      job->PrepareToParseOnMainThread();
-      break;
-
-    case CompileJobStatus::kReadyToParse:
-      job->Parse();
-      break;
-
-    case CompileJobStatus::kParsed:
-      job->FinalizeParsingOnMainThread();
-      break;
-
-    case CompileJobStatus::kReadyToAnalyze:
-      job->AnalyzeOnMainThread();
-      break;
-
-    case CompileJobStatus::kAnalyzed:
-      job->PrepareToCompileOnMainThread();
-      break;
+  job->StepNextOnMainThread();
 
-    case CompileJobStatus::kReadyToCompile:
-      job->Compile();
-      break;
-
-    case CompileJobStatus::kCompiled:
-      job->FinalizeCompilingOnMainThread();
-      break;
-
-    case CompileJobStatus::kFailed:
-    case CompileJobStatus::kDone:
-      break;
-  }
-
-  DCHECK_EQ(job->status() == CompileJobStatus::kFailed,
-            isolate->has_pending_exception());
-  if (job->status() == CompileJobStatus::kFailed &&
-      exception_handling == ExceptionHandling::kSwallow) {
+  DCHECK_EQ(job->IsFailed(), isolate->has_pending_exception());
+  if (job->IsFailed() && exception_handling == ExceptionHandling::kSwallow) {
     isolate->clear_pending_exception();
   }
-  return job->status() != CompileJobStatus::kFailed;
+  return job->IsFailed();
 }
 
 void DoNextStepOnBackgroundThread(CompilerDispatcherJob* job) {
-  DCHECK(CanRunOnAnyThread(job));
+  DCHECK(job->CanStepNextOnAnyThread());
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                "V8.CompilerDispatcherBackgroundStep");
-
-  switch (job->status()) {
-    case CompileJobStatus::kReadyToParse:
-      job->Parse();
-      break;
-
-    case CompileJobStatus::kReadyToCompile:
-      job->Compile();
-      break;
-
-    default:
-      UNREACHABLE();
-  }
+  job->StepNextOnBackgroundThread();
 }
 
 // Theoretically we get 50ms of idle time max, however it's unlikely that
@@ -277,7 +221,7 @@ bool CompilerDispatcher::CanEnqueue(Handle<SharedFunctionInfo> function) {
 
 CompilerDispatcher::JobId CompilerDispatcher::Enqueue(
     std::unique_ptr<CompilerDispatcherJob> job) {
-  DCHECK(!IsFinished(job.get()));
+  DCHECK(!job->IsFinished());
   bool added;
   JobMap::const_iterator it;
   std::tie(it, added) =
@@ -293,7 +237,7 @@ CompilerDispatcher::JobId CompilerDispatcher::Enqueue(
 
 CompilerDispatcher::JobId CompilerDispatcher::EnqueueAndStep(
     std::unique_ptr<CompilerDispatcherJob> job) {
-  DCHECK(!IsFinished(job.get()));
+  DCHECK(!job->IsFinished());
   bool added;
   JobMap::const_iterator it;
   std::tie(it, added) =
@@ -461,10 +405,10 @@ bool CompilerDispatcher::FinishNow(CompilerDispatcherJob* job) {
     PrintF(" now\n");
   }
   WaitForJobIfRunningOnBackground(job);
-  while (!IsFinished(job)) {
+  while (!job->IsFinished()) {
     DoNextStepOnMainThread(isolate_, job, ExceptionHandling::kThrow);
   }
-  return job->status() != CompileJobStatus::kFailed;
+  return !job->IsFailed();
 }
 
 bool CompilerDispatcher::FinishNow(Handle<SharedFunctionInfo> function) {
@@ -489,7 +433,7 @@ void CompilerDispatcher::FinishAllNow() {
       pending_background_jobs_.erase(job);
     }
     if (!is_running_in_background) {
-      while (!IsFinished(job)) {
+      while (!job->IsFinished()) {
         DoNextStepOnMainThread(isolate_, job, ExceptionHandling::kThrow);
       }
       it = RemoveIfFinished(it);
@@ -638,7 +582,7 @@ void CompilerDispatcher::ScheduleAbortTask() {
 
 void CompilerDispatcher::ConsiderJobForBackgroundProcessing(
     CompilerDispatcherJob* job) {
-  if (!CanRunOnAnyThread(job)) return;
+  if (!job->CanStepNextOnAnyThread()) return;
   {
     base::LockGuard<base::Mutex> lock(&mutex_);
     pending_background_jobs_.insert(job);
@@ -773,7 +717,7 @@ void CompilerDispatcher::DoIdleWork(double deadline_in_seconds) {
         ConsiderJobForBackgroundProcessing(job->second.get());
       }
       ++job;
-    } else if (IsFinished(job->second.get())) {
+    } else if (job->second->IsFinished()) {
       DCHECK(it == pending_background_jobs_.end());
       lock.reset();
       job = RemoveJob(job);
@@ -794,12 +738,12 @@ void CompilerDispatcher::DoIdleWork(double deadline_in_seconds) {
 
 CompilerDispatcher::JobMap::const_iterator CompilerDispatcher::RemoveIfFinished(
     JobMap::const_iterator job) {
-  if (!IsFinished(job->second.get())) {
+  if (!job->second->IsFinished()) {
     return job;
   }
 
   if (trace_compiler_dispatcher_) {
-    bool result = job->second->status() != CompileJobStatus::kFailed;
+    bool result = !job->second->IsFailed();
     PrintF("CompilerDispatcher: finished working on ");
     job->second->ShortPrint();
     PrintF(": %s\n", result ? "success" : "failure");