Use idle time to make progress on scheduled compilation jobs

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 8d503780cd5511b7af36cbbf865e6037277a5282..bed64a5ddb979a340af1e6b1ac381ec4bbbe3507 100644
--- a/src/compiler-dispatcher/compiler-dispatcher.cc
+++ b/src/compiler-dispatcher/compiler-dispatcher.cc
@@ -4,6 +4,10 @@
 
 #include "src/compiler-dispatcher/compiler-dispatcher.h"
 
+#include "include/v8-platform.h"
+#include "include/v8.h"
+#include "src/base/platform/time.h"
+#include "src/cancelable-task.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-job.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-tracer.h"
 #include "src/objects-inl.h"
@@ -13,7 +17,12 @@ namespace internal {
 
 namespace {
 
-bool DoNextStepOnMainThread(CompilerDispatcherJob* job) {
+enum class ExceptionHandling { kSwallow, kKeep };
+
+bool DoNextStepOnMainThread(Isolate* isolate, CompilerDispatcherJob* job,
+                            ExceptionHandling exception_handling) {
+  DCHECK(ThreadId::Current().Equals(isolate->thread_id()));
+  v8::TryCatch try_catch(reinterpret_cast<v8::Isolate*>(isolate));
   switch (job->status()) {
     case CompileJobStatus::kInitial:
       job->PrepareToParseOnMainThread();
@@ -44,6 +53,10 @@ bool DoNextStepOnMainThread(CompilerDispatcherJob* job) {
       break;
   }
 
+  if (exception_handling == ExceptionHandling::kKeep && try_catch.HasCaught()) {
+    DCHECK(job->status() == CompileJobStatus::kFailed);
+    try_catch.ReThrow();
+  }
   return job->status() != CompileJobStatus::kFailed;
 }
 
@@ -52,14 +65,48 @@ bool IsFinished(CompilerDispatcherJob* job) {
          job->status() == CompileJobStatus::kFailed;
 }
 
+// Theoretically we get 50ms of idle time max, however it's unlikely that
+// we'll get all of it so try to be a conservative.
+const double kMaxIdleTimeToExpectInMs = 40;
+
 }  // namespace
 
-CompilerDispatcher::CompilerDispatcher(Isolate* isolate, size_t max_stack_size)
+class CompilerDispatcher::IdleTask : public CancelableIdleTask {
+ public:
+  IdleTask(Isolate* isolate, CompilerDispatcher* dispatcher);
+  ~IdleTask() override;
+
+  // CancelableIdleTask implementation.
+  void RunInternal(double deadline_in_seconds) override;
+
+ private:
+  CompilerDispatcher* dispatcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(IdleTask);
+};
+
+CompilerDispatcher::IdleTask::IdleTask(Isolate* isolate,
+                                       CompilerDispatcher* dispatcher)
+    : CancelableIdleTask(isolate), dispatcher_(dispatcher) {}
+
+CompilerDispatcher::IdleTask::~IdleTask() {}
+
+void CompilerDispatcher::IdleTask::RunInternal(double deadline_in_seconds) {
+  dispatcher_->DoIdleWork(deadline_in_seconds);
+}
+
+CompilerDispatcher::CompilerDispatcher(Isolate* isolate, Platform* platform,
+                                       size_t max_stack_size)
     : isolate_(isolate),
+      platform_(platform),
       max_stack_size_(max_stack_size),
-      tracer_(new CompilerDispatcherTracer(isolate_)) {}
+      tracer_(new CompilerDispatcherTracer(isolate_)),
+      idle_task_scheduled_(false) {}
 
-CompilerDispatcher::~CompilerDispatcher() {}
+CompilerDispatcher::~CompilerDispatcher() {
+  // To avoid crashing in unit tests due to unfished jobs.
+  AbortAll(BlockingBehavior::kBlock);
+}
 
 bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function) {
   // We only handle functions (no eval / top-level code / wasm) that are
@@ -75,6 +122,7 @@ bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function) {
   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;
 }
 
@@ -89,9 +137,11 @@ bool CompilerDispatcher::FinishNow(Handle<SharedFunctionInfo> function) {
   // TODO(jochen): Check if there's an in-flight background task working on this
   // job.
   while (!IsFinished(job->second.get())) {
-    DoNextStepOnMainThread(job->second.get());
+    DoNextStepOnMainThread(isolate_, job->second.get(),
+                           ExceptionHandling::kKeep);
   }
   bool result = job->second->status() != CompileJobStatus::kFailed;
+  job->second->ResetOnMainThread();
   jobs_.erase(job);
   return result;
 }
@@ -104,6 +154,7 @@ void CompilerDispatcher::Abort(Handle<SharedFunctionInfo> function,
 
   // TODO(jochen): Check if there's an in-flight background task working on this
   // job.
+  job->second->ResetOnMainThread();
   jobs_.erase(job);
 }
 
@@ -111,6 +162,9 @@ void CompilerDispatcher::AbortAll(BlockingBehavior blocking) {
   USE(blocking);
   // TODO(jochen): Check if there's an in-flight background task working on this
   // job.
+  for (auto& kv : jobs_) {
+    kv.second->ResetOnMainThread();
+  }
   jobs_.clear();
 }
 
@@ -126,5 +180,57 @@ CompilerDispatcher::JobMap::const_iterator CompilerDispatcher::GetJobFor(
   return jobs_.end();
 }
 
+void CompilerDispatcher::ScheduleIdleTaskIfNeeded() {
+  v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
+  if (!platform_->IdleTasksEnabled(v8_isolate)) return;
+  if (idle_task_scheduled_) return;
+  if (jobs_.empty()) return;
+  idle_task_scheduled_ = true;
+  platform_->CallIdleOnForegroundThread(v8_isolate,
+                                        new IdleTask(isolate_, this));
+}
+
+void CompilerDispatcher::DoIdleWork(double deadline_in_seconds) {
+  idle_task_scheduled_ = false;
+
+  bool work_left = false;
+
+  for (auto job = jobs_.begin(); job != jobs_.end();) {
+    double idle_time_in_seconds =
+        deadline_in_seconds - platform_->MonotonicallyIncreasingTime();
+    if (idle_time_in_seconds <= 0.0) {
+      work_left = true;
+      break;
+    }
+
+    // Advance the current job as much as possible.
+    for (;;) {
+      double estimate_in_ms = job->second->EstimateRuntimeOfNextStepInMs();
+      if (idle_time_in_seconds <
+          (estimate_in_ms /
+           static_cast<double>(base::Time::kMillisecondsPerSecond))) {
+        // If there's not enough time left, try to estimate whether we would
+        // have managed to finish the job in a large idle task to assess
+        // whether we should ask for another idle callback.
+        if (estimate_in_ms <= kMaxIdleTimeToExpectInMs) work_left = true;
+        ++job;
+        break;
+      }
+      DoNextStepOnMainThread(isolate_, job->second.get(),
+                             ExceptionHandling::kSwallow);
+
+      if (IsFinished(job->second.get())) {
+        job->second->ResetOnMainThread();
+        job = jobs_.erase(job);
+        break;
+      }
+
+      idle_time_in_seconds =
+          deadline_in_seconds - platform_->MonotonicallyIncreasingTime();
+    }
+  }

[vogelheim, 2016/12/16 10:45:08]

I like this version much better.


---------------------------------
I'm still bothered by the loop-in-loop and the many loop exits, and toyed around
a bit to find something where we could just have a single loop, that loops over
jobs & time. This is admittedly entirely a matter of taste, so please feel free
to ignore anything below.


// Iterate over available jobs & remaining time.
// For each job, decide whether to 1, skip it (if not enough time), 2, erase it
(if finished), or 3, let it do some work.
size_t hopeless_jobs = 0;
double idle_time_in_seconds = deadline_in_seconds -
platform_->MonotonicallyIncreasingTime();
for (auto job = jobs_.begin();
     job != jobs.end() && idle_time_in_seconds > 0.0;
     idle_time_in_seconds = deadline_in_seconds -
platform_->MonotonicallyIncreasingTime()) {
  double estimate_in_ms = ...
  if (estimate_in_ms <= kMaxIdleTimeToExcpectInMs) {
    // Skip: Job is too large for idle time.
    ++ job;
    ++ hopeless_jobs;
  } else if (idle_time_in_seconds < estimate_...) {
    // Skip: Not enough time left in this idle call.
    ++ job;
  } else if (IsFinished(job->second.get())) {
    // Erase: Job has already finished.
    job->second...
    job = jobs_erase(job);
  } else {
    // Make progress. Then revisit current job.
    DoNextStepOnMainThread(....)
  }
}

if (jobs_.size() > hopeless_jobs) ScheduleIdleTaskIfNeeded();

+  if (work_left) ScheduleIdleTaskIfNeeded();
+}
+
 }  // namespace internal
 }  // namespace v8