[V8Platform] Remove deprecated Background threads APIs and make new A…

…PIs pure virtual. Also fixup some implementations that were lagging behind per the lack of pure virtual not having enforced everything yet. Also fixed recently introduced PredictablePlatform::CallDelayedOnWorkerThread() to ignore delayed tasks after realizing the intent is to intercept worker tasks instead of sending them to |platform_|. Node.js migrated off these APIs @ v8/node#69 [email protected], [email protected] Bug: chromium:817421 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I92171f213b5fc64ab1f21e8eec72738f5ce228bd Reviewed-on: https://chromium-review.googlesource.com/1045310 Commit-Queue: Gabriel Charette <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Reviewed-by: Andreas Haas <[email protected]> Cr-Commit-Position: refs/heads/master@{#53223}
paul99 · May 16, 2018 · 8f6ffbf · 8f6ffbf
1 parent c6c28f7
commit 8f6ffbf
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 85 deletions.
diff --git a/include/v8-platform.h b/include/v8-platform.h
@@ -246,16 +246,6 @@ class PageAllocator {
  */
 class Platform {
  public:
-  /**
-   * This enum is used to indicate whether a task is potentially long running,
-   * or causes a long wait. The embedder might want to use this hint to decide
-   * whether to execute the task on a dedicated thread.
-   */
-  enum ExpectedRuntime {
-    kShortRunningTask,
-    kLongRunningTask
-  };
-
   virtual ~Platform() = default;
 
   /**
@@ -296,75 +286,19 @@ class Platform {
    * that there are no worker threads available. Note that a value of 0 won't
    * prohibit V8 from posting tasks using |CallOnWorkerThread|.
    */
-  virtual int NumberOfWorkerThreads() {
-    return static_cast<int>(NumberOfAvailableBackgroundThreads());
-  }
-
-  /**
-   * Deprecated. Use NumberOfWorkerThreads() instead.
-   * TODO(gab): Remove this when all embedders override
-   * NumberOfWorkerThreads() instead.
-   */
-  V8_DEPRECATE_SOON(
-      "NumberOfAvailableBackgroundThreads() is deprecated, use "
-      "NumberOfAvailableBackgroundThreads() instead.",
-      virtual size_t NumberOfAvailableBackgroundThreads()) {
-    return 0;
-  }
+  virtual int NumberOfWorkerThreads() = 0;
 
   /**
    * Returns a TaskRunner which can be used to post a task on the foreground.
    * This function should only be called from a foreground thread.
    */
   virtual std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner(
-      Isolate* isolate) {
-    // TODO(ahaas): Make this function abstract after it got implemented on all
-    // platforms.
-    return {};
-  }
-
-  /**
-   * Returns a TaskRunner which can be used to post a task on a background.
-   * This function should only be called from a foreground thread.
-   */
-  V8_DEPRECATE_SOON(
-      "GetBackgroundTaskRunner() is deprecated, use "
-      "CallOnWorkerThread() instead.",
-      virtual std::shared_ptr<v8::TaskRunner> GetBackgroundTaskRunner(
-          Isolate* isolate)) {
-    // Deprecated and unused.
-    // TODO(gab): Remove this when embedders no longer override it.
-    abort();
-  }
-
-  /**
-   * Schedules a task to be invoked on a background thread. |expected_runtime|
-   * indicates that the task will run a long time. The Platform implementation
-   * takes ownership of |task|. There is no guarantee about order of execution
-   * of tasks wrt order of scheduling, nor is there a guarantee about the
-   * thread the task will be run on.
-   */
-  V8_DEPRECATE_SOON(
-      "ExpectedRuntime is deprecated, use CallOnWorkerThread() instead.",
-      virtual void CallOnBackgroundThread(Task* task,
-                                          ExpectedRuntime expected_runtime)) {
-    // An implementation needs to be provided here because this is called by the
-    // default implementation below. In practice however, all code either:
-    //  - Overrides the new method (thus not making this call) -- i.e. all v8
-    //    code.
-    //  - Overrides this method (thus not making this call) -- i.e. all
-    //    unadapted embedders.
-    abort();
-  }
+      Isolate* isolate) = 0;
 
   /**
    * Schedules a task to be invoked on a worker thread.
-   * TODO(gab): Make pure virtual when all embedders override this instead of
-   * CallOnBackgroundThread().
    */
-  virtual void CallOnWorkerThread(std::unique_ptr<Task> task) {
-    CallOnBackgroundThread(task.release(), kShortRunningTask);
-  }
+  virtual void CallOnWorkerThread(std::unique_ptr<Task> task) = 0;
 
   /**
    * Schedules a task that blocks the main thread to be invoked with
@@ -379,14 +313,9 @@ class Platform {
   /**
    * Schedules a task to be invoked on a worker thread after |delay_in_seconds|
    * expires.
-   * TODO(gab): Make pure virtual when all embedders override this instead of
-   * GetBackgroundTaskRunner().
    */
   virtual void CallDelayedOnWorkerThread(std::unique_ptr<Task> task,
-                                         double delay_in_seconds) {
-    GetBackgroundTaskRunner(nullptr)->PostDelayedTask(std::move(task),
-                                                      delay_in_seconds);
-  }
+                                         double delay_in_seconds) = 0;
 
   /**
    * Schedules a task to be invoked on a foreground thread wrt a specific
@@ -413,14 +342,14 @@ class Platform {
    * The definition of "foreground" is opaque to V8.
    */
   virtual void CallIdleOnForegroundThread(Isolate* isolate, IdleTask* task) {
-    // TODO(ulan): Make this function abstract after V8 roll in Chromium.
+    // This must be overriden if |IdleTasksEnabled()|.
+    abort();
   }
 
   /**
    * Returns true if idle tasks are enabled for the given |isolate|.
    */
   virtual bool IdleTasksEnabled(Isolate* isolate) {
-    // TODO(ulan): Make this function abstract after V8 roll in Chromium.
     return false;
   }
 

