From 27f97c7d195e56218a36fbc352ab40382b7999c4 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 19 Nov 2017 14:35:59 +0100 Subject: [PATCH 1/5] deps: cherry-pick c690f54d95802 from V8 upstream Original commit message: [platform] Add TaskRunner to the platform API With the existing platform API it is not possible to post foreground tasks from background tasks. This is, however, required to implement asynchronous compilation for WebAssembly. With this CL we add the concept of a TaskRunner to the platform API. The TaskRunner contains all data needed to post a foreground task and can be used both from a foreground task and a background task. Eventually the TaskRunner should replace the existing API. In addition, this CL contains a default implementation of the TaskRunner. This implementation has tempory workaround for platforms which do not provide a TaskRunner implementation yet. This default implementation should be deleted again when all platforms provide a TaskRunner implementation. R=rmcilroy@chromium.org Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I6ea4a1c9da1eb9a19e8ce8f2163000dbc2598802 Reviewed-on: https://chromium-review.googlesource.com/741588 Commit-Queue: Andreas Haas Reviewed-by: Ross McIlroy Cr-Commit-Position: refs/heads/master@{#49041} Refs: https://github.com/v8/v8/commit/c690f54d9580243c53f7d892fcff1ce6bae4bfc0 --- deps/v8/include/v8-platform.h | 67 +++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/deps/v8/include/v8-platform.h b/deps/v8/include/v8-platform.h index ed2acc3a74e16c..b1a57c9e2c18a5 100644 --- a/deps/v8/include/v8-platform.h +++ b/deps/v8/include/v8-platform.h @@ -36,6 +36,51 @@ class IdleTask { virtual void Run(double deadline_in_seconds) = 0; }; +/** + * A TaskRunner allows scheduling of tasks. The TaskRunner may still be used to + * post tasks after the isolate gets destructed, but these tasks may not get + * executed anymore. All tasks posted to a given TaskRunner will be invoked in + * sequence. Tasks can be posted from any thread. + */ +class TaskRunner { + public: + /** + * Schedules a task to be invoked by this TaskRunner. The TaskRunner + * implementation takes ownership of |task|. + */ + virtual void PostTask(std::unique_ptr task) = 0; + + /** + * Schedules a task to be invoked by this TaskRunner. The task is scheduled + * after the given number of seconds |delay_in_seconds|. The TaskRunner + * implementation takes ownership of |task|. + */ + virtual void PostDelayedTask(std::unique_ptr task, + double delay_in_seconds) = 0; + + /** + * Schedules an idle task to be invoked by this TaskRunner. The task is + * scheduled when the embedder is idle. Requires that + * TaskRunner::SupportsIdleTasks(isolate) is true. Idle tasks may be reordered + * relative to other task types and may be starved for an arbitrarily long + * time if no idle time is available. The TaskRunner implementation takes + * ownership of |task|. + */ + virtual void PostIdleTask(std::unique_ptr task) = 0; + + /** + * Returns true if idle tasks are enabled for this TaskRunner. + */ + virtual bool IdleTasksEnabled() = 0; + + TaskRunner() = default; + virtual ~TaskRunner() = default; + + private: + TaskRunner(const TaskRunner&) = delete; + TaskRunner& operator=(const TaskRunner&) = delete; +}; + /** * The interface represents complex arguments to trace events. */ @@ -150,6 +195,28 @@ class Platform { */ virtual size_t NumberOfAvailableBackgroundThreads() { return 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::unique_ptr 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. + */ + virtual std::unique_ptr GetBackgroundTaskRunner( + Isolate* isolate) { + // TODO(ahaas): Make this function abstract after it got implemented on all + // platforms. + return {}; + } + /** * Schedules a task to be invoked on a background thread. |expected_runtime| * indicates that the task will run a long time. The Platform implementation From d2848bb9e0ad0dc78f65e6579023a029b86813a0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 19 Nov 2017 15:02:51 +0100 Subject: [PATCH 2/5] deps: cherry-pick 98c40a4bae915 from V8 upstream Original commit message: [platform] Return task runners as shared_ptr At the moment, task runners are returned as unique_ptr. This is inconvenient, however. In all implementations I did, the platform holds a shared pointer of the task runner and wraps it in a wrapper class just to return it as a unique_ptr. With this CL the platform API is changed to return a shared_ptr directly. R=rmcilroy@chromium.org Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: Ide278db855199ea239ad0ae14d97fd17349dac8c Reviewed-on: https://chromium-review.googlesource.com/768867 Commit-Queue: Andreas Haas Reviewed-by: Ross McIlroy Cr-Commit-Position: refs/heads/master@{#49366} Refs: https://github.com/v8/v8/commit/98c40a4bae915a9762c73de3307300c61e4fd91a --- deps/v8/include/v8-platform.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deps/v8/include/v8-platform.h b/deps/v8/include/v8-platform.h index b1a57c9e2c18a5..6c3c4292c5c3c8 100644 --- a/deps/v8/include/v8-platform.h +++ b/deps/v8/include/v8-platform.h @@ -199,7 +199,7 @@ class Platform { * 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::unique_ptr GetForegroundTaskRunner( + virtual std::shared_ptr GetForegroundTaskRunner( Isolate* isolate) { // TODO(ahaas): Make this function abstract after it got implemented on all // platforms. @@ -210,7 +210,7 @@ class Platform { * Returns a TaskRunner which can be used to post a task on a background. * This function should only be called from a foreground thread. */ - virtual std::unique_ptr GetBackgroundTaskRunner( + virtual std::shared_ptr GetBackgroundTaskRunner( Isolate* isolate) { // TODO(ahaas): Make this function abstract after it got implemented on all // platforms. From 8be600b5df5ea160f7b44a1fa93d9b4f17c85b80 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 19 Nov 2017 14:29:08 +0100 Subject: [PATCH 3/5] src: implement v8::TaskRunner API in NodePlatform V8 is switching APIs for scheduling tasks. Implement the new APIs. Fixes: https://github.com/nodejs/node-v8/issues/24 Refs: https://github.com/v8/v8/commit/c690f54d9580243c53f7d892fcff1ce6bae4bfc0 --- src/node_platform.cc | 118 ++++++++++++++++++++++++++++++------------- src/node_platform.h | 48 ++++++++++++++---- 2 files changed, 122 insertions(+), 44 deletions(-) diff --git a/src/node_platform.cc b/src/node_platform.cc index 0c50e8468d0f44..3e10d1489ec052 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -24,6 +24,46 @@ static void BackgroundRunner(void* data) { } } +BackgroundTaskRunner::BackgroundTaskRunner(int thread_pool_size) { + for (int i = 0; i < thread_pool_size; i++) { + uv_thread_t* t = new uv_thread_t(); + if (uv_thread_create(t, BackgroundRunner, &background_tasks_) != 0) { + delete t; + break; + } + threads_.push_back(std::unique_ptr(t)); + } +} + +void BackgroundTaskRunner::PostTask(std::unique_ptr task) { + background_tasks_.Push(std::move(task)); +} + +void BackgroundTaskRunner::PostIdleTask(std::unique_ptr task) { + CHECK(false && "idle tasks not enabled"); +} + +void BackgroundTaskRunner::PostDelayedTask(std::unique_ptr task, + double delay_in_seconds) { + // It's unclear whether this is required at all for background tasks. + PostTask(std::move(task)); +} + +void BackgroundTaskRunner::BlockingDrain() { + background_tasks_.BlockingDrain(); +} + +void BackgroundTaskRunner::Shutdown() { + background_tasks_.Stop(); + for (size_t i = 0; i < threads_.size(); i++) { + CHECK_EQ(0, uv_thread_join(threads_[i].get())); + } +} + +size_t BackgroundTaskRunner::NumberOfAvailableBackgroundThreads() const { + return threads_.size(); +} + PerIsolatePlatformData::PerIsolatePlatformData( v8::Isolate* isolate, uv_loop_t* loop) : isolate_(isolate), loop_(loop) { @@ -38,17 +78,20 @@ void PerIsolatePlatformData::FlushTasks(uv_async_t* handle) { platform_data->FlushForegroundTasksInternal(); } -void PerIsolatePlatformData::CallOnForegroundThread( - std::unique_ptr task) { +void PerIsolatePlatformData::PostIdleTask(std::unique_ptr task) { + CHECK(false && "idle tasks not enabled"); +} + +void PerIsolatePlatformData::PostTask(std::unique_ptr task) { foreground_tasks_.Push(std::move(task)); uv_async_send(flush_tasks_); } -void PerIsolatePlatformData::CallDelayedOnForegroundThread( - std::unique_ptr task, double delay_in_seconds) { +void PerIsolatePlatformData::PostDelayedTask( + std::unique_ptr task, double delay_in_seconds) { std::unique_ptr delayed(new DelayedTask()); delayed->task = std::move(task); - delayed->platform_data = this; + delayed->platform_data = shared_from_this(); delayed->timeout = delay_in_seconds; foreground_delayed_tasks_.Push(std::move(delayed)); uv_async_send(flush_tasks_); @@ -80,49 +123,43 @@ NodePlatform::NodePlatform(int thread_pool_size, TracingController* controller = new TracingController(); tracing_controller_.reset(controller); } - for (int i = 0; i < thread_pool_size; i++) { - uv_thread_t* t = new uv_thread_t(); - if (uv_thread_create(t, BackgroundRunner, &background_tasks_) != 0) { - delete t; - break; - } - threads_.push_back(std::unique_ptr(t)); - } + background_task_runner_ = + std::make_shared(thread_pool_size); } void NodePlatform::RegisterIsolate(IsolateData* isolate_data, uv_loop_t* loop) { Isolate* isolate = isolate_data->isolate(); Mutex::ScopedLock lock(per_isolate_mutex_); - PerIsolatePlatformData* existing = per_isolate_[isolate]; - if (existing != nullptr) + std::shared_ptr existing = per_isolate_[isolate]; + if (existing) { existing->ref(); - else - per_isolate_[isolate] = new PerIsolatePlatformData(isolate, loop); + } else { + per_isolate_[isolate] = + std::make_shared(isolate, loop); + } } void NodePlatform::UnregisterIsolate(IsolateData* isolate_data) { Isolate* isolate = isolate_data->isolate(); Mutex::ScopedLock lock(per_isolate_mutex_); - PerIsolatePlatformData* existing = per_isolate_[isolate]; - CHECK_NE(existing, nullptr); + std::shared_ptr existing = per_isolate_[isolate]; + CHECK(existing); if (existing->unref() == 0) { - delete existing; per_isolate_.erase(isolate); } } void NodePlatform::Shutdown() { - background_tasks_.Stop(); - for (size_t i = 0; i < threads_.size(); i++) { - CHECK_EQ(0, uv_thread_join(threads_[i].get())); + background_task_runner_->Shutdown(); + + { + Mutex::ScopedLock lock(per_isolate_mutex_); + per_isolate_.clear(); } - Mutex::ScopedLock lock(per_isolate_mutex_); - for (const auto& pair : per_isolate_) - delete pair.second; } size_t NodePlatform::NumberOfAvailableBackgroundThreads() { - return threads_.size(); + return background_task_runner_->NumberOfAvailableBackgroundThreads(); } void PerIsolatePlatformData::RunForegroundTask(std::unique_ptr task) { @@ -155,14 +192,14 @@ void PerIsolatePlatformData::CancelPendingDelayedTasks() { } void NodePlatform::DrainBackgroundTasks(Isolate* isolate) { - PerIsolatePlatformData* per_isolate = ForIsolate(isolate); + std::shared_ptr per_isolate = ForIsolate(isolate); do { // Right now, there is no way to drain only background tasks associated // with a specific isolate, so this sometimes does more work than // necessary. In the long run, that functionality is probably going to // be available anyway, though. - background_tasks_.BlockingDrain(); + background_task_runner_->BlockingDrain(); } while (per_isolate->FlushForegroundTasksInternal()); } @@ -198,24 +235,25 @@ bool PerIsolatePlatformData::FlushForegroundTasksInternal() { void NodePlatform::CallOnBackgroundThread(Task* task, ExpectedRuntime expected_runtime) { - background_tasks_.Push(std::unique_ptr(task)); + background_task_runner_->PostTask(std::unique_ptr(task)); } -PerIsolatePlatformData* NodePlatform::ForIsolate(Isolate* isolate) { +std::shared_ptr +NodePlatform::ForIsolate(Isolate* isolate) { Mutex::ScopedLock lock(per_isolate_mutex_); - PerIsolatePlatformData* data = per_isolate_[isolate]; - CHECK_NE(data, nullptr); + std::shared_ptr data = per_isolate_[isolate]; + CHECK(data); return data; } void NodePlatform::CallOnForegroundThread(Isolate* isolate, Task* task) { - ForIsolate(isolate)->CallOnForegroundThread(std::unique_ptr(task)); + ForIsolate(isolate)->PostTask(std::unique_ptr(task)); } void NodePlatform::CallDelayedOnForegroundThread(Isolate* isolate, Task* task, double delay_in_seconds) { - ForIsolate(isolate)->CallDelayedOnForegroundThread( + ForIsolate(isolate)->PostDelayedTask( std::unique_ptr(task), delay_in_seconds); } @@ -229,6 +267,16 @@ void NodePlatform::CancelPendingDelayedTasks(v8::Isolate* isolate) { bool NodePlatform::IdleTasksEnabled(Isolate* isolate) { return false; } +std::shared_ptr +NodePlatform::GetBackgroundTaskRunner(Isolate* isolate) { + return background_task_runner_; +} + +std::shared_ptr +NodePlatform::GetForegroundTaskRunner(Isolate* isolate) { + return ForIsolate(isolate); +} + double NodePlatform::MonotonicallyIncreasingTime() { // Convert nanos to seconds. return uv_hrtime() / 1e9; diff --git a/src/node_platform.h b/src/node_platform.h index 48301a05a11d8c..e5253cac10c3b0 100644 --- a/src/node_platform.h +++ b/src/node_platform.h @@ -43,17 +43,22 @@ struct DelayedTask { std::unique_ptr task; uv_timer_t timer; double timeout; - PerIsolatePlatformData* platform_data; + std::shared_ptr platform_data; }; -class PerIsolatePlatformData { +// This acts as the foreground task runner for a given Isolate. +class PerIsolatePlatformData : + public v8::TaskRunner, + public std::enable_shared_from_this { public: PerIsolatePlatformData(v8::Isolate* isolate, uv_loop_t* loop); ~PerIsolatePlatformData(); - void CallOnForegroundThread(std::unique_ptr task); - void CallDelayedOnForegroundThread(std::unique_ptr task, - double delay_in_seconds); + void PostTask(std::unique_ptr task) override; + void PostIdleTask(std::unique_ptr task) override; + void PostDelayedTask(std::unique_ptr task, + double delay_in_seconds) override; + bool IdleTasksEnabled() override { return false; }; void Shutdown(); @@ -84,6 +89,26 @@ class PerIsolatePlatformData { std::vector scheduled_delayed_tasks_; }; +// This acts as the single background task runner for all Isolates. +class BackgroundTaskRunner : public v8::TaskRunner { + public: + explicit BackgroundTaskRunner(int thread_pool_size); + + void PostTask(std::unique_ptr task) override; + void PostIdleTask(std::unique_ptr task) override; + void PostDelayedTask(std::unique_ptr task, + double delay_in_seconds) override; + bool IdleTasksEnabled() override { return false; }; + + void BlockingDrain(); + void Shutdown(); + + size_t NumberOfAvailableBackgroundThreads() const; + private: + TaskQueue background_tasks_; + std::vector> threads_; +}; + class NodePlatform : public MultiIsolatePlatform { public: NodePlatform(int thread_pool_size, v8::TracingController* tracing_controller); @@ -109,15 +134,20 @@ class NodePlatform : public MultiIsolatePlatform { void RegisterIsolate(IsolateData* isolate_data, uv_loop_t* loop) override; void UnregisterIsolate(IsolateData* isolate_data) override; + std::shared_ptr GetBackgroundTaskRunner( + v8::Isolate* isolate) override; + std::shared_ptr GetForegroundTaskRunner( + v8::Isolate* isolate) override; + private: - PerIsolatePlatformData* ForIsolate(v8::Isolate* isolate); + std::shared_ptr ForIsolate(v8::Isolate* isolate); Mutex per_isolate_mutex_; - std::unordered_map per_isolate_; - TaskQueue background_tasks_; - std::vector> threads_; + std::unordered_map> per_isolate_; std::unique_ptr tracing_controller_; + std::shared_ptr background_task_runner_; }; } // namespace node From 2cd135b4dd1df30688df8e40b7327763e7c1cec5 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 20 Nov 2017 13:03:24 +0100 Subject: [PATCH 4/5] [squash] gahaas nit --- src/node_platform.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/node_platform.cc b/src/node_platform.cc index 3e10d1489ec052..a111c6c9cc65e6 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -26,12 +26,10 @@ static void BackgroundRunner(void* data) { BackgroundTaskRunner::BackgroundTaskRunner(int thread_pool_size) { for (int i = 0; i < thread_pool_size; i++) { - uv_thread_t* t = new uv_thread_t(); - if (uv_thread_create(t, BackgroundRunner, &background_tasks_) != 0) { - delete t; + std::unique_ptr t { new uv_thread_t() }; + if (uv_thread_create(t.get(), BackgroundRunner, &background_tasks_) != 0) break; - } - threads_.push_back(std::unique_ptr(t)); + threads_.push_back(std::move(t)); } } From 9bd517ea6f47d63b44232dbd25ded60aa959896c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 21 Nov 2017 08:44:30 +0100 Subject: [PATCH 5/5] [squash] `UNREACHABLE();` all the things --- src/node_platform.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/node_platform.cc b/src/node_platform.cc index a111c6c9cc65e6..65d1ef6c5dfe28 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -38,13 +38,12 @@ void BackgroundTaskRunner::PostTask(std::unique_ptr task) { } void BackgroundTaskRunner::PostIdleTask(std::unique_ptr task) { - CHECK(false && "idle tasks not enabled"); + UNREACHABLE(); } void BackgroundTaskRunner::PostDelayedTask(std::unique_ptr task, double delay_in_seconds) { - // It's unclear whether this is required at all for background tasks. - PostTask(std::move(task)); + UNREACHABLE(); } void BackgroundTaskRunner::BlockingDrain() { @@ -77,7 +76,7 @@ void PerIsolatePlatformData::FlushTasks(uv_async_t* handle) { } void PerIsolatePlatformData::PostIdleTask(std::unique_ptr task) { - CHECK(false && "idle tasks not enabled"); + UNREACHABLE(); } void PerIsolatePlatformData::PostTask(std::unique_ptr task) {