[d8] Bring PredictablePlatform in line with default platform

backes · Commit Bot · commit d72768000eab · 2017-05-26T11:31:57.000Z
This removes a lot of special handling for the predictable platform. Instead of executing spawned foreground and background tasks immediately (i.e. inside the scope that spawns the tasks), just add both to the foreground task queue. This avoids existing special handling for predictable mode in wasm async compilation, and should fix current failures on the predictable bot. BUG=v8:6427 Change-Id: Idbaa764a3dc8c230c29f3937d885e12174691ac4 Reviewed-on: https://chromium-review.googlesource.com/509694 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#45538}
diff --git a/src/d8.cc b/src/d8.cc
@@ -192,72 +192,61 @@ class MockArrayBufferAllocator : public ArrayBufferAllocatorBase {
   }
 };
 
-
-// Predictable v8::Platform implementation. All background and foreground
-// tasks are run immediately, delayed tasks are not executed at all.
+// Predictable v8::Platform implementation. Background tasks and idle tasks are
+// disallowed, and the time reported by {MonotonicallyIncreasingTime} is
+// deterministic.
 class PredictablePlatform : public Platform {
  public:
-  PredictablePlatform() {}
+  explicit PredictablePlatform(std::unique_ptr<Platform> platform)
+      : platform_(std::move(platform)) {
+    DCHECK_NOT_NULL(platform_);
+  }
 
   void CallOnBackgroundThread(Task* task,
                               ExpectedRuntime expected_runtime) override {
+    // It's not defined when background tasks are being executed, so we can just
+    // execute them right away.
     task->Run();
     delete task;
   }
 
   void CallOnForegroundThread(v8::Isolate* isolate, Task* task) override {
-    task->Run();
-    delete task;
+    platform_->CallOnForegroundThread(isolate, task);
   }
 
   void CallDelayedOnForegroundThread(v8::Isolate* isolate, Task* task,
                                      double delay_in_seconds) override {
-    delete task;
+    platform_->CallDelayedOnForegroundThread(isolate, task, delay_in_seconds);
   }
 
-  void CallIdleOnForegroundThread(v8::Isolate* isolate,
-                                  IdleTask* task) override {
+  void CallIdleOnForegroundThread(Isolate* isolate, IdleTask* task) override {
     UNREACHABLE();
   }
 
-  bool IdleTasksEnabled(v8::Isolate* isolate) override { return false; }
+  bool IdleTasksEnabled(Isolate* isolate) override { return false; }
 
   double MonotonicallyIncreasingTime() override {
     return synthetic_time_in_sec_ += 0.00001;
   }
 
-  using Platform::AddTraceEvent;
-  uint64_t AddTraceEvent(char phase, const uint8_t* categoryEnabledFlag,
-                         const char* name, const char* scope, uint64_t id,
-                         uint64_t bind_id, int numArgs, const char** argNames,
-                         const uint8_t* argTypes, const uint64_t* argValues,
-                         unsigned int flags) override {
-    return 0;
-  }
-
-  void UpdateTraceEventDuration(const uint8_t* categoryEnabledFlag,
-                                const char* name, uint64_t handle) override {}
-
-  const uint8_t* GetCategoryGroupEnabled(const char* name) override {
-    static uint8_t no = 0;
-    return &no;
-  }
-
-  const char* GetCategoryGroupName(
-      const uint8_t* categoryEnabledFlag) override {
-    static const char* dummy = "dummy";
-    return dummy;
-  }
+  Platform* platform() const { return platform_.get(); }
 
  private:
   double synthetic_time_in_sec_ = 0.0;
+  std::unique_ptr<Platform> platform_;
 
   DISALLOW_COPY_AND_ASSIGN(PredictablePlatform);
 };
 
 
 v8::Platform* g_platform = NULL;
 
+v8::Platform* GetDefaultPlatform() {
+  return i::FLAG_verify_predictable
+             ? static_cast<PredictablePlatform*>(g_platform)->platform()
+             : g_platform;
+}
+
 static Local<Value> Throw(Isolate* isolate, const char* message) {
   return isolate->ThrowException(
       String::NewFromUtf8(isolate, message, NewStringType::kNormal)
@@ -1386,8 +1375,6 @@ void Shell::Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
                  const_cast<v8::FunctionCallbackInfo<v8::Value>*>(&args));
 }
 
-// Note that both WaitUntilDone and NotifyDone are no-op when
-// --verify-predictable. See comment in Shell::EnsureEventLoopInitialized.
 void Shell::WaitUntilDone(const v8::FunctionCallbackInfo<v8::Value>& args) {
   SetWaitUntilDone(args.GetIsolate(), true);
 }
@@ -2764,13 +2751,8 @@ void Shell::CollectGarbage(Isolate* isolate) {
 }
 
 void Shell::EnsureEventLoopInitialized(Isolate* isolate) {
-  // When using PredictablePlatform (i.e. FLAG_verify_predictable),
-  // we don't need event loop support, because tasks are completed
-  // immediately - both background and foreground ones.
-  if (!i::FLAG_verify_predictable) {
-    v8::platform::EnsureEventLoopInitialized(g_platform, isolate);
-    SetWaitUntilDone(isolate, false);
-  }
+  v8::platform::EnsureEventLoopInitialized(GetDefaultPlatform(), isolate);
+  SetWaitUntilDone(isolate, false);
 }
 
 void Shell::SetWaitUntilDone(Isolate* isolate, bool value) {
@@ -2789,29 +2771,32 @@ bool Shell::IsWaitUntilDone(Isolate* isolate) {
 }
 
 void Shell::CompleteMessageLoop(Isolate* isolate) {
-  // See comment in EnsureEventLoopInitialized.
-  if (i::FLAG_verify_predictable) return;
+  Platform* platform = GetDefaultPlatform();
   while (v8::platform::PumpMessageLoop(
-      g_platform, isolate,
+      platform, isolate,
       Shell::IsWaitUntilDone(isolate)
           ? platform::MessageLoopBehavior::kWaitForWork
           : platform::MessageLoopBehavior::kDoNotWait)) {
     isolate->RunMicrotasks();
   }
-  v8::platform::RunIdleTasks(g_platform, isolate,
-                             50.0 / base::Time::kMillisecondsPerSecond);
+  if (platform->IdleTasksEnabled(isolate)) {
+    v8::platform::RunIdleTasks(platform, isolate,
+                               50.0 / base::Time::kMillisecondsPerSecond);
+  }
 }
 
 void Shell::EmptyMessageQueues(Isolate* isolate) {
-  if (i::FLAG_verify_predictable) return;
+  Platform* platform = GetDefaultPlatform();
   // Pump the message loop until it is empty.
   while (v8::platform::PumpMessageLoop(
-      g_platform, isolate, platform::MessageLoopBehavior::kDoNotWait)) {
+      platform, isolate, platform::MessageLoopBehavior::kDoNotWait)) {
     isolate->RunMicrotasks();
   }
   // Run the idle tasks.
-  v8::platform::RunIdleTasks(g_platform, isolate,
-                             50.0 / base::Time::kMillisecondsPerSecond);
+  if (platform->IdleTasksEnabled(isolate)) {
+    v8::platform::RunIdleTasks(platform, isolate,
+                               50.0 / base::Time::kMillisecondsPerSecond);
+  }
 }
 
 class Serializer : public ValueSerializer::Delegate {
@@ -3069,24 +3054,22 @@ int Shell::Main(int argc, char* argv[]) {
           ? v8::platform::InProcessStackDumping::kDisabled
           : v8::platform::InProcessStackDumping::kEnabled;
 
-  g_platform = i::FLAG_verify_predictable
-                   ? new PredictablePlatform()
-                   : v8::platform::CreateDefaultPlatform(
-                         0, v8::platform::IdleTaskSupport::kEnabled,
-                         in_process_stack_dumping);
+  g_platform = v8::platform::CreateDefaultPlatform(
+      0, v8::platform::IdleTaskSupport::kEnabled, in_process_stack_dumping);
+  if (i::FLAG_verify_predictable) {
+    g_platform = new PredictablePlatform(std::unique_ptr<Platform>(g_platform));
+  }
 
   platform::tracing::TracingController* tracing_controller;
-  if (options.trace_enabled) {
+  if (options.trace_enabled && !i::FLAG_verify_predictable) {
     trace_file.open("v8_trace.json");
     tracing_controller = new platform::tracing::TracingController();
     platform::tracing::TraceBuffer* trace_buffer =
         platform::tracing::TraceBuffer::CreateTraceBufferRingBuffer(
             platform::tracing::TraceBuffer::kRingBufferChunks,
             platform::tracing::TraceWriter::CreateJSONTraceWriter(trace_file));
     tracing_controller->Initialize(trace_buffer);
-    if (!i::FLAG_verify_predictable) {
-      platform::SetTracingController(g_platform, tracing_controller);
-    }
+    platform::SetTracingController(g_platform, tracing_controller);
   }
 
   v8::V8::InitializePlatform(g_platform);
@@ -3135,6 +3118,7 @@ int Shell::Main(int argc, char* argv[]) {
   }
 
   Isolate* isolate = Isolate::New(create_params);
+
   D8Console console(isolate);
   {
     Isolate::Scope scope(isolate);
@@ -3204,9 +3188,6 @@ int Shell::Main(int argc, char* argv[]) {
   V8::Dispose();
   V8::ShutdownPlatform();
   delete g_platform;
-  if (i::FLAG_verify_predictable) {
-    delete tracing_controller;
-  }
 
   return result;
 }
diff --git a/src/libplatform/default-platform.cc b/src/libplatform/default-platform.cc
@@ -43,25 +43,25 @@ v8::Platform* CreateDefaultPlatform(
 
 bool PumpMessageLoop(v8::Platform* platform, v8::Isolate* isolate,
                      MessageLoopBehavior behavior) {
-  return reinterpret_cast<DefaultPlatform*>(platform)->PumpMessageLoop(
-      isolate, behavior);
+  return static_cast<DefaultPlatform*>(platform)->PumpMessageLoop(isolate,
+                                                                  behavior);
 }
 
 void EnsureEventLoopInitialized(v8::Platform* platform, v8::Isolate* isolate) {
-  return reinterpret_cast<DefaultPlatform*>(platform)
-      ->EnsureEventLoopInitialized(isolate);
+  return static_cast<DefaultPlatform*>(platform)->EnsureEventLoopInitialized(
+      isolate);
 }
 
 void RunIdleTasks(v8::Platform* platform, v8::Isolate* isolate,
                   double idle_time_in_seconds) {
-  reinterpret_cast<DefaultPlatform*>(platform)->RunIdleTasks(
-      isolate, idle_time_in_seconds);
+  static_cast<DefaultPlatform*>(platform)->RunIdleTasks(isolate,
+                                                        idle_time_in_seconds);
 }
 
 void SetTracingController(
     v8::Platform* platform,
     v8::platform::tracing::TracingController* tracing_controller) {
-  return reinterpret_cast<DefaultPlatform*>(platform)->SetTracingController(
+  return static_cast<DefaultPlatform*>(platform)->SetTracingController(
       tracing_controller);
 }
 
diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc
@@ -2728,10 +2728,6 @@ void wasm::AsyncInstantiate(Isolate* isolate, Handle<JSPromise> promise,
 // foreground task. All other tasks (e.g. decoding and validating, the majority
 // of the work of compilation) can be background tasks.
 // TODO(wasm): factor out common parts of this with the synchronous pipeline.
-//
-// Note: In predictable mode, DoSync and DoAsync execute the referenced function
-// immediately before returning. Thus we handle the predictable mode specially,
-// e.g. when we synchronizing tasks or when we delete the AyncCompileJob.
 class AsyncCompileJob {
   // TODO(ahaas): Fix https://bugs.chromium.org/p/v8/issues/detail?id=6263 to
   // make sure that d8 does not shut down before the AsyncCompileJob is
@@ -2796,14 +2792,14 @@ class AsyncCompileJob {
     RejectPromise(isolate_, context_, thrower, module_promise_);
     // The AsyncCompileJob is finished, we resolved the promise, we do not need
     // the data anymore. We can delete the AsyncCompileJob object.
-    if (!FLAG_verify_predictable) delete this;
+    delete this;
   }
 
   void AsyncCompileSucceeded(Handle<Object> result) {
     ResolvePromise(isolate_, context_, module_promise_, result);
     // The AsyncCompileJob is finished, we resolved the promise, we do not need
     // the data anymore. We can delete the AsyncCompileJob object.
-    if (!FLAG_verify_predictable) delete this;
+    delete this;
   }
 
   enum TaskType { SYNC, ASYNC };
@@ -3008,9 +3004,7 @@ class AsyncCompileJob {
           break;
         }
       }
-      // Special handling for predictable mode, see above.
-      if (!FLAG_verify_predictable)
-        job_->helper_->module_->pending_tasks.get()->Signal();
+      job_->helper_->module_->pending_tasks.get()->Signal();
     }
   };
 
@@ -3090,12 +3084,9 @@ class AsyncCompileJob {
       // Bump next_unit_, such that background tasks stop processing the queue.
       job_->helper_->next_unit_.SetValue(
           job_->helper_->compilation_units_.size());
-      // Special handling for predictable mode, see above.
-      if (!FLAG_verify_predictable) {
-        for (size_t i = 0; i < job_->num_background_tasks_; ++i) {
-          // We wait for it to finish.
-          job_->helper_->module_->pending_tasks.get()->Wait();
-        }
+      for (size_t i = 0; i < job_->num_background_tasks_; ++i) {
+        // We wait for it to finish.
+        job_->helper_->module_->pending_tasks.get()->Wait();
       }
       if (thrower_.error()) {
         job_->DoSync<FailCompile>(std::move(thrower_));
@@ -3256,8 +3247,6 @@ void wasm::AsyncCompile(Isolate* isolate, Handle<JSPromise> promise,
   auto job = new AsyncCompileJob(isolate, std::move(copy), bytes.length(),
                                  handle(isolate->context()), promise);
   job->Start();
-  // Special handling for predictable mode, see above.
-  if (FLAG_verify_predictable) delete job;
 }
 
 Handle<Code> wasm::CompileLazy(Isolate* isolate) {