diff --git a/src/d8.cc b/src/d8.cc
@@ -154,9 +154,9 @@ class MockArrayBufferAllocator : public ArrayBufferAllocatorBase {
   }
 };
 
-// Predictable v8::Platform implementation. Background tasks and idle tasks are
-// disallowed, and the time reported by {MonotonicallyIncreasingTime} is
-// deterministic.
+// Predictable v8::Platform implementation. Worker threads are disabled, idle
+// tasks are disallowed, and the time reported by {MonotonicallyIncreasingTime}
+// is deterministic.
 class PredictablePlatform : public Platform {
  public:
   explicit PredictablePlatform(std::unique_ptr<Platform> platform)
@@ -181,6 +181,8 @@ class PredictablePlatform : public Platform {
     return platform_->GetForegroundTaskRunner(isolate);
   }
 
+  int NumberOfWorkerThreads() override { return 0; }
+
   void CallOnWorkerThread(std::unique_ptr<Task> task) override {
     // It's not defined when background tasks are being executed, so we can just
     // execute them right away.
@@ -189,7 +191,7 @@ class PredictablePlatform : public Platform {
 
   void CallDelayedOnWorkerThread(std::unique_ptr<Task> task,
                                  double delay_in_seconds) override {
-    platform_->CallDelayedOnWorkerThread(std::move(task), delay_in_seconds);
+    // Never run delayed tasks.
   }
 
   void CallOnForegroundThread(v8::Isolate* isolate, Task* task) override {

diff --git a/test/cctest/cctest.h b/test/cctest/cctest.h
@@ -680,6 +680,10 @@ class TestPlatform : public v8::Platform {
     return old_platform_->OnCriticalMemoryPressure(length);
   }
 
+  int NumberOfWorkerThreads() override {
+    return old_platform_->NumberOfWorkerThreads();
+  }
+
   std::shared_ptr<v8::TaskRunner> GetForegroundTaskRunner(
       v8::Isolate* isolate) override {
     return old_platform_->GetForegroundTaskRunner(isolate);

diff --git a/test/cctest/heap/test-unmapper.cc b/test/cctest/heap/test-unmapper.cc
@@ -46,10 +46,6 @@ class MockPlatformForUnmapper : public TestPlatform {
     return old_platform_->NumberOfWorkerThreads();
   }
 
-  size_t NumberOfAvailableBackgroundThreads() override {
-    return old_platform_->NumberOfAvailableBackgroundThreads();
-  }
-
  private:
   Task* task_;
   std::vector<std::unique_ptr<Task>> worker_tasks_;

diff --git a/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc b/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc
@@ -110,6 +110,11 @@ class MockPlatform : public v8::Platform {
     worker_tasks_.push_back(std::move(task));
   }
 
+  void CallDelayedOnWorkerThread(std::unique_ptr<Task> task,
+                                 double delay_in_seconds) override {
+    UNREACHABLE();
+  }
+
   void CallOnForegroundThread(v8::Isolate* isolate, Task* task) override {
     base::LockGuard<base::Mutex> lock(&mutex_);
     foreground_tasks_.push_back(std::unique_ptr<Task>(task));