From 140955eeccd98eeb622eaa26dd8896cd2b49002a Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Tue, 27 Jun 2017 10:49:06 +0200 Subject: [PATCH 1/8] deps: backport c4852ea from upstream V8 Original commit message: Pull tracing related methods out of Platform This will allow for embedders to easily implement their own Platform without duplicating the tracing controller code. BUG=v8:6511 R=fmeawad@chromium.org Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I7c64933d12b2cf53f0636fbc87f6ad5d22019f5c Reviewed-on: https://chromium-review.googlesource.com/543015 Commit-Queue: Jochen Eisinger Reviewed-by: Fadi Meawad Cr-Commit-Position: refs/heads/master@{#46118} --- deps/v8/include/libplatform/v8-tracing.h | 32 +++++-- deps/v8/include/v8-platform.h | 89 ++++++++++++++++--- deps/v8/src/libplatform/default-platform.cc | 4 + deps/v8/src/libplatform/default-platform.h | 1 + .../libplatform/tracing/tracing-controller.cc | 8 +- .../cctest/heap/test-incremental-marking.cc | 23 ----- 6 files changed, 107 insertions(+), 50 deletions(-) diff --git a/deps/v8/include/libplatform/v8-tracing.h b/deps/v8/include/libplatform/v8-tracing.h index 902f8ea93dbaa5..8c1febf7627ae7 100644 --- a/deps/v8/include/libplatform/v8-tracing.h +++ b/deps/v8/include/libplatform/v8-tracing.h @@ -209,7 +209,15 @@ class V8_PLATFORM_EXPORT TraceConfig { void operator=(const TraceConfig&) = delete; }; -class V8_PLATFORM_EXPORT TracingController { +#if defined(_MSC_VER) +#define V8_PLATFORM_NON_EXPORTED_BASE(code) \ + __pragma(warning(suppress : 4275)) code +#else +#define V8_PLATFORM_NON_EXPORTED_BASE(code) code +#endif // defined(_MSC_VER) + +class V8_PLATFORM_EXPORT TracingController + : public V8_PLATFORM_NON_EXPORTED_BASE(v8::TracingController) { public: enum Mode { DISABLED = 0, RECORDING_MODE }; @@ -227,25 +235,29 @@ class V8_PLATFORM_EXPORT TracingController { }; TracingController(); - ~TracingController(); + ~TracingController() override; void Initialize(TraceBuffer* trace_buffer); - const uint8_t* GetCategoryGroupEnabled(const char* category_group); - static const char* GetCategoryGroupName(const uint8_t* category_enabled_flag); + + // v8::TracingController implementation. + const uint8_t* GetCategoryGroupEnabled(const char* category_group) override; uint64_t AddTraceEvent( char phase, const uint8_t* category_enabled_flag, const char* name, const char* scope, uint64_t id, uint64_t bind_id, int32_t num_args, const char** arg_names, const uint8_t* arg_types, const uint64_t* arg_values, std::unique_ptr* arg_convertables, - unsigned int flags); + unsigned int flags) override; void UpdateTraceEventDuration(const uint8_t* category_enabled_flag, - const char* name, uint64_t handle); + const char* name, uint64_t handle) override; + void AddTraceStateObserver( + v8::TracingController::TraceStateObserver* observer) override; + void RemoveTraceStateObserver( + v8::TracingController::TraceStateObserver* observer) override; void StartTracing(TraceConfig* trace_config); void StopTracing(); - void AddTraceStateObserver(Platform::TraceStateObserver* observer); - void RemoveTraceStateObserver(Platform::TraceStateObserver* observer); + static const char* GetCategoryGroupName(const uint8_t* category_enabled_flag); private: const uint8_t* GetCategoryGroupEnabledInternal(const char* category_group); @@ -255,7 +267,7 @@ class V8_PLATFORM_EXPORT TracingController { std::unique_ptr trace_buffer_; std::unique_ptr trace_config_; std::unique_ptr mutex_; - std::unordered_set observers_; + std::unordered_set observers_; Mode mode_ = DISABLED; // Disallow copy and assign @@ -263,6 +275,8 @@ class V8_PLATFORM_EXPORT TracingController { void operator=(const TracingController&) = delete; }; +#undef V8_PLATFORM_NON_EXPORTED_BASE + } // namespace tracing } // namespace platform } // namespace v8 diff --git a/deps/v8/include/v8-platform.h b/deps/v8/include/v8-platform.h index 8f6d8042abd86a..0ab153492feaa1 100644 --- a/deps/v8/include/v8-platform.h +++ b/deps/v8/include/v8-platform.h @@ -52,6 +52,66 @@ class ConvertableToTraceFormat { virtual void AppendAsTraceFormat(std::string* out) const = 0; }; +/** + * V8 Tracing controller. + * + * Can be implemented by an embedder to record trace events from V8. + */ +class TracingController { + public: + virtual ~TracingController() = default; + + /** + * Called by TRACE_EVENT* macros, don't call this directly. + * The name parameter is a category group for example: + * TRACE_EVENT0("v8,parse", "V8.Parse") + * The pointer returned points to a value with zero or more of the bits + * defined in CategoryGroupEnabledFlags. + **/ + virtual const uint8_t* GetCategoryGroupEnabled(const char* name) { + static uint8_t no = 0; + return &no; + } + + /** + * Adds a trace event to the platform tracing system. This function call is + * usually the result of a TRACE_* macro from trace_event_common.h when + * tracing and the category of the particular trace are enabled. It is not + * advisable to call this function on its own; it is really only meant to be + * used by the trace macros. The returned handle can be used by + * UpdateTraceEventDuration to update the duration of COMPLETE events. + */ + virtual uint64_t AddTraceEvent( + char phase, const uint8_t* category_enabled_flag, const char* name, + const char* scope, uint64_t id, uint64_t bind_id, int32_t num_args, + const char** arg_names, const uint8_t* arg_types, + const uint64_t* arg_values, + std::unique_ptr* arg_convertables, + unsigned int flags) { + return 0; + } + + /** + * Sets the duration field of a COMPLETE trace event. It must be called with + * the handle returned from AddTraceEvent(). + **/ + virtual void UpdateTraceEventDuration(const uint8_t* category_enabled_flag, + const char* name, uint64_t handle) {} + + class TraceStateObserver { + public: + virtual ~TraceStateObserver() = default; + virtual void OnTraceEnabled() = 0; + virtual void OnTraceDisabled() = 0; + }; + + /** Adds tracing state change observer. */ + virtual void AddTraceStateObserver(TraceStateObserver*) {} + + /** Removes tracing state change observer. */ + virtual void RemoveTraceStateObserver(TraceStateObserver*) {} +}; + /** * V8 Platform abstraction layer. * @@ -135,6 +195,20 @@ class Platform { * the epoch. **/ virtual double MonotonicallyIncreasingTime() = 0; + typedef void (*StackTracePrinter)(); + + /** + * Returns a function pointer that print a stack trace of the current stack + * on invocation. Disables printing of the stack trace if nullptr. + */ + virtual StackTracePrinter GetStackTracePrinter() { return nullptr; } + + /** + * Returns an instance of a v8::TracingController. This must be non-nullptr. + */ + virtual TracingController* GetTracingController() { return nullptr; } + + // DEPRECATED methods, use TracingController interface instead. /** * Called by TRACE_EVENT* macros, don't call this directly. @@ -200,26 +274,13 @@ class Platform { virtual void UpdateTraceEventDuration(const uint8_t* category_enabled_flag, const char* name, uint64_t handle) {} - class TraceStateObserver { - public: - virtual ~TraceStateObserver() = default; - virtual void OnTraceEnabled() = 0; - virtual void OnTraceDisabled() = 0; - }; + typedef v8::TracingController::TraceStateObserver TraceStateObserver; /** Adds tracing state change observer. */ virtual void AddTraceStateObserver(TraceStateObserver*) {} /** Removes tracing state change observer. */ virtual void RemoveTraceStateObserver(TraceStateObserver*) {} - - typedef void (*StackTracePrinter)(); - - /** - * Returns a function pointer that print a stack trace of the current stack - * on invocation. Disables printing of the stack trace if nullptr. - */ - virtual StackTracePrinter GetStackTracePrinter() { return nullptr; } }; } // namespace v8 diff --git a/deps/v8/src/libplatform/default-platform.cc b/deps/v8/src/libplatform/default-platform.cc index 66290726833b67..fce946ad6f3456 100644 --- a/deps/v8/src/libplatform/default-platform.cc +++ b/deps/v8/src/libplatform/default-platform.cc @@ -272,6 +272,10 @@ double DefaultPlatform::MonotonicallyIncreasingTime() { static_cast(base::Time::kMicrosecondsPerSecond); } +TracingController* DefaultPlatform::GetTracingController() { + return tracing_controller_.get(); +} + uint64_t DefaultPlatform::AddTraceEvent( char phase, const uint8_t* category_enabled_flag, const char* name, const char* scope, uint64_t id, uint64_t bind_id, int num_args, diff --git a/deps/v8/src/libplatform/default-platform.h b/deps/v8/src/libplatform/default-platform.h index 40268647493673..3d91433132e638 100644 --- a/deps/v8/src/libplatform/default-platform.h +++ b/deps/v8/src/libplatform/default-platform.h @@ -58,6 +58,7 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) { void CallIdleOnForegroundThread(Isolate* isolate, IdleTask* task) override; bool IdleTasksEnabled(Isolate* isolate) override; double MonotonicallyIncreasingTime() override; + TracingController* GetTracingController() override; const uint8_t* GetCategoryGroupEnabled(const char* name) override; const char* GetCategoryGroupName( const uint8_t* category_enabled_flag) override; diff --git a/deps/v8/src/libplatform/tracing/tracing-controller.cc b/deps/v8/src/libplatform/tracing/tracing-controller.cc index c1a4057c0555b4..4fff4be65d8dde 100644 --- a/deps/v8/src/libplatform/tracing/tracing-controller.cc +++ b/deps/v8/src/libplatform/tracing/tracing-controller.cc @@ -98,7 +98,7 @@ const char* TracingController::GetCategoryGroupName( void TracingController::StartTracing(TraceConfig* trace_config) { trace_config_.reset(trace_config); - std::unordered_set observers_copy; + std::unordered_set observers_copy; { base::LockGuard lock(mutex_.get()); mode_ = RECORDING_MODE; @@ -113,7 +113,7 @@ void TracingController::StartTracing(TraceConfig* trace_config) { void TracingController::StopTracing() { mode_ = DISABLED; UpdateCategoryGroupEnabledFlags(); - std::unordered_set observers_copy; + std::unordered_set observers_copy; { base::LockGuard lock(mutex_.get()); observers_copy = observers_; @@ -196,7 +196,7 @@ const uint8_t* TracingController::GetCategoryGroupEnabledInternal( } void TracingController::AddTraceStateObserver( - Platform::TraceStateObserver* observer) { + v8::TracingController::TraceStateObserver* observer) { { base::LockGuard lock(mutex_.get()); observers_.insert(observer); @@ -207,7 +207,7 @@ void TracingController::AddTraceStateObserver( } void TracingController::RemoveTraceStateObserver( - Platform::TraceStateObserver* observer) { + v8::TracingController::TraceStateObserver* observer) { base::LockGuard lock(mutex_.get()); DCHECK(observers_.find(observer) != observers_.end()); observers_.erase(observer); diff --git a/deps/v8/test/cctest/heap/test-incremental-marking.cc b/deps/v8/test/cctest/heap/test-incremental-marking.cc index ce1fb349517f48..84415389cf22b2 100644 --- a/deps/v8/test/cctest/heap/test-incremental-marking.cc +++ b/deps/v8/test/cctest/heap/test-incremental-marking.cc @@ -71,29 +71,6 @@ class MockPlatform : public v8::Platform { delete task; } - 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; - } - private: v8::Platform* platform_; Task* task_; From 9c2a46f7fef356ffcf5d1a12c021e11ad789e828 Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Tue, 27 Jun 2017 11:05:22 +0200 Subject: [PATCH 2/8] deps: backport 5152d97 from upstream V8 Original commit message: Add API to create a platform with a tracing controller BUG=v8:6511 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: Ie6b62df693d3b847837c071e1f985b7ce3b420c8 Reviewed-on: https://chromium-review.googlesource.com/548499 Reviewed-by: Fadi Meawad Commit-Queue: Jochen Eisinger Cr-Commit-Position: refs/heads/master@{#46227} --- deps/v8/include/libplatform/libplatform.h | 5 ++++- deps/v8/src/libplatform/default-platform.cc | 16 +++++++--------- deps/v8/src/libplatform/default-platform.h | 8 ++------ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/deps/v8/include/libplatform/libplatform.h b/deps/v8/include/libplatform/libplatform.h index e9450456299698..39158e7bc84591 100644 --- a/deps/v8/include/libplatform/libplatform.h +++ b/deps/v8/include/libplatform/libplatform.h @@ -30,12 +30,15 @@ enum class MessageLoopBehavior : bool { * If |idle_task_support| is enabled then the platform will accept idle * tasks (IdleTasksEnabled will return true) and will rely on the embedder * calling v8::platform::RunIdleTasks to process the idle tasks. + * If |tracing_controller| is nullptr, the default platform will create a + * v8::platform::TracingController instance and use it. */ V8_PLATFORM_EXPORT v8::Platform* CreateDefaultPlatform( int thread_pool_size = 0, IdleTaskSupport idle_task_support = IdleTaskSupport::kDisabled, InProcessStackDumping in_process_stack_dumping = - InProcessStackDumping::kEnabled); + InProcessStackDumping::kEnabled, + v8::TracingController* tracing_controller = nullptr); /** * Pumps the message loop for the given isolate. diff --git a/deps/v8/src/libplatform/default-platform.cc b/deps/v8/src/libplatform/default-platform.cc index fce946ad6f3456..86fdabce4fbf6b 100644 --- a/deps/v8/src/libplatform/default-platform.cc +++ b/deps/v8/src/libplatform/default-platform.cc @@ -29,15 +29,18 @@ void PrintStackTrace() { } // namespace -v8::Platform* CreateDefaultPlatform( - int thread_pool_size, IdleTaskSupport idle_task_support, - InProcessStackDumping in_process_stack_dumping) { +v8::Platform* CreateDefaultPlatform(int thread_pool_size, + IdleTaskSupport idle_task_support, + InProcessStackDumping in_process_stack_dumping, + v8::TracingController* tracing_controller) { if (in_process_stack_dumping == InProcessStackDumping::kEnabled) { v8::base::debug::EnableInProcessStackDumping(); } DefaultPlatform* platform = new DefaultPlatform(idle_task_support); platform->SetThreadPoolSize(thread_pool_size); platform->EnsureInitialized(); + if (tracing_controller != nullptr) + platform->SetTracingController(tracing_controller); return platform; } @@ -73,11 +76,6 @@ DefaultPlatform::DefaultPlatform(IdleTaskSupport idle_task_support) idle_task_support_(idle_task_support) {} DefaultPlatform::~DefaultPlatform() { - if (tracing_controller_) { - tracing_controller_->StopTracing(); - tracing_controller_.reset(); - } - base::LockGuard guard(&lock_); queue_.Terminate(); if (initialized_) { @@ -316,7 +314,7 @@ const char* DefaultPlatform::GetCategoryGroupName( } void DefaultPlatform::SetTracingController( - tracing::TracingController* tracing_controller) { + TracingController* tracing_controller) { tracing_controller_.reset(tracing_controller); } diff --git a/deps/v8/src/libplatform/default-platform.h b/deps/v8/src/libplatform/default-platform.h index 3d91433132e638..80ea4e85d4d601 100644 --- a/deps/v8/src/libplatform/default-platform.h +++ b/deps/v8/src/libplatform/default-platform.h @@ -27,10 +27,6 @@ class TaskQueue; class Thread; class WorkerThread; -namespace tracing { -class TracingController; -} - class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) { public: explicit DefaultPlatform( @@ -72,7 +68,7 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) { unsigned int flags) override; void UpdateTraceEventDuration(const uint8_t* category_enabled_flag, const char* name, uint64_t handle) override; - void SetTracingController(tracing::TracingController* tracing_controller); + void SetTracingController(TracingController* tracing_controller); void AddTraceStateObserver(TraceStateObserver* observer) override; void RemoveTraceStateObserver(TraceStateObserver* observer) override; @@ -103,7 +99,7 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) { std::priority_queue, std::greater > > main_thread_delayed_queue_; - std::unique_ptr tracing_controller_; + std::unique_ptr tracing_controller_; DISALLOW_COPY_AND_ASSIGN(DefaultPlatform); }; From d4f4bdc8d31c83ddd6652cde257348544ba7a019 Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Thu, 29 Jun 2017 10:45:40 +0200 Subject: [PATCH 3/8] deps: backport 3d8e87a from upstream V8 Original commit message: Switch tracing to use v8::TracingController BUG=v8:6511 R=fmeawad@chromium.org Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I4961e4b61a9ddc98385ed97c3ffcbcaef2d9cba7 Reviewed-on: https://chromium-review.googlesource.com/543144 Commit-Queue: Jochen Eisinger Reviewed-by: Fadi Meawad Cr-Commit-Position: refs/heads/master@{#46307} --- deps/v8/include/libplatform/libplatform.h | 2 + deps/v8/include/v8-platform.h | 2 +- deps/v8/src/d8.cc | 32 ++++---- deps/v8/src/libplatform/default-platform.cc | 75 +++++------------- deps/v8/src/libplatform/default-platform.h | 24 ++---- .../v8/src/libplatform/tracing/trace-writer.h | 8 ++ .../libplatform/tracing/tracing-controller.cc | 6 +- deps/v8/src/profiler/tracing-cpu-profiler.cc | 5 +- deps/v8/src/profiler/tracing-cpu-profiler.h | 7 +- deps/v8/src/tracing/trace-event.cc | 4 +- deps/v8/src/tracing/trace-event.h | 20 ++--- .../src/tracing/tracing-category-observer.cc | 10 ++- .../src/tracing/tracing-category-observer.h | 4 +- .../cctest/heap/test-incremental-marking.cc | 4 + .../test/cctest/libplatform/test-tracing.cc | 23 +++--- deps/v8/test/cctest/test-cpu-profiler.cc | 4 +- deps/v8/test/cctest/test-trace-event.cc | 76 +++++++++++-------- .../compiler-dispatcher-unittest.cc | 63 ++++++++------- 18 files changed, 188 insertions(+), 181 deletions(-) diff --git a/deps/v8/include/libplatform/libplatform.h b/deps/v8/include/libplatform/libplatform.h index 39158e7bc84591..b615088300e444 100644 --- a/deps/v8/include/libplatform/libplatform.h +++ b/deps/v8/include/libplatform/libplatform.h @@ -70,6 +70,8 @@ V8_PLATFORM_EXPORT void RunIdleTasks(v8::Platform* platform, * Attempts to set the tracing controller for the given platform. * * The |platform| has to be created using |CreateDefaultPlatform|. + * + * DEPRECATED: Will be removed soon. */ V8_PLATFORM_EXPORT void SetTracingController( v8::Platform* platform, diff --git a/deps/v8/include/v8-platform.h b/deps/v8/include/v8-platform.h index 0ab153492feaa1..3df78a81c0a837 100644 --- a/deps/v8/include/v8-platform.h +++ b/deps/v8/include/v8-platform.h @@ -206,7 +206,7 @@ class Platform { /** * Returns an instance of a v8::TracingController. This must be non-nullptr. */ - virtual TracingController* GetTracingController() { return nullptr; } + virtual TracingController* GetTracingController() = 0; // DEPRECATED methods, use TracingController interface instead. diff --git a/deps/v8/src/d8.cc b/deps/v8/src/d8.cc index efa8dbc6331bc5..d92f8e0b05b06f 100644 --- a/deps/v8/src/d8.cc +++ b/deps/v8/src/d8.cc @@ -225,6 +225,10 @@ class PredictablePlatform : public Platform { return synthetic_time_in_sec_ += 0.00001; } + v8::TracingController* GetTracingController() override { + return platform_->GetTracingController(); + } + using Platform::AddTraceEvent; uint64_t AddTraceEvent(char phase, const uint8_t* categoryEnabledFlag, const char* name, const char* scope, uint64_t id, @@ -3063,18 +3067,7 @@ int Shell::Main(int argc, char* argv[]) { if (!SetOptions(argc, argv)) return 1; v8::V8::InitializeICUDefaultLocation(argv[0], options.icu_data_file); - v8::platform::InProcessStackDumping in_process_stack_dumping = - options.disable_in_process_stack_traces - ? 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); - - platform::tracing::TracingController* tracing_controller; + platform::tracing::TracingController* tracing_controller = nullptr; if (options.trace_enabled) { trace_file.open("v8_trace.json"); tracing_controller = new platform::tracing::TracingController(); @@ -3083,11 +3076,20 @@ int Shell::Main(int argc, char* argv[]) { 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); - } } + v8::platform::InProcessStackDumping in_process_stack_dumping = + options.disable_in_process_stack_traces + ? 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, + tracing_controller); + v8::V8::InitializePlatform(g_platform); v8::V8::Initialize(); if (options.natives_blob || options.snapshot_blob) { diff --git a/deps/v8/src/libplatform/default-platform.cc b/deps/v8/src/libplatform/default-platform.cc index 86fdabce4fbf6b..f873a7bb62ee6e 100644 --- a/deps/v8/src/libplatform/default-platform.cc +++ b/deps/v8/src/libplatform/default-platform.cc @@ -13,6 +13,8 @@ #include "src/base/platform/platform.h" #include "src/base/platform/time.h" #include "src/base/sys-info.h" +#include "src/libplatform/tracing/trace-buffer.h" +#include "src/libplatform/tracing/trace-writer.h" #include "src/libplatform/worker-thread.h" namespace v8 { @@ -36,11 +38,10 @@ v8::Platform* CreateDefaultPlatform(int thread_pool_size, if (in_process_stack_dumping == InProcessStackDumping::kEnabled) { v8::base::debug::EnableInProcessStackDumping(); } - DefaultPlatform* platform = new DefaultPlatform(idle_task_support); + DefaultPlatform* platform = + new DefaultPlatform(idle_task_support, tracing_controller); platform->SetThreadPoolSize(thread_pool_size); platform->EnsureInitialized(); - if (tracing_controller != nullptr) - platform->SetTracingController(tracing_controller); return platform; } @@ -70,10 +71,22 @@ void SetTracingController( const int DefaultPlatform::kMaxThreadPoolSize = 8; -DefaultPlatform::DefaultPlatform(IdleTaskSupport idle_task_support) +DefaultPlatform::DefaultPlatform(IdleTaskSupport idle_task_support, + v8::TracingController* tracing_controller) : initialized_(false), thread_pool_size_(0), - idle_task_support_(idle_task_support) {} + idle_task_support_(idle_task_support) { + if (tracing_controller) { + tracing_controller_.reset(tracing_controller); + } else { + tracing::TraceWriter* writer = new tracing::NullTraceWriter(); + tracing::TraceBuffer* ring_buffer = + new tracing::TraceBufferRingBuffer(1, writer); + tracing::TracingController* controller = new tracing::TracingController(); + controller->Initialize(ring_buffer); + tracing_controller_.reset(controller); + } +} DefaultPlatform::~DefaultPlatform() { base::LockGuard guard(&lock_); @@ -274,47 +287,9 @@ TracingController* DefaultPlatform::GetTracingController() { return tracing_controller_.get(); } -uint64_t DefaultPlatform::AddTraceEvent( - char phase, const uint8_t* category_enabled_flag, const char* name, - const char* scope, uint64_t id, uint64_t bind_id, int num_args, - const char** arg_names, const uint8_t* arg_types, - const uint64_t* arg_values, - std::unique_ptr* arg_convertables, - unsigned int flags) { - if (tracing_controller_) { - return tracing_controller_->AddTraceEvent( - phase, category_enabled_flag, name, scope, id, bind_id, num_args, - arg_names, arg_types, arg_values, arg_convertables, flags); - } - - return 0; -} - -void DefaultPlatform::UpdateTraceEventDuration( - const uint8_t* category_enabled_flag, const char* name, uint64_t handle) { - if (tracing_controller_) { - tracing_controller_->UpdateTraceEventDuration(category_enabled_flag, name, - handle); - } -} - -const uint8_t* DefaultPlatform::GetCategoryGroupEnabled(const char* name) { - if (tracing_controller_) { - return tracing_controller_->GetCategoryGroupEnabled(name); - } - static uint8_t no = 0; - return &no; -} - - -const char* DefaultPlatform::GetCategoryGroupName( - const uint8_t* category_enabled_flag) { - static const char dummy[] = "dummy"; - return dummy; -} - void DefaultPlatform::SetTracingController( - TracingController* tracing_controller) { + v8::TracingController* tracing_controller) { + DCHECK_NOT_NULL(tracing_controller); tracing_controller_.reset(tracing_controller); } @@ -322,16 +297,6 @@ size_t DefaultPlatform::NumberOfAvailableBackgroundThreads() { return static_cast(thread_pool_size_); } -void DefaultPlatform::AddTraceStateObserver(TraceStateObserver* observer) { - if (!tracing_controller_) return; - tracing_controller_->AddTraceStateObserver(observer); -} - -void DefaultPlatform::RemoveTraceStateObserver(TraceStateObserver* observer) { - if (!tracing_controller_) return; - tracing_controller_->RemoveTraceStateObserver(observer); -} - Platform::StackTracePrinter DefaultPlatform::GetStackTracePrinter() { return PrintStackTrace; } diff --git a/deps/v8/src/libplatform/default-platform.h b/deps/v8/src/libplatform/default-platform.h index 80ea4e85d4d601..a5fa7342181235 100644 --- a/deps/v8/src/libplatform/default-platform.h +++ b/deps/v8/src/libplatform/default-platform.h @@ -30,7 +30,8 @@ class WorkerThread; class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) { public: explicit DefaultPlatform( - IdleTaskSupport idle_task_support = IdleTaskSupport::kDisabled); + IdleTaskSupport idle_task_support = IdleTaskSupport::kDisabled, + v8::TracingController* tracing_controller = nullptr); virtual ~DefaultPlatform(); void SetThreadPoolSize(int thread_pool_size); @@ -44,6 +45,8 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) { void RunIdleTasks(v8::Isolate* isolate, double idle_time_in_seconds); + void SetTracingController(v8::TracingController* tracing_controller); + // v8::Platform implementation. size_t NumberOfAvailableBackgroundThreads() override; void CallOnBackgroundThread(Task* task, @@ -54,24 +57,7 @@ class V8_PLATFORM_EXPORT DefaultPlatform : public NON_EXPORTED_BASE(Platform) { void CallIdleOnForegroundThread(Isolate* isolate, IdleTask* task) override; bool IdleTasksEnabled(Isolate* isolate) override; double MonotonicallyIncreasingTime() override; - TracingController* GetTracingController() override; - const uint8_t* GetCategoryGroupEnabled(const char* name) override; - const char* GetCategoryGroupName( - const uint8_t* category_enabled_flag) override; - using Platform::AddTraceEvent; - uint64_t AddTraceEvent( - char phase, const uint8_t* category_enabled_flag, const char* name, - const char* scope, uint64_t id, uint64_t bind_id, int32_t num_args, - const char** arg_names, const uint8_t* arg_types, - const uint64_t* arg_values, - std::unique_ptr* arg_convertables, - unsigned int flags) override; - void UpdateTraceEventDuration(const uint8_t* category_enabled_flag, - const char* name, uint64_t handle) override; - void SetTracingController(TracingController* tracing_controller); - - void AddTraceStateObserver(TraceStateObserver* observer) override; - void RemoveTraceStateObserver(TraceStateObserver* observer) override; + v8::TracingController* GetTracingController() override; StackTracePrinter GetStackTracePrinter() override; private: diff --git a/deps/v8/src/libplatform/tracing/trace-writer.h b/deps/v8/src/libplatform/tracing/trace-writer.h index 43d7cb6a9063f8..67559f91fe559b 100644 --- a/deps/v8/src/libplatform/tracing/trace-writer.h +++ b/deps/v8/src/libplatform/tracing/trace-writer.h @@ -26,6 +26,14 @@ class JSONTraceWriter : public TraceWriter { bool append_comma_ = false; }; +class NullTraceWriter : public TraceWriter { + public: + NullTraceWriter() = default; + ~NullTraceWriter() = default; + void AppendTraceEvent(TraceObject*) override {} + void Flush() override {} +}; + } // namespace tracing } // namespace platform } // namespace v8 diff --git a/deps/v8/src/libplatform/tracing/tracing-controller.cc b/deps/v8/src/libplatform/tracing/tracing-controller.cc index 4fff4be65d8dde..4e71f432e84046 100644 --- a/deps/v8/src/libplatform/tracing/tracing-controller.cc +++ b/deps/v8/src/libplatform/tracing/tracing-controller.cc @@ -40,7 +40,7 @@ v8::base::AtomicWord g_category_index = g_num_builtin_categories; TracingController::TracingController() {} -TracingController::~TracingController() {} +TracingController::~TracingController() { StopTracing(); } void TracingController::Initialize(TraceBuffer* trace_buffer) { trace_buffer_.reset(trace_buffer); @@ -111,6 +111,10 @@ void TracingController::StartTracing(TraceConfig* trace_config) { } void TracingController::StopTracing() { + if (mode_ == DISABLED) { + return; + } + DCHECK(trace_buffer_); mode_ = DISABLED; UpdateCategoryGroupEnabledFlags(); std::unordered_set observers_copy; diff --git a/deps/v8/src/profiler/tracing-cpu-profiler.cc b/deps/v8/src/profiler/tracing-cpu-profiler.cc index a9b84b66345c2e..601adf60ac32ea 100644 --- a/deps/v8/src/profiler/tracing-cpu-profiler.cc +++ b/deps/v8/src/profiler/tracing-cpu-profiler.cc @@ -25,12 +25,13 @@ TracingCpuProfilerImpl::TracingCpuProfilerImpl(Isolate* isolate) TRACE_EVENT_WARMUP_CATEGORY(TRACE_DISABLED_BY_DEFAULT("v8.cpu_profiler")); TRACE_EVENT_WARMUP_CATEGORY( TRACE_DISABLED_BY_DEFAULT("v8.cpu_profiler.hires")); - V8::GetCurrentPlatform()->AddTraceStateObserver(this); + V8::GetCurrentPlatform()->GetTracingController()->AddTraceStateObserver(this); } TracingCpuProfilerImpl::~TracingCpuProfilerImpl() { StopProfiling(); - V8::GetCurrentPlatform()->RemoveTraceStateObserver(this); + V8::GetCurrentPlatform()->GetTracingController()->RemoveTraceStateObserver( + this); } void TracingCpuProfilerImpl::OnTraceEnabled() { diff --git a/deps/v8/src/profiler/tracing-cpu-profiler.h b/deps/v8/src/profiler/tracing-cpu-profiler.h index a512a940f8578b..e654f2be9d645a 100644 --- a/deps/v8/src/profiler/tracing-cpu-profiler.h +++ b/deps/v8/src/profiler/tracing-cpu-profiler.h @@ -17,13 +17,14 @@ namespace internal { class CpuProfiler; class Isolate; -class TracingCpuProfilerImpl final : public TracingCpuProfiler, - private v8::Platform::TraceStateObserver { +class TracingCpuProfilerImpl final + : public TracingCpuProfiler, + private v8::TracingController::TraceStateObserver { public: explicit TracingCpuProfilerImpl(Isolate*); ~TracingCpuProfilerImpl(); - // v8::Platform::TraceStateObserver + // v8::TracingController::TraceStateObserver void OnTraceEnabled() final; void OnTraceDisabled() final; diff --git a/deps/v8/src/tracing/trace-event.cc b/deps/v8/src/tracing/trace-event.cc index 97da1de056ce1c..41c59269e82a45 100644 --- a/deps/v8/src/tracing/trace-event.cc +++ b/deps/v8/src/tracing/trace-event.cc @@ -15,8 +15,8 @@ namespace v8 { namespace internal { namespace tracing { -v8::Platform* TraceEventHelper::GetCurrentPlatform() { - return v8::internal::V8::GetCurrentPlatform(); +v8::TracingController* TraceEventHelper::GetTracingController() { + return v8::internal::V8::GetCurrentPlatform()->GetTracingController(); } void CallStatsScopedTracer::AddEndTraceEvent() { diff --git a/deps/v8/src/tracing/trace-event.h b/deps/v8/src/tracing/trace-event.h index 8fbd56f6b5de23..6550e3e6fa9c64 100644 --- a/deps/v8/src/tracing/trace-event.h +++ b/deps/v8/src/tracing/trace-event.h @@ -72,8 +72,8 @@ enum CategoryGroupEnabledFlags { // for best performance when tracing is disabled. // const uint8_t* // TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(const char* category_group) -#define TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED \ - v8::internal::tracing::TraceEventHelper::GetCurrentPlatform() \ +#define TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED \ + v8::internal::tracing::TraceEventHelper::GetTracingController() \ ->GetCategoryGroupEnabled // Get the number of times traces have been recorded. This is used to implement @@ -101,8 +101,8 @@ enum CategoryGroupEnabledFlags { // const uint8_t* category_group_enabled, // const char* name, // uint64_t id) -#define TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION \ - v8::internal::tracing::TraceEventHelper::GetCurrentPlatform() \ +#define TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION \ + v8::internal::tracing::TraceEventHelper::GetTracingController() \ ->UpdateTraceEventDuration // Defines atomic operations used internally by the tracing system. @@ -277,7 +277,7 @@ const uint64_t kNoId = 0; class TraceEventHelper { public: - static v8::Platform* GetCurrentPlatform(); + static v8::TracingController* GetTracingController(); }; // TraceID encapsulates an ID that can either be an integer or pointer. Pointers @@ -424,11 +424,11 @@ static V8_INLINE uint64_t AddTraceEventImpl( static_cast(arg_values[1]))); } DCHECK(num_args <= 2); - v8::Platform* platform = - v8::internal::tracing::TraceEventHelper::GetCurrentPlatform(); - return platform->AddTraceEvent(phase, category_group_enabled, name, scope, id, - bind_id, num_args, arg_names, arg_types, - arg_values, arg_convertables, flags); + v8::TracingController* controller = + v8::internal::tracing::TraceEventHelper::GetTracingController(); + return controller->AddTraceEvent(phase, category_group_enabled, name, scope, + id, bind_id, num_args, arg_names, arg_types, + arg_values, arg_convertables, flags); } // Define SetTraceValue for each allowed type. It stores the type and diff --git a/deps/v8/src/tracing/tracing-category-observer.cc b/deps/v8/src/tracing/tracing-category-observer.cc index 6a3615874184f6..3e286620dc555b 100644 --- a/deps/v8/src/tracing/tracing-category-observer.cc +++ b/deps/v8/src/tracing/tracing-category-observer.cc @@ -15,8 +15,9 @@ TracingCategoryObserver* TracingCategoryObserver::instance_ = nullptr; void TracingCategoryObserver::SetUp() { TracingCategoryObserver::instance_ = new TracingCategoryObserver(); - v8::internal::V8::GetCurrentPlatform()->AddTraceStateObserver( - TracingCategoryObserver::instance_); + v8::internal::V8::GetCurrentPlatform() + ->GetTracingController() + ->AddTraceStateObserver(TracingCategoryObserver::instance_); TRACE_EVENT_WARMUP_CATEGORY(TRACE_DISABLED_BY_DEFAULT("v8.runtime_stats")); TRACE_EVENT_WARMUP_CATEGORY( TRACE_DISABLED_BY_DEFAULT("v8.runtime_stats_sampling")); @@ -25,8 +26,9 @@ void TracingCategoryObserver::SetUp() { } void TracingCategoryObserver::TearDown() { - v8::internal::V8::GetCurrentPlatform()->RemoveTraceStateObserver( - TracingCategoryObserver::instance_); + v8::internal::V8::GetCurrentPlatform() + ->GetTracingController() + ->RemoveTraceStateObserver(TracingCategoryObserver::instance_); delete TracingCategoryObserver::instance_; } diff --git a/deps/v8/src/tracing/tracing-category-observer.h b/deps/v8/src/tracing/tracing-category-observer.h index 66dd2d78f11419..858bf0bdf81d53 100644 --- a/deps/v8/src/tracing/tracing-category-observer.h +++ b/deps/v8/src/tracing/tracing-category-observer.h @@ -10,7 +10,7 @@ namespace v8 { namespace tracing { -class TracingCategoryObserver : public Platform::TraceStateObserver { +class TracingCategoryObserver : public TracingController::TraceStateObserver { public: enum Mode { ENABLED_BY_NATIVE = 1 << 0, @@ -21,7 +21,7 @@ class TracingCategoryObserver : public Platform::TraceStateObserver { static void SetUp(); static void TearDown(); - // v8::Platform::TraceStateObserver + // v8::TracingController::TraceStateObserver void OnTraceEnabled() final; void OnTraceDisabled() final; diff --git a/deps/v8/test/cctest/heap/test-incremental-marking.cc b/deps/v8/test/cctest/heap/test-incremental-marking.cc index 84415389cf22b2..6e4aa04d255c4a 100644 --- a/deps/v8/test/cctest/heap/test-incremental-marking.cc +++ b/deps/v8/test/cctest/heap/test-incremental-marking.cc @@ -62,6 +62,10 @@ class MockPlatform : public v8::Platform { bool IdleTasksEnabled(v8::Isolate* isolate) override { return false; } + v8::TracingController* GetTracingController() override { + return platform_->GetTracingController(); + } + bool PendingTask() { return task_ != nullptr; } void PerformTask() { diff --git a/deps/v8/test/cctest/libplatform/test-tracing.cc b/deps/v8/test/cctest/libplatform/test-tracing.cc index 5dc6b965f16c48..369d7bc7621a3f 100644 --- a/deps/v8/test/cctest/libplatform/test-tracing.cc +++ b/deps/v8/test/cctest/libplatform/test-tracing.cc @@ -4,6 +4,7 @@ #include #include "include/libplatform/v8-tracing.h" +#include "src/libplatform/default-platform.h" #include "src/tracing/trace-event.h" #include "test/cctest/cctest.h" @@ -135,7 +136,8 @@ TEST(TestJSONTraceWriter) { // Create a scope for the tracing controller to terminate the trace writer. { TracingController tracing_controller; - platform::SetTracingController(default_platform, &tracing_controller); + static_cast(default_platform) + ->SetTracingController(&tracing_controller); TraceWriter* writer = TraceWriter::CreateJSONTraceWriter(stream); TraceBuffer* ring_buffer = @@ -178,7 +180,8 @@ TEST(TestTracingController) { i::V8::SetPlatformForTesting(default_platform); TracingController tracing_controller; - platform::SetTracingController(default_platform, &tracing_controller); + static_cast(default_platform) + ->SetTracingController(&tracing_controller); MockTraceWriter* writer = new MockTraceWriter(); TraceBuffer* ring_buffer = @@ -244,7 +247,8 @@ TEST(TestTracingControllerMultipleArgsAndCopy) { // Create a scope for the tracing controller to terminate the trace writer. { TracingController tracing_controller; - platform::SetTracingController(default_platform, &tracing_controller); + static_cast(default_platform) + ->SetTracingController(&tracing_controller); TraceWriter* writer = TraceWriter::CreateJSONTraceWriter(stream); TraceBuffer* ring_buffer = @@ -339,7 +343,7 @@ TEST(TestTracingControllerMultipleArgsAndCopy) { namespace { -class TraceStateObserverImpl : public Platform::TraceStateObserver { +class TraceStateObserverImpl : public TracingController::TraceStateObserver { public: void OnTraceEnabled() override { ++enabled_count; } void OnTraceDisabled() override { ++disabled_count; } @@ -356,7 +360,8 @@ TEST(TracingObservers) { i::V8::SetPlatformForTesting(default_platform); v8::platform::tracing::TracingController tracing_controller; - v8::platform::SetTracingController(default_platform, &tracing_controller); + static_cast(default_platform) + ->SetTracingController(&tracing_controller); MockTraceWriter* writer = new MockTraceWriter(); v8::platform::tracing::TraceBuffer* ring_buffer = v8::platform::tracing::TraceBuffer::CreateTraceBufferRingBuffer(1, @@ -367,7 +372,7 @@ TEST(TracingObservers) { trace_config->AddIncludedCategory("v8"); TraceStateObserverImpl observer; - default_platform->AddTraceStateObserver(&observer); + tracing_controller.AddTraceStateObserver(&observer); CHECK_EQ(0, observer.enabled_count); CHECK_EQ(0, observer.disabled_count); @@ -378,12 +383,12 @@ TEST(TracingObservers) { CHECK_EQ(0, observer.disabled_count); TraceStateObserverImpl observer2; - default_platform->AddTraceStateObserver(&observer2); + tracing_controller.AddTraceStateObserver(&observer2); CHECK_EQ(1, observer2.enabled_count); CHECK_EQ(0, observer2.disabled_count); - default_platform->RemoveTraceStateObserver(&observer2); + tracing_controller.RemoveTraceStateObserver(&observer2); CHECK_EQ(1, observer2.enabled_count); CHECK_EQ(0, observer2.disabled_count); @@ -395,7 +400,7 @@ TEST(TracingObservers) { CHECK_EQ(1, observer2.enabled_count); CHECK_EQ(0, observer2.disabled_count); - default_platform->RemoveTraceStateObserver(&observer); + tracing_controller.RemoveTraceStateObserver(&observer); CHECK_EQ(1, observer.enabled_count); CHECK_EQ(1, observer.disabled_count); diff --git a/deps/v8/test/cctest/test-cpu-profiler.cc b/deps/v8/test/cctest/test-cpu-profiler.cc index 9ccc93f0f5b3f0..1ffb5dfcaf6256 100644 --- a/deps/v8/test/cctest/test-cpu-profiler.cc +++ b/deps/v8/test/cctest/test-cpu-profiler.cc @@ -33,6 +33,7 @@ #include "src/api.h" #include "src/base/platform/platform.h" #include "src/deoptimizer.h" +#include "src/libplatform/default-platform.h" #include "src/objects-inl.h" #include "src/profiler/cpu-profiler-inl.h" #include "src/profiler/profiler-listener.h" @@ -2152,7 +2153,8 @@ TEST(TracingCpuProfiler) { i::V8::SetPlatformForTesting(default_platform); v8::platform::tracing::TracingController tracing_controller; - v8::platform::SetTracingController(default_platform, &tracing_controller); + static_cast(default_platform) + ->SetTracingController(&tracing_controller); CpuProfileEventChecker* event_checker = new CpuProfileEventChecker(); TraceBuffer* ring_buffer = diff --git a/deps/v8/test/cctest/test-trace-event.cc b/deps/v8/test/cctest/test-trace-event.cc index 88f295f30126f6..bc1684dfa59fa1 100644 --- a/deps/v8/test/cctest/test-trace-event.cc +++ b/deps/v8/test/cctest/test-trace-event.cc @@ -40,38 +40,16 @@ struct MockTraceObject { typedef v8::internal::List MockTraceObjectList; -class MockTracingPlatform : public v8::Platform { +class MockTracingController : public v8::TracingController { public: - explicit MockTracingPlatform(v8::Platform* platform) {} - virtual ~MockTracingPlatform() { + MockTracingController() = default; + ~MockTracingController() { for (int i = 0; i < trace_object_list_.length(); ++i) { delete trace_object_list_[i]; } trace_object_list_.Clear(); } - void CallOnBackgroundThread(Task* task, - ExpectedRuntime expected_runtime) override {} - - void CallOnForegroundThread(Isolate* isolate, Task* task) override {} - - void CallDelayedOnForegroundThread(Isolate* isolate, Task* task, - double delay_in_seconds) override {} - - double MonotonicallyIncreasingTime() override { return 0.0; } - - void CallIdleOnForegroundThread(Isolate* isolate, IdleTask* task) override {} - - bool IdleTasksEnabled(Isolate* isolate) override { return false; } - bool PendingIdleTask() { return false; } - - void PerformIdleTask(double idle_time_in_seconds) {} - - bool PendingDelayedTask() { return false; } - - void PerformDelayedTask() {} - - using Platform::AddTraceEvent; uint64_t AddTraceEvent( char phase, const uint8_t* category_enabled_flag, const char* name, const char* scope, uint64_t id, uint64_t bind_id, int num_args, @@ -98,16 +76,52 @@ class MockTracingPlatform : public v8::Platform { } } - const char* GetCategoryGroupName( - const uint8_t* category_enabled_flag) override { - static const char dummy[] = "dummy"; - return dummy; - } - MockTraceObjectList* GetMockTraceObjects() { return &trace_object_list_; } private: MockTraceObjectList trace_object_list_; + + DISALLOW_COPY_AND_ASSIGN(MockTracingController); +}; + +class MockTracingPlatform : public v8::Platform { + public: + explicit MockTracingPlatform(v8::Platform* platform) {} + virtual ~MockTracingPlatform() {} + void CallOnBackgroundThread(Task* task, + ExpectedRuntime expected_runtime) override {} + + void CallOnForegroundThread(Isolate* isolate, Task* task) override {} + + void CallDelayedOnForegroundThread(Isolate* isolate, Task* task, + double delay_in_seconds) override {} + + double MonotonicallyIncreasingTime() override { return 0.0; } + + void CallIdleOnForegroundThread(Isolate* isolate, IdleTask* task) override {} + + bool IdleTasksEnabled(Isolate* isolate) override { return false; } + + v8::TracingController* GetTracingController() override { + return &tracing_controller_; + } + + bool PendingIdleTask() { return false; } + + void PerformIdleTask(double idle_time_in_seconds) {} + + bool PendingDelayedTask() { return false; } + + void PerformDelayedTask() {} + + MockTraceObjectList* GetMockTraceObjects() { + return tracing_controller_.GetMockTraceObjects(); + } + + private: + MockTracingController tracing_controller_; + + DISALLOW_COPY_AND_ASSIGN(MockTracingPlatform); }; diff --git a/deps/v8/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc b/deps/v8/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc index 143b5d4ad58e3e..7766fb6a2146fd 100644 --- a/deps/v8/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc +++ b/deps/v8/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc @@ -103,7 +103,12 @@ namespace { class MockPlatform : public v8::Platform { public: - MockPlatform() : time_(0.0), time_step_(0.0), idle_task_(nullptr), sem_(0) {} + explicit MockPlatform(v8::TracingController* tracing_controller) + : time_(0.0), + time_step_(0.0), + idle_task_(nullptr), + sem_(0), + tracing_controller_(tracing_controller) {} ~MockPlatform() override { base::LockGuard lock(&mutex_); EXPECT_TRUE(foreground_tasks_.empty()); @@ -143,6 +148,10 @@ class MockPlatform : public v8::Platform { return time_; } + v8::TracingController* GetTracingController() override { + return tracing_controller_; + } + void RunIdleTask(double deadline_in_seconds, double time_step) { time_step_ = time_step; IdleTask* task; @@ -269,6 +278,8 @@ class MockPlatform : public v8::Platform { base::Semaphore sem_; + v8::TracingController* tracing_controller_; + DISALLOW_COPY_AND_ASSIGN(MockPlatform); }; @@ -277,12 +288,12 @@ const char test_script[] = "(x) { x*x; }"; } // namespace TEST_F(CompilerDispatcherTest, Construct) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); } TEST_F(CompilerDispatcherTest, IsEnqueued) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -300,7 +311,7 @@ TEST_F(CompilerDispatcherTest, IsEnqueued) { } TEST_F(CompilerDispatcherTest, FinishNow) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -319,7 +330,7 @@ TEST_F(CompilerDispatcherTest, FinishNow) { } TEST_F(CompilerDispatcherTest, FinishAllNow) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); constexpr int num_funcs = 2; @@ -349,7 +360,7 @@ TEST_F(CompilerDispatcherTest, FinishAllNow) { } TEST_F(CompilerDispatcherTest, IdleTask) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -370,7 +381,7 @@ TEST_F(CompilerDispatcherTest, IdleTask) { } TEST_F(CompilerDispatcherTest, IdleTaskSmallIdleTime) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -409,7 +420,7 @@ TEST_F(CompilerDispatcherTest, IdleTaskSmallIdleTime) { } TEST_F(CompilerDispatcherTest, IdleTaskException) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, 50); std::string func_name("f" STR(__LINE__)); @@ -436,7 +447,7 @@ TEST_F(CompilerDispatcherTest, IdleTaskException) { } TEST_F(CompilerDispatcherTest, CompileOnBackgroundThread) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -480,7 +491,7 @@ TEST_F(CompilerDispatcherTest, CompileOnBackgroundThread) { } TEST_F(CompilerDispatcherTest, FinishNowWithBackgroundTask) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -520,7 +531,7 @@ TEST_F(CompilerDispatcherTest, FinishNowWithBackgroundTask) { } TEST_F(CompilerDispatcherTest, IdleTaskMultipleJobs) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script1[] = TEST_SCRIPT(); @@ -549,7 +560,7 @@ TEST_F(CompilerDispatcherTest, IdleTaskMultipleJobs) { } TEST_F(CompilerDispatcherTest, FinishNowException) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, 50); std::string func_name("f" STR(__LINE__)); @@ -577,7 +588,7 @@ TEST_F(CompilerDispatcherTest, FinishNowException) { } TEST_F(CompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -620,7 +631,7 @@ TEST_F(CompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) { } TEST_F(CompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script1[] = TEST_SCRIPT(); @@ -702,7 +713,7 @@ TEST_F(CompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) { } TEST_F(CompilerDispatcherTest, FinishNowDuringAbortAll) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -781,7 +792,7 @@ TEST_F(CompilerDispatcherTest, FinishNowDuringAbortAll) { } TEST_F(CompilerDispatcherTest, MemoryPressure) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -829,7 +840,7 @@ class PressureNotificationTask : public CancelableTask { } // namespace TEST_F(CompilerDispatcherTest, MemoryPressureFromBackground) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -862,7 +873,7 @@ TEST_F(CompilerDispatcherTest, MemoryPressureFromBackground) { } TEST_F(CompilerDispatcherTest, EnqueueJob) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); Handle f = @@ -881,7 +892,7 @@ TEST_F(CompilerDispatcherTest, EnqueueJob) { } TEST_F(CompilerDispatcherTest, EnqueueWithoutSFI) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); ASSERT_TRUE(dispatcher.jobs_.empty()); ASSERT_TRUE(dispatcher.shared_to_job_id_.empty()); @@ -906,7 +917,7 @@ TEST_F(CompilerDispatcherTest, EnqueueWithoutSFI) { } TEST_F(CompilerDispatcherTest, EnqueueAndStep) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script[] = TEST_SCRIPT(); @@ -928,7 +939,7 @@ TEST_F(CompilerDispatcherTest, EnqueueAndStep) { } TEST_F(CompilerDispatcherTest, EnqueueParsed) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char source[] = TEST_SCRIPT(); @@ -955,7 +966,7 @@ TEST_F(CompilerDispatcherTest, EnqueueParsed) { } TEST_F(CompilerDispatcherTest, EnqueueAndStepParsed) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char source[] = TEST_SCRIPT(); @@ -984,7 +995,7 @@ TEST_F(CompilerDispatcherTest, EnqueueAndStepParsed) { } TEST_F(CompilerDispatcherTest, CompileParsedOutOfScope) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char source[] = TEST_SCRIPT(); @@ -1046,7 +1057,7 @@ class MockNativeFunctionExtension : public Extension { } // namespace TEST_F(CompilerDispatcherTestWithoutContext, CompileExtensionWithoutContext) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); Local context = v8::Context::New(isolate()); @@ -1145,7 +1156,7 @@ TEST_F(CompilerDispatcherTest, CompileLazy2FinishesDispatcherJob) { } TEST_F(CompilerDispatcherTest, EnqueueAndStepTwice) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char source[] = TEST_SCRIPT(); @@ -1186,7 +1197,7 @@ TEST_F(CompilerDispatcherTest, EnqueueAndStepTwice) { } TEST_F(CompilerDispatcherTest, CompileMultipleOnBackgroundThread) { - MockPlatform platform; + MockPlatform platform(V8::GetCurrentPlatform()->GetTracingController()); CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size); const char script1[] = TEST_SCRIPT(); From e928c4acf7171834c3e7b09a80d6910d7ca6a929 Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Thu, 3 Aug 2017 07:05:36 -0700 Subject: [PATCH 4/8] deps: backport 6e9e2e5 from upstream V8 Original commit message: [heap] Move SweeperTask to CancelableTask This mitigates the problem of blocking on the main thread when the platform is unable to execute background tasks in a timely manner. Bug: v8:6655 Change-Id: Icdaae744ee73146b86b9a28c8035138746721971 Reviewed-on: https://chromium-review.googlesource.com/595467 Commit-Queue: Michael Lippautz Reviewed-by: Ulan Degenbaev Cr-Commit-Position: refs/heads/master@{#47036} --- deps/v8/src/heap/mark-compact.cc | 40 ++++++++++++++++++++------------ deps/v8/src/heap/mark-compact.h | 13 ++++++----- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/deps/v8/src/heap/mark-compact.cc b/deps/v8/src/heap/mark-compact.cc index 6bb7d3e352a65b..d970e1a50e3f8a 100644 --- a/deps/v8/src/heap/mark-compact.cc +++ b/deps/v8/src/heap/mark-compact.cc @@ -7,6 +7,7 @@ #include "src/base/atomicops.h" #include "src/base/bits.h" #include "src/base/sys-info.h" +#include "src/cancelable-task.h" #include "src/code-stubs.h" #include "src/compilation-cache.h" #include "src/deoptimizer.h" @@ -546,12 +547,14 @@ void MarkCompactCollector::ClearMarkbits() { heap_->lo_space()->ClearMarkingStateOfLiveObjects(); } -class MarkCompactCollector::Sweeper::SweeperTask : public v8::Task { +class MarkCompactCollector::Sweeper::SweeperTask final : public CancelableTask { public: - SweeperTask(Sweeper* sweeper, base::Semaphore* pending_sweeper_tasks, + SweeperTask(Isolate* isolate, Sweeper* sweeper, + base::Semaphore* pending_sweeper_tasks, base::AtomicNumber* num_sweeping_tasks, AllocationSpace space_to_start) - : sweeper_(sweeper), + : CancelableTask(isolate), + sweeper_(sweeper), pending_sweeper_tasks_(pending_sweeper_tasks), num_sweeping_tasks_(num_sweeping_tasks), space_to_start_(space_to_start) {} @@ -559,8 +562,7 @@ class MarkCompactCollector::Sweeper::SweeperTask : public v8::Task { virtual ~SweeperTask() {} private: - // v8::Task overrides. - void Run() override { + void RunInternal() final { DCHECK_GE(space_to_start_, FIRST_SPACE); DCHECK_LE(space_to_start_, LAST_PAGED_SPACE); const int offset = space_to_start_ - FIRST_SPACE; @@ -575,9 +577,9 @@ class MarkCompactCollector::Sweeper::SweeperTask : public v8::Task { pending_sweeper_tasks_->Signal(); } - Sweeper* sweeper_; - base::Semaphore* pending_sweeper_tasks_; - base::AtomicNumber* num_sweeping_tasks_; + Sweeper* const sweeper_; + base::Semaphore* const pending_sweeper_tasks_; + base::AtomicNumber* const num_sweeping_tasks_; AllocationSpace space_to_start_; DISALLOW_COPY_AND_ASSIGN(SweeperTask); @@ -595,15 +597,19 @@ void MarkCompactCollector::Sweeper::StartSweeping() { } void MarkCompactCollector::Sweeper::StartSweeperTasks() { + DCHECK_EQ(0, num_tasks_); + DCHECK_EQ(0, num_sweeping_tasks_.Value()); if (FLAG_concurrent_sweeping && sweeping_in_progress_) { ForAllSweepingSpaces([this](AllocationSpace space) { if (space == NEW_SPACE) return; num_sweeping_tasks_.Increment(1); - semaphore_counter_++; + SweeperTask* task = new SweeperTask(heap_->isolate(), this, + &pending_sweeper_tasks_semaphore_, + &num_sweeping_tasks_, space); + DCHECK_LT(num_tasks_, kMaxSweeperTasks); + task_ids_[num_tasks_++] = task->id(); V8::GetCurrentPlatform()->CallOnBackgroundThread( - new SweeperTask(this, &pending_sweeper_tasks_semaphore_, - &num_sweeping_tasks_, space), - v8::Platform::kShortRunningTask); + task, v8::Platform::kShortRunningTask); }); } } @@ -646,10 +652,14 @@ void MarkCompactCollector::Sweeper::EnsureCompleted() { [this](AllocationSpace space) { ParallelSweepSpace(space, 0); }); if (FLAG_concurrent_sweeping) { - while (semaphore_counter_ > 0) { - pending_sweeper_tasks_semaphore_.Wait(); - semaphore_counter_--; + for (int i = 0; i < num_tasks_; i++) { + if (heap_->isolate()->cancelable_task_manager()->TryAbort(task_ids_[i]) != + CancelableTaskManager::kTaskAborted) { + pending_sweeper_tasks_semaphore_.Wait(); + } } + num_tasks_ = 0; + num_sweeping_tasks_.SetValue(0); } ForAllSweepingSpaces([this](AllocationSpace space) { diff --git a/deps/v8/src/heap/mark-compact.h b/deps/v8/src/heap/mark-compact.h index 24ec7043abe22d..2babbc92964803 100644 --- a/deps/v8/src/heap/mark-compact.h +++ b/deps/v8/src/heap/mark-compact.h @@ -408,8 +408,6 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { class Sweeper { public: - class SweeperTask; - enum FreeListRebuildingMode { REBUILD_FREE_LIST, IGNORE_FREE_LIST }; enum ClearOldToNewSlotsMode { DO_NOT_CLEAR, @@ -425,8 +423,8 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { explicit Sweeper(Heap* heap) : heap_(heap), + num_tasks_(0), pending_sweeper_tasks_semaphore_(0), - semaphore_counter_(0), sweeping_in_progress_(false), num_sweeping_tasks_(0) {} @@ -452,7 +450,10 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { Page* GetSweptPageSafe(PagedSpace* space); private: + class SweeperTask; + static const int kAllocationSpaces = LAST_PAGED_SPACE + 1; + static const int kMaxSweeperTasks = kAllocationSpaces; static ClearOldToNewSlotsMode GetClearOldToNewSlotsMode(Page* p); @@ -468,10 +469,10 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { void PrepareToBeSweptPage(AllocationSpace space, Page* page); - Heap* heap_; + Heap* const heap_; + int num_tasks_; + uint32_t task_ids_[kMaxSweeperTasks]; base::Semaphore pending_sweeper_tasks_semaphore_; - // Counter is only used for waiting on the semaphore. - intptr_t semaphore_counter_; base::Mutex mutex_; SweptList swept_list_[kAllocationSpaces]; SweepingList sweeping_list_[kAllocationSpaces]; From 904a9b59771d57d803ac5461fde05018a7344048 Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Thu, 3 Aug 2017 07:14:43 -0700 Subject: [PATCH 5/8] deps: backport bca8409 from upstream V8 Original commit message: Make CancelableTask ids unique They were only limited to 32 bit when using the internal Hashmap. Since this has changed alreay some time ago, we can switch to 64 bit ids and check that we never overflow. Bug: Change-Id: Ia6c6d02d6b5e555c6941185a79427dc4aa2a1d62 Reviewed-on: https://chromium-review.googlesource.com/598229 Commit-Queue: Michael Lippautz Reviewed-by: Ulan Degenbaev Cr-Commit-Position: refs/heads/master@{#47085} --- deps/v8/src/cancelable-task.cc | 13 ++++++------- deps/v8/src/cancelable-task.h | 18 ++++++++++-------- deps/v8/src/heap/item-parallel-job.h | 3 ++- deps/v8/src/heap/mark-compact.h | 2 +- .../unittests/cancelable-tasks-unittest.cc | 4 ++-- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/deps/v8/src/cancelable-task.cc b/deps/v8/src/cancelable-task.cc index b0387f4dc05b55..9e48fe7593cf03 100644 --- a/deps/v8/src/cancelable-task.cc +++ b/deps/v8/src/cancelable-task.cc @@ -29,18 +29,17 @@ Cancelable::~Cancelable() { CancelableTaskManager::CancelableTaskManager() : task_id_counter_(0), canceled_(false) {} -uint32_t CancelableTaskManager::Register(Cancelable* task) { +CancelableTaskManager::Id CancelableTaskManager::Register(Cancelable* task) { base::LockGuard guard(&mutex_); - uint32_t id = ++task_id_counter_; - // The loop below is just used when task_id_counter_ overflows. - while (cancelable_tasks_.count(id) > 0) ++id; + CancelableTaskManager::Id id = ++task_id_counter_; + // Id overflows are not supported. + CHECK_NE(0, id); CHECK(!canceled_); cancelable_tasks_[id] = task; return id; } - -void CancelableTaskManager::RemoveFinishedTask(uint32_t id) { +void CancelableTaskManager::RemoveFinishedTask(CancelableTaskManager::Id id) { base::LockGuard guard(&mutex_); size_t removed = cancelable_tasks_.erase(id); USE(removed); @@ -49,7 +48,7 @@ void CancelableTaskManager::RemoveFinishedTask(uint32_t id) { } CancelableTaskManager::TryAbortResult CancelableTaskManager::TryAbort( - uint32_t id) { + CancelableTaskManager::Id id) { base::LockGuard guard(&mutex_); auto entry = cancelable_tasks_.find(id); if (entry != cancelable_tasks_.end()) { diff --git a/deps/v8/src/cancelable-task.h b/deps/v8/src/cancelable-task.h index 5b1a5f1def2f8d..8a1ad325c8c6ac 100644 --- a/deps/v8/src/cancelable-task.h +++ b/deps/v8/src/cancelable-task.h @@ -5,7 +5,7 @@ #ifndef V8_CANCELABLE_TASK_H_ #define V8_CANCELABLE_TASK_H_ -#include +#include #include "include/v8-platform.h" #include "src/base/atomic-utils.h" @@ -24,12 +24,14 @@ class Isolate; // from any fore- and background task/thread. class V8_EXPORT_PRIVATE CancelableTaskManager { public: + using Id = uint64_t; + CancelableTaskManager(); // Registers a new cancelable {task}. Returns the unique {id} of the task that // can be used to try to abort a task by calling {Abort}. // Must not be called after CancelAndWait. - uint32_t Register(Cancelable* task); + Id Register(Cancelable* task); // Try to abort running a task identified by {id}. The possible outcomes are: // (1) The task is already finished running or was canceled before and @@ -39,7 +41,7 @@ class V8_EXPORT_PRIVATE CancelableTaskManager { // removed. // enum TryAbortResult { kTaskRemoved, kTaskRunning, kTaskAborted }; - TryAbortResult TryAbort(uint32_t id); + TryAbortResult TryAbort(Id id); // Cancels all remaining registered tasks and waits for tasks that are // already running. This disallows subsequent Register calls. @@ -59,13 +61,13 @@ class V8_EXPORT_PRIVATE CancelableTaskManager { private: // Only called by {Cancelable} destructor. The task is done with executing, // but needs to be removed. - void RemoveFinishedTask(uint32_t id); + void RemoveFinishedTask(Id id); // To mitigate the ABA problem, the api refers to tasks through an id. - uint32_t task_id_counter_; + Id task_id_counter_; // A set of cancelable tasks that are currently registered. - std::map cancelable_tasks_; + std::unordered_map cancelable_tasks_; // Mutex and condition variable enabling concurrent register and removing, as // well as waiting for background tasks on {CancelAndWait}. @@ -89,7 +91,7 @@ class V8_EXPORT_PRIVATE Cancelable { // a platform. This step transfers ownership to the platform, which destroys // the task after running it. Since the exact time is not known, we cannot // access the object after handing it to a platform. - uint32_t id() { return id_; } + CancelableTaskManager::Id id() { return id_; } protected: bool TryRun() { return status_.TrySetValue(kWaiting, kRunning); } @@ -120,7 +122,7 @@ class V8_EXPORT_PRIVATE Cancelable { CancelableTaskManager* parent_; base::AtomicValue status_; - uint32_t id_; + CancelableTaskManager::Id id_; // The counter is incremented for failing tries to cancel a task. This can be // used by the task itself as an indication how often external entities tried diff --git a/deps/v8/src/heap/item-parallel-job.h b/deps/v8/src/heap/item-parallel-job.h index c3228403ff87ab..4c2b37e9e6ad1a 100644 --- a/deps/v8/src/heap/item-parallel-job.h +++ b/deps/v8/src/heap/item-parallel-job.h @@ -133,7 +133,8 @@ class ItemParallelJob { const size_t num_tasks = tasks_.size(); const size_t num_items = items_.size(); const size_t items_per_task = (num_items + num_tasks - 1) / num_tasks; - uint32_t* task_ids = new uint32_t[num_tasks]; + CancelableTaskManager::Id* task_ids = + new CancelableTaskManager::Id[num_tasks]; size_t start_index = 0; Task* main_task = nullptr; Task* task = nullptr; diff --git a/deps/v8/src/heap/mark-compact.h b/deps/v8/src/heap/mark-compact.h index 2babbc92964803..e32ab4c6f19eba 100644 --- a/deps/v8/src/heap/mark-compact.h +++ b/deps/v8/src/heap/mark-compact.h @@ -471,7 +471,7 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { Heap* const heap_; int num_tasks_; - uint32_t task_ids_[kMaxSweeperTasks]; + CancelableTaskManager::Id task_ids_[kMaxSweeperTasks]; base::Semaphore pending_sweeper_tasks_semaphore_; base::Mutex mutex_; SweptList swept_list_[kAllocationSpaces]; diff --git a/deps/v8/test/unittests/cancelable-tasks-unittest.cc b/deps/v8/test/unittests/cancelable-tasks-unittest.cc index eb5dd915897dc8..d0462877f58256 100644 --- a/deps/v8/test/unittests/cancelable-tasks-unittest.cc +++ b/deps/v8/test/unittests/cancelable-tasks-unittest.cc @@ -180,7 +180,7 @@ TEST(CancelableTask, RemoveBeforeCancelAndWait) { ResultType result1 = 0; TestTask* task1 = new TestTask(&manager, &result1, TestTask::kCheckNotRun); ThreadedRunner runner1(task1); - uint32_t id = task1->id(); + CancelableTaskManager::Id id = task1->id(); EXPECT_EQ(id, 1u); EXPECT_TRUE(manager.TryAbort(id)); runner1.Start(); @@ -195,7 +195,7 @@ TEST(CancelableTask, RemoveAfterCancelAndWait) { ResultType result1 = 0; TestTask* task1 = new TestTask(&manager, &result1); ThreadedRunner runner1(task1); - uint32_t id = task1->id(); + CancelableTaskManager::Id id = task1->id(); EXPECT_EQ(id, 1u); runner1.Start(); runner1.Join(); From 7a43eb7a6bf54b046ab4be75d2971b2510fb48d5 Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Thu, 3 Aug 2017 07:23:13 -0700 Subject: [PATCH 6/8] deps: backport f9c4b7a from upstream V8 Original commit message: [heap] Move UnmapFreeMemoryTask to CancelableTask This mitigates the problem of blocking on the main thread when the platform is unable to execute background tasks in a timely manner. Bug: v8:6671 Change-Id: I741d4b7594e8d62721dad32cbfb19551ffacd0c3 Reviewed-on: https://chromium-review.googlesource.com/599528 Commit-Queue: Michael Lippautz Reviewed-by: Ulan Degenbaev Cr-Commit-Position: refs/heads/master@{#47126} --- deps/v8/src/heap/heap.cc | 2 ++ deps/v8/src/heap/heap.h | 4 +++ deps/v8/src/heap/spaces.cc | 40 ++++++++++++++----------- deps/v8/src/heap/spaces.h | 13 +++++--- deps/v8/src/isolate.cc | 1 + deps/v8/test/cctest/heap/test-spaces.cc | 1 + 6 files changed, 40 insertions(+), 21 deletions(-) diff --git a/deps/v8/src/heap/heap.cc b/deps/v8/src/heap/heap.cc index 2b26e0d1b9f0f7..fa47dc825b71d3 100644 --- a/deps/v8/src/heap/heap.cc +++ b/deps/v8/src/heap/heap.cc @@ -161,6 +161,7 @@ Heap::Heap() heap_iterator_depth_(0), local_embedder_heap_tracer_(nullptr), fast_promotion_mode_(false), + use_tasks_(true), force_oom_(false), delay_sweeper_tasks_for_testing_(false), pending_layout_change_object_(nullptr) { @@ -5823,6 +5824,7 @@ void Heap::RegisterExternallyReferencedObject(Object** object) { } void Heap::TearDown() { + use_tasks_ = false; #ifdef VERIFY_HEAP if (FLAG_verify_heap) { Verify(); diff --git a/deps/v8/src/heap/heap.h b/deps/v8/src/heap/heap.h index f18f3edd3f1231..7f213eff2724cf 100644 --- a/deps/v8/src/heap/heap.h +++ b/deps/v8/src/heap/heap.h @@ -1016,6 +1016,8 @@ class Heap { // Returns whether SetUp has been called. bool HasBeenSetUp(); + bool use_tasks() const { return use_tasks_; } + // =========================================================================== // Getters for spaces. ======================================================= // =========================================================================== @@ -2375,6 +2377,8 @@ class Heap { bool fast_promotion_mode_; + bool use_tasks_; + // Used for testing purposes. bool force_oom_; bool delay_sweeper_tasks_for_testing_; diff --git a/deps/v8/src/heap/spaces.cc b/deps/v8/src/heap/spaces.cc index 71e1b60be978ec..3e677888280e0c 100644 --- a/deps/v8/src/heap/spaces.cc +++ b/deps/v8/src/heap/spaces.cc @@ -300,7 +300,7 @@ MemoryAllocator::MemoryAllocator(Isolate* isolate) size_executable_(0), lowest_ever_allocated_(reinterpret_cast(-1)), highest_ever_allocated_(reinterpret_cast(0)), - unmapper_(this) {} + unmapper_(isolate->heap(), this) {} bool MemoryAllocator::SetUp(size_t capacity, size_t code_range_size) { capacity_ = RoundUp(capacity, Page::kPageSize); @@ -332,40 +332,46 @@ void MemoryAllocator::TearDown() { code_range_ = nullptr; } -class MemoryAllocator::Unmapper::UnmapFreeMemoryTask : public v8::Task { +class MemoryAllocator::Unmapper::UnmapFreeMemoryTask : public CancelableTask { public: - explicit UnmapFreeMemoryTask(Unmapper* unmapper) : unmapper_(unmapper) {} + explicit UnmapFreeMemoryTask(Isolate* isolate, Unmapper* unmapper) + : CancelableTask(isolate), unmapper_(unmapper) {} private: - // v8::Task overrides. - void Run() override { + void RunInternal() override { unmapper_->PerformFreeMemoryOnQueuedChunks(); unmapper_->pending_unmapping_tasks_semaphore_.Signal(); } - Unmapper* unmapper_; + Unmapper* const unmapper_; DISALLOW_COPY_AND_ASSIGN(UnmapFreeMemoryTask); }; void MemoryAllocator::Unmapper::FreeQueuedChunks() { ReconsiderDelayedChunks(); - if (FLAG_concurrent_sweeping) { + if (heap_->use_tasks() && FLAG_concurrent_sweeping) { + if (concurrent_unmapping_tasks_active_ >= kMaxUnmapperTasks) { + // kMaxUnmapperTasks are already running. Avoid creating any more. + return; + } + UnmapFreeMemoryTask* task = new UnmapFreeMemoryTask(heap_->isolate(), this); + DCHECK_LT(concurrent_unmapping_tasks_active_, kMaxUnmapperTasks); + task_ids_[concurrent_unmapping_tasks_active_++] = task->id(); V8::GetCurrentPlatform()->CallOnBackgroundThread( - new UnmapFreeMemoryTask(this), v8::Platform::kShortRunningTask); - concurrent_unmapping_tasks_active_++; + task, v8::Platform::kShortRunningTask); } else { PerformFreeMemoryOnQueuedChunks(); } } -bool MemoryAllocator::Unmapper::WaitUntilCompleted() { - bool waited = false; - while (concurrent_unmapping_tasks_active_ > 0) { - pending_unmapping_tasks_semaphore_.Wait(); - concurrent_unmapping_tasks_active_--; - waited = true; +void MemoryAllocator::Unmapper::WaitUntilCompleted() { + for (int i = 0; i < concurrent_unmapping_tasks_active_; i++) { + if (heap_->isolate()->cancelable_task_manager()->TryAbort(task_ids_[i]) != + CancelableTaskManager::kTaskAborted) { + pending_unmapping_tasks_semaphore_.Wait(); + } + concurrent_unmapping_tasks_active_ = 0; } - return waited; } template @@ -392,7 +398,7 @@ void MemoryAllocator::Unmapper::PerformFreeMemoryOnQueuedChunks() { } void MemoryAllocator::Unmapper::TearDown() { - WaitUntilCompleted(); + CHECK_EQ(0, concurrent_unmapping_tasks_active_); ReconsiderDelayedChunks(); CHECK(delayed_regular_chunks_.empty()); PerformFreeMemoryOnQueuedChunks(); diff --git a/deps/v8/src/heap/spaces.h b/deps/v8/src/heap/spaces.h index dc49f3d4a03973..5c37482ac2161b 100644 --- a/deps/v8/src/heap/spaces.h +++ b/deps/v8/src/heap/spaces.h @@ -15,6 +15,7 @@ #include "src/base/bits.h" #include "src/base/hashmap.h" #include "src/base/platform/mutex.h" +#include "src/cancelable-task.h" #include "src/flags.h" #include "src/globals.h" #include "src/heap/heap.h" @@ -1149,8 +1150,9 @@ class V8_EXPORT_PRIVATE MemoryAllocator { public: class UnmapFreeMemoryTask; - explicit Unmapper(MemoryAllocator* allocator) - : allocator_(allocator), + Unmapper(Heap* heap, MemoryAllocator* allocator) + : heap_(heap), + allocator_(allocator), pending_unmapping_tasks_semaphore_(0), concurrent_unmapping_tasks_active_(0) { chunks_[kRegular].reserve(kReservedQueueingSlots); @@ -1184,13 +1186,14 @@ class V8_EXPORT_PRIVATE MemoryAllocator { } void FreeQueuedChunks(); - bool WaitUntilCompleted(); + void WaitUntilCompleted(); void TearDown(); bool has_delayed_chunks() { return delayed_regular_chunks_.size() > 0; } private: static const int kReservedQueueingSlots = 64; + static const int kMaxUnmapperTasks = 24; enum ChunkQueueType { kRegular, // Pages of kPageSize that do not live in a CodeRange and @@ -1229,13 +1232,15 @@ class V8_EXPORT_PRIVATE MemoryAllocator { template void PerformFreeMemoryOnQueuedChunks(); + Heap* const heap_; + MemoryAllocator* const allocator_; base::Mutex mutex_; - MemoryAllocator* allocator_; std::vector chunks_[kNumberOfChunkQueues]; // Delayed chunks cannot be processed in the current unmapping cycle because // of dependencies such as an active sweeper. // See MemoryAllocator::CanFreeMemoryChunk. std::list delayed_regular_chunks_; + CancelableTaskManager::Id task_ids_[kMaxUnmapperTasks]; base::Semaphore pending_unmapping_tasks_semaphore_; intptr_t concurrent_unmapping_tasks_active_; diff --git a/deps/v8/src/isolate.cc b/deps/v8/src/isolate.cc index faa04848cff43c..366c14fb11dc2b 100644 --- a/deps/v8/src/isolate.cc +++ b/deps/v8/src/isolate.cc @@ -2455,6 +2455,7 @@ void Isolate::Deinit() { } heap_.mark_compact_collector()->EnsureSweepingCompleted(); + heap_.memory_allocator()->unmapper()->WaitUntilCompleted(); DumpAndResetStats(); diff --git a/deps/v8/test/cctest/heap/test-spaces.cc b/deps/v8/test/cctest/heap/test-spaces.cc index 0d625ca408078b..e9d2045f6f4f29 100644 --- a/deps/v8/test/cctest/heap/test-spaces.cc +++ b/deps/v8/test/cctest/heap/test-spaces.cc @@ -370,6 +370,7 @@ TEST(NewSpace) { } new_space.TearDown(); + memory_allocator->unmapper()->WaitUntilCompleted(); memory_allocator->TearDown(); delete memory_allocator; } From ef3f6ebd472a37ad5f8a290c7d436d16341da4fc Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Thu, 29 Jun 2017 10:55:38 +0200 Subject: [PATCH 7/8] tracing: Update to use new Platform tracing apis V8 modified the platform API to accept a tracing controller at platform creation time that is required to be present for the lifetime of the platform if tracing will every be enabled. This will simplify the implementation of a v8::Platform subclass for node. --- src/node.cc | 15 ++++++++------- src/tracing/agent.cc | 20 ++++++++------------ src/tracing/agent.h | 7 ++++--- src/tracing/trace_event.cc | 10 +++++----- src/tracing/trace_event.h | 18 +++++++++--------- 5 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/node.cc b/src/node.cc index 8ed4fc4bf25aed..a37753e385d629 100644 --- a/src/node.cc +++ b/src/node.cc @@ -251,12 +251,15 @@ node::DebugOptions debug_options; static struct { #if NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) { + tracing_agent_ = + trace_enabled ? new tracing::Agent() : nullptr; platform_ = v8::platform::CreateDefaultPlatform( - thread_pool_size, - v8::platform::IdleTaskSupport::kDisabled, - v8::platform::InProcessStackDumping::kDisabled); + thread_pool_size, v8::platform::IdleTaskSupport::kDisabled, + v8::platform::InProcessStackDumping::kDisabled, + trace_enabled ? tracing_agent_->GetTracingController() : nullptr); V8::InitializePlatform(platform_); - tracing::TraceEventHelper::SetCurrentPlatform(platform_); + tracing::TraceEventHelper::SetTracingController( + trace_enabled ? tracing_agent_->GetTracingController() : nullptr); } void PumpMessageLoop(Isolate* isolate) { @@ -283,9 +286,7 @@ static struct { #endif // HAVE_INSPECTOR void StartTracingAgent() { - CHECK(tracing_agent_ == nullptr); - tracing_agent_ = new tracing::Agent(); - tracing_agent_->Start(platform_, trace_enabled_categories); + tracing_agent_->Start(trace_enabled_categories); } void StopTracingAgent() { diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc index 669bae130c7f94..38e651ebb2a40e 100644 --- a/src/tracing/agent.cc +++ b/src/tracing/agent.cc @@ -12,20 +12,18 @@ namespace tracing { using v8::platform::tracing::TraceConfig; using std::string; -Agent::Agent() {} - -void Agent::Start(v8::Platform* platform, const string& enabled_categories) { - platform_ = platform; - +Agent::Agent() { int err = uv_loop_init(&tracing_loop_); CHECK_EQ(err, 0); NodeTraceWriter* trace_writer = new NodeTraceWriter(&tracing_loop_); TraceBuffer* trace_buffer = new NodeTraceBuffer( NodeTraceBuffer::kBufferChunks, trace_writer, &tracing_loop_); - tracing_controller_ = new TracingController(); + tracing_controller_->Initialize(trace_buffer); +} +void Agent::Start(const string& enabled_categories) { TraceConfig* trace_config = new TraceConfig(); if (!enabled_categories.empty()) { std::stringstream category_list(enabled_categories); @@ -42,27 +40,25 @@ void Agent::Start(v8::Platform* platform, const string& enabled_categories) { // This thread should be created *after* async handles are created // (within NodeTraceWriter and NodeTraceBuffer constructors). // Otherwise the thread could shut down prematurely. - err = uv_thread_create(&thread_, ThreadCb, this); + int err = uv_thread_create(&thread_, ThreadCb, this); CHECK_EQ(err, 0); - tracing_controller_->Initialize(trace_buffer); tracing_controller_->StartTracing(trace_config); - v8::platform::SetTracingController(platform, tracing_controller_); + started_ = true; } void Agent::Stop() { - if (!IsStarted()) { + if (!started_) { return; } // Perform final Flush on TraceBuffer. We don't want the tracing controller // to flush the buffer again on destruction of the V8::Platform. tracing_controller_->StopTracing(); tracing_controller_->Initialize(nullptr); - tracing_controller_ = nullptr; + started_ = false; // Thread should finish when the tracing loop is stopped. uv_thread_join(&thread_); - v8::platform::SetTracingController(platform_, nullptr); } // static diff --git a/src/tracing/agent.h b/src/tracing/agent.h index 6966c2e43be4d2..cc00c53144b1fa 100644 --- a/src/tracing/agent.h +++ b/src/tracing/agent.h @@ -12,16 +12,17 @@ namespace tracing { class Agent { public: Agent(); - void Start(v8::Platform* platform, const std::string& enabled_categories); + void Start(const std::string& enabled_categories); void Stop(); + TracingController* GetTracingController() { return tracing_controller_; } + private: - bool IsStarted() { return platform_ != nullptr; } static void ThreadCb(void* arg); uv_thread_t thread_; uv_loop_t tracing_loop_; - v8::Platform* platform_ = nullptr; + bool started_ = false; TracingController* tracing_controller_ = nullptr; }; diff --git a/src/tracing/trace_event.cc b/src/tracing/trace_event.cc index b83cae6e05c10c..856b344e9d2294 100644 --- a/src/tracing/trace_event.cc +++ b/src/tracing/trace_event.cc @@ -3,14 +3,14 @@ namespace node { namespace tracing { -v8::Platform* platform_ = nullptr; +v8::TracingController* controller_ = nullptr; -void TraceEventHelper::SetCurrentPlatform(v8::Platform* platform) { - platform_ = platform; +void TraceEventHelper::SetTracingController(v8::TracingController* controller) { + controller_ = controller; } -v8::Platform* TraceEventHelper::GetCurrentPlatform() { - return platform_; +v8::TracingController* TraceEventHelper::GetTracingController() { + return controller_; } } // namespace tracing diff --git a/src/tracing/trace_event.h b/src/tracing/trace_event.h index ac0c1f757885f2..24806d375fad58 100644 --- a/src/tracing/trace_event.h +++ b/src/tracing/trace_event.h @@ -70,7 +70,7 @@ enum CategoryGroupEnabledFlags { // const uint8_t* // TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(const char* category_group) #define TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED \ - node::tracing::TraceEventHelper::GetCurrentPlatform() \ + node::tracing::TraceEventHelper::GetTracingController() \ ->GetCategoryGroupEnabled // Get the number of times traces have been recorded. This is used to implement @@ -99,7 +99,7 @@ enum CategoryGroupEnabledFlags { // const char* name, // uint64_t id) #define TRACE_EVENT_API_UPDATE_TRACE_EVENT_DURATION \ - node::tracing::TraceEventHelper::GetCurrentPlatform() \ + node::tracing::TraceEventHelper::GetTracingController() \ ->UpdateTraceEventDuration // Defines atomic operations used internally by the tracing system. @@ -258,8 +258,8 @@ extern intptr_t kRuntimeCallStatsTracingEnabled; class TraceEventHelper { public: - static v8::Platform* GetCurrentPlatform(); - static void SetCurrentPlatform(v8::Platform* platform); + static v8::TracingController* GetTracingController(); + static void SetTracingController(v8::TracingController* controller); }; // TraceID encapsulates an ID that can either be an integer or pointer. Pointers @@ -406,11 +406,11 @@ static inline uint64_t AddTraceEventImpl( static_cast(arg_values[1]))); } // DCHECK(num_args <= 2); - v8::Platform* platform = - node::tracing::TraceEventHelper::GetCurrentPlatform(); - return platform->AddTraceEvent(phase, category_group_enabled, name, scope, id, - bind_id, num_args, arg_names, arg_types, - arg_values, arg_convertibles, flags); + v8::TracingController* controller = + node::tracing::TraceEventHelper::GetTracingController(); + return controller->AddTraceEvent(phase, category_group_enabled, name, scope, id, + bind_id, num_args, arg_names, arg_types, + arg_values, arg_convertibles, flags); } // Define SetTraceValue for each allowed type. It stores the type and From 0b0a805f67f8d8e4cb28065e96af89bf7137cbd4 Mon Sep 17 00:00:00 2001 From: Matt Loring Date: Mon, 13 Mar 2017 15:17:57 -0700 Subject: [PATCH 8/8] src: Node implementation of v8::Platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Node.js currently uses the V8 implementation of the DefaultPlatform which schedules VM tasks on a V8 managed thread pool. Since the Node.js event loop is not aware of these tasks, the Node.js process may exit while there are outstanding VM tasks. This will become problematic once asynchronous wasm compilation lands in V8. This PR introduces a Node.js specific implementation of the v8::Platform on top of libuv so that the event loop is aware of outstanding VM tasks. PR-URL: https://github.com/nodejs/node/pull/14001 Fixes: https://github.com/nodejs/node/issues/3665 Fixes: https://github.com/nodejs/node/issues/8496 Fixes: https://github.com/nodejs/node/issues/12980 Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis Reviewed-By: Tobias Nießen --- node.gyp | 4 + src/inspector_agent.cc | 15 +-- src/inspector_agent.h | 3 +- src/node.cc | 60 +++++----- src/node_platform.cc | 189 ++++++++++++++++++++++++++++++++ src/node_platform.h | 69 ++++++++++++ src/tracing/agent.cc | 1 - src/tracing/agent.h | 1 + src/tracing/trace_event.cc | 6 +- src/tracing/trace_event.h | 1 + test/cctest/node_test_fixture.h | 15 ++- test/cctest/test_environment.cc | 3 +- 12 files changed, 322 insertions(+), 45 deletions(-) create mode 100644 src/node_platform.cc create mode 100644 src/node_platform.h diff --git a/node.gyp b/node.gyp index 5ef250d5dd7023..08b792ef309c2a 100644 --- a/node.gyp +++ b/node.gyp @@ -190,6 +190,7 @@ 'src/node_http_parser.cc', 'src/node_main.cc', 'src/node_os.cc', + 'src/node_platform.cc', 'src/node_revert.cc', 'src/node_serdes.cc', 'src/node_url.cc', @@ -238,6 +239,7 @@ 'src/node_internals.h', 'src/node_javascript.h', 'src/node_mutex.h', + 'src/node_platform.h', 'src/node_root_certs.h', 'src/node_version.h', 'src/node_watchdog.h', @@ -656,6 +658,8 @@ 'defines': [ 'NODE_WANT_INTERNALS=1' ], 'sources': [ + 'src/node_platform.cc', + 'src/node_platform.h', 'test/cctest/test_base64.cc', 'test/cctest/test_environment.cc', 'test/cctest/test_util.cc', diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 2520fbdd533bdc..828006ecf2fbb4 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -502,11 +502,9 @@ class InspectorTimerHandle { class NodeInspectorClient : public V8InspectorClient { public: - NodeInspectorClient(node::Environment* env, - v8::Platform* platform) : env_(env), - platform_(platform), - terminated_(false), - running_nested_loop_(false) { + NodeInspectorClient(node::Environment* env, node::NodePlatform* platform) + : env_(env), platform_(platform), terminated_(false), + running_nested_loop_(false) { client_ = V8Inspector::create(env->isolate(), this); contextCreated(env->context(), "Node.js Main Context"); } @@ -518,8 +516,7 @@ class NodeInspectorClient : public V8InspectorClient { terminated_ = false; running_nested_loop_ = true; while (!terminated_ && channel_->waitForFrontendMessage()) { - while (v8::platform::PumpMessageLoop(platform_, env_->isolate())) - {} + platform_->FlushForegroundTasksInternal(); } terminated_ = false; running_nested_loop_ = false; @@ -647,7 +644,7 @@ class NodeInspectorClient : public V8InspectorClient { private: node::Environment* env_; - v8::Platform* platform_; + node::NodePlatform* platform_; bool terminated_; bool running_nested_loop_; std::unique_ptr client_; @@ -666,7 +663,7 @@ Agent::Agent(Environment* env) : parent_env_(env), Agent::~Agent() { } -bool Agent::Start(v8::Platform* platform, const char* path, +bool Agent::Start(node::NodePlatform* platform, const char* path, const DebugOptions& options) { path_ = path == nullptr ? "" : path; debug_options_ = options; diff --git a/src/inspector_agent.h b/src/inspector_agent.h index 6ec1bc28dc2e22..8195e001c2eb3c 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -14,6 +14,7 @@ // Forward declaration to break recursive dependency chain with src/env.h. namespace node { class Environment; +class NodePlatform; } // namespace node #include "v8.h" @@ -42,7 +43,7 @@ class Agent { ~Agent(); // Create client_, may create io_ if option enabled - bool Start(v8::Platform* platform, const char* path, + bool Start(node::NodePlatform* platform, const char* path, const DebugOptions& options); // Stop and destroy io_ void Stop(); diff --git a/src/node.cc b/src/node.cc index a37753e385d629..1ef5adce3bb7d1 100644 --- a/src/node.cc +++ b/src/node.cc @@ -23,6 +23,7 @@ #include "node_buffer.h" #include "node_constants.h" #include "node_javascript.h" +#include "node_platform.h" #include "node_version.h" #include "node_internals.h" #include "node_revert.h" @@ -250,25 +251,26 @@ node::DebugOptions debug_options; static struct { #if NODE_USE_V8_PLATFORM - void Initialize(int thread_pool_size) { + void Initialize(int thread_pool_size, uv_loop_t* loop) { tracing_agent_ = - trace_enabled ? new tracing::Agent() : nullptr; - platform_ = v8::platform::CreateDefaultPlatform( - thread_pool_size, v8::platform::IdleTaskSupport::kDisabled, - v8::platform::InProcessStackDumping::kDisabled, - trace_enabled ? tracing_agent_->GetTracingController() : nullptr); + trace_enabled ? new tracing::Agent() : nullptr; + platform_ = new NodePlatform(thread_pool_size, loop, + trace_enabled ? tracing_agent_->GetTracingController() : nullptr); V8::InitializePlatform(platform_); tracing::TraceEventHelper::SetTracingController( - trace_enabled ? tracing_agent_->GetTracingController() : nullptr); - } - - void PumpMessageLoop(Isolate* isolate) { - v8::platform::PumpMessageLoop(platform_, isolate); + trace_enabled ? tracing_agent_->GetTracingController() : nullptr); } void Dispose() { + platform_->Shutdown(); delete platform_; platform_ = nullptr; + delete tracing_agent_; + tracing_agent_ = nullptr; + } + + void DrainVMTasks() { + platform_->DrainBackgroundTasks(); } #if HAVE_INSPECTOR @@ -293,12 +295,12 @@ static struct { tracing_agent_->Stop(); } - v8::Platform* platform_; tracing::Agent* tracing_agent_; + NodePlatform* platform_; #else // !NODE_USE_V8_PLATFORM - void Initialize(int thread_pool_size) {} - void PumpMessageLoop(Isolate* isolate) {} + void Initialize(int thread_pool_size, uv_loop_t* loop) {} void Dispose() {} + void DrainVMTasks() {} bool StartInspector(Environment *env, const char* script_path, const node::DebugOptions& options) { env->ThrowError("Node compiled with NODE_USE_V8_PLATFORM=0"); @@ -4555,19 +4557,14 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, SealHandleScope seal(isolate); bool more; do { - v8_platform.PumpMessageLoop(isolate); - more = uv_run(env.event_loop(), UV_RUN_ONCE); - - if (more == false) { - v8_platform.PumpMessageLoop(isolate); - EmitBeforeExit(&env); - - // Emit `beforeExit` if the loop became alive either after emitting - // event, or after running some callbacks. - more = uv_loop_alive(env.event_loop()); - if (uv_run(env.event_loop(), UV_RUN_NOWAIT) != 0) - more = true; - } + uv_run(env.event_loop(), UV_RUN_DEFAULT); + + EmitBeforeExit(&env); + + v8_platform.DrainVMTasks(); + // Emit `beforeExit` if the loop became alive either after emitting + // event, or after running some callbacks. + more = uv_loop_alive(env.event_loop()); } while (more == true); } @@ -4577,6 +4574,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, RunAtExit(&env); uv_key_delete(&thread_local_env); + v8_platform.DrainVMTasks(); WaitForInspectorDisconnect(&env); #if defined(LEAK_SANITIZER) __lsan_do_leak_check(); @@ -4665,7 +4663,7 @@ int Start(int argc, char** argv) { V8::SetEntropySource(crypto::EntropySource); #endif // HAVE_OPENSSL - v8_platform.Initialize(v8_thread_pool_size); + v8_platform.Initialize(v8_thread_pool_size, uv_default_loop()); // Enable tracing when argv has --trace-events-enabled. if (trace_enabled) { fprintf(stderr, "Warning: Trace event is an experimental feature " @@ -4682,6 +4680,12 @@ int Start(int argc, char** argv) { v8_initialized = false; V8::Dispose(); + // uv_run cannot be called from the time before the beforeExit callback + // runs until the program exits unless the event loop has any referenced + // handles after beforeExit terminates. This prevents unrefed timers + // that happen to terminate during shutdown from being run unsafely. + // Since uv_run cannot be called, uv_async handles held by the platform + // will never be fully cleaned up. v8_platform.Dispose(); delete[] exec_argv; diff --git a/src/node_platform.cc b/src/node_platform.cc new file mode 100644 index 00000000000000..3d023114ad2691 --- /dev/null +++ b/src/node_platform.cc @@ -0,0 +1,189 @@ +#include "node_platform.h" + +#include "util.h" + +namespace node { + +using v8::Isolate; +using v8::Platform; +using v8::Task; +using v8::TracingController; + +static void FlushTasks(uv_async_t* handle) { + NodePlatform* platform = static_cast(handle->data); + platform->FlushForegroundTasksInternal(); +} + +static void BackgroundRunner(void* data) { + TaskQueue* background_tasks = static_cast*>(data); + while (Task* task = background_tasks->BlockingPop()) { + task->Run(); + delete task; + background_tasks->NotifyOfCompletion(); + } +} + +NodePlatform::NodePlatform(int thread_pool_size, uv_loop_t* loop, + TracingController* tracing_controller) + : loop_(loop) { + CHECK_EQ(0, uv_async_init(loop, &flush_tasks_, FlushTasks)); + flush_tasks_.data = static_cast(this); + uv_unref(reinterpret_cast(&flush_tasks_)); + if (tracing_controller) { + tracing_controller_.reset(tracing_controller); + } else { + 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)); + } +} + +void NodePlatform::Shutdown() { + background_tasks_.Stop(); + for (size_t i = 0; i < threads_.size(); i++) { + CHECK_EQ(0, uv_thread_join(threads_[i].get())); + } + // uv_run cannot be called from the time before the beforeExit callback + // runs until the program exits unless the event loop has any referenced + // handles after beforeExit terminates. This prevents unrefed timers + // that happen to terminate during shutdown from being run unsafely. + // Since uv_run cannot be called, this handle will never be fully cleaned + // up. + uv_close(reinterpret_cast(&flush_tasks_), nullptr); +} + +size_t NodePlatform::NumberOfAvailableBackgroundThreads() { + return threads_.size(); +} + +static void RunForegroundTask(uv_timer_t* handle) { + Task* task = static_cast(handle->data); + task->Run(); + delete task; + uv_close(reinterpret_cast(handle), [](uv_handle_t* handle) { + delete reinterpret_cast(handle); + }); +} + +void NodePlatform::DrainBackgroundTasks() { + FlushForegroundTasksInternal(); + background_tasks_.BlockingDrain(); +} + +void NodePlatform::FlushForegroundTasksInternal() { + while (auto delayed = foreground_delayed_tasks_.Pop()) { + uint64_t delay_millis = + static_cast(delayed->second + 0.5) * 1000; + uv_timer_t* handle = new uv_timer_t(); + handle->data = static_cast(delayed->first); + uv_timer_init(loop_, handle); + // Timers may not guarantee queue ordering of events with the same delay if + // the delay is non-zero. This should not be a problem in practice. + uv_timer_start(handle, RunForegroundTask, delay_millis, 0); + uv_unref(reinterpret_cast(handle)); + delete delayed; + } + while (Task* task = foreground_tasks_.Pop()) { + task->Run(); + delete task; + } +} + +void NodePlatform::CallOnBackgroundThread(Task* task, + ExpectedRuntime expected_runtime) { + background_tasks_.Push(task); +} + +void NodePlatform::CallOnForegroundThread(Isolate* isolate, Task* task) { + foreground_tasks_.Push(task); + uv_async_send(&flush_tasks_); +} + +void NodePlatform::CallDelayedOnForegroundThread(Isolate* isolate, + Task* task, + double delay_in_seconds) { + auto pair = new std::pair(task, delay_in_seconds); + foreground_delayed_tasks_.Push(pair); + uv_async_send(&flush_tasks_); +} + +bool NodePlatform::IdleTasksEnabled(Isolate* isolate) { return false; } + +double NodePlatform::MonotonicallyIncreasingTime() { + // Convert nanos to seconds. + return uv_hrtime() / 1e9; +} + +TracingController* NodePlatform::GetTracingController() { + return tracing_controller_.get(); +} + +template +TaskQueue::TaskQueue() + : lock_(), tasks_available_(), tasks_drained_(), + outstanding_tasks_(0), stopped_(false), task_queue_() { } + +template +void TaskQueue::Push(T* task) { + Mutex::ScopedLock scoped_lock(lock_); + outstanding_tasks_++; + task_queue_.push(task); + tasks_available_.Signal(scoped_lock); +} + +template +T* TaskQueue::Pop() { + Mutex::ScopedLock scoped_lock(lock_); + T* result = nullptr; + if (!task_queue_.empty()) { + result = task_queue_.front(); + task_queue_.pop(); + } + return result; +} + +template +T* TaskQueue::BlockingPop() { + Mutex::ScopedLock scoped_lock(lock_); + while (task_queue_.empty() && !stopped_) { + tasks_available_.Wait(scoped_lock); + } + if (stopped_) { + return nullptr; + } + T* result = task_queue_.front(); + task_queue_.pop(); + return result; +} + +template +void TaskQueue::NotifyOfCompletion() { + Mutex::ScopedLock scoped_lock(lock_); + if (--outstanding_tasks_ == 0) { + tasks_drained_.Broadcast(scoped_lock); + } +} + +template +void TaskQueue::BlockingDrain() { + Mutex::ScopedLock scoped_lock(lock_); + while (outstanding_tasks_ > 0) { + tasks_drained_.Wait(scoped_lock); + } +} + +template +void TaskQueue::Stop() { + Mutex::ScopedLock scoped_lock(lock_); + stopped_ = true; + tasks_available_.Broadcast(scoped_lock); +} + +} // namespace node diff --git a/src/node_platform.h b/src/node_platform.h new file mode 100644 index 00000000000000..668fcf28e40233 --- /dev/null +++ b/src/node_platform.h @@ -0,0 +1,69 @@ +#ifndef SRC_NODE_PLATFORM_H_ +#define SRC_NODE_PLATFORM_H_ + +#include +#include + +#include "libplatform/libplatform.h" +#include "node_mutex.h" +#include "uv.h" + +namespace node { + +template +class TaskQueue { + public: + TaskQueue(); + ~TaskQueue() {} + + void Push(T* task); + T* Pop(); + T* BlockingPop(); + void NotifyOfCompletion(); + void BlockingDrain(); + void Stop(); + + private: + Mutex lock_; + ConditionVariable tasks_available_; + ConditionVariable tasks_drained_; + int outstanding_tasks_; + bool stopped_; + std::queue task_queue_; +}; + +class NodePlatform : public v8::Platform { + public: + NodePlatform(int thread_pool_size, uv_loop_t* loop, + v8::TracingController* tracing_controller); + virtual ~NodePlatform() {} + + void DrainBackgroundTasks(); + void FlushForegroundTasksInternal(); + void Shutdown(); + + // v8::Platform implementation. + size_t NumberOfAvailableBackgroundThreads() override; + void CallOnBackgroundThread(v8::Task* task, + ExpectedRuntime expected_runtime) override; + void CallOnForegroundThread(v8::Isolate* isolate, v8::Task* task) override; + void CallDelayedOnForegroundThread(v8::Isolate* isolate, v8::Task* task, + double delay_in_seconds) override; + bool IdleTasksEnabled(v8::Isolate* isolate) override; + double MonotonicallyIncreasingTime() override; + v8::TracingController* GetTracingController() override; + + private: + uv_loop_t* const loop_; + uv_async_t flush_tasks_; + TaskQueue foreground_tasks_; + TaskQueue> foreground_delayed_tasks_; + TaskQueue background_tasks_; + std::vector> threads_; + + std::unique_ptr tracing_controller_; +}; + +} // namespace node + +#endif // SRC_NODE_PLATFORM_H_ diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc index 38e651ebb2a40e..1ac99bbb34bbbf 100644 --- a/src/tracing/agent.cc +++ b/src/tracing/agent.cc @@ -4,7 +4,6 @@ #include #include "env-inl.h" -#include "libplatform/libplatform.h" namespace node { namespace tracing { diff --git a/src/tracing/agent.h b/src/tracing/agent.h index cc00c53144b1fa..e781281712bbf6 100644 --- a/src/tracing/agent.h +++ b/src/tracing/agent.h @@ -1,6 +1,7 @@ #ifndef SRC_TRACING_AGENT_H_ #define SRC_TRACING_AGENT_H_ +#include "node_platform.h" #include "tracing/node_trace_buffer.h" #include "tracing/node_trace_writer.h" #include "uv.h" diff --git a/src/tracing/trace_event.cc b/src/tracing/trace_event.cc index 856b344e9d2294..f661dd5c69a07a 100644 --- a/src/tracing/trace_event.cc +++ b/src/tracing/trace_event.cc @@ -3,14 +3,14 @@ namespace node { namespace tracing { -v8::TracingController* controller_ = nullptr; +v8::TracingController* g_controller = nullptr; void TraceEventHelper::SetTracingController(v8::TracingController* controller) { - controller_ = controller; + g_controller = controller; } v8::TracingController* TraceEventHelper::GetTracingController() { - return controller_; + return g_controller; } } // namespace tracing diff --git a/src/tracing/trace_event.h b/src/tracing/trace_event.h index 24806d375fad58..61808eb94f75a1 100644 --- a/src/tracing/trace_event.h +++ b/src/tracing/trace_event.h @@ -7,6 +7,7 @@ #include +#include "node_platform.h" #include "v8-platform.h" #include "trace_event_common.h" diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index e52b1b5dfd47ca..f30823a8fdb46a 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -66,7 +66,12 @@ struct Argv { int nr_args_; }; +uv_loop_t current_loop; + class NodeTestFixture : public ::testing::Test { + public: + static uv_loop_t* CurrentLoop() { return ¤t_loop; } + protected: v8::Isolate::CreateParams params_; ArrayBufferAllocator allocator_; @@ -77,7 +82,8 @@ class NodeTestFixture : public ::testing::Test { } virtual void SetUp() { - platform_ = v8::platform::CreateDefaultPlatform(); + CHECK_EQ(0, uv_loop_init(¤t_loop)); + platform_ = new node::NodePlatform(8, ¤t_loop, nullptr); v8::V8::InitializePlatform(platform_); v8::V8::Initialize(); params_.array_buffer_allocator = &allocator_; @@ -86,13 +92,18 @@ class NodeTestFixture : public ::testing::Test { virtual void TearDown() { if (platform_ == nullptr) return; + platform_->Shutdown(); + while (uv_loop_alive(¤t_loop)) { + uv_run(¤t_loop, UV_RUN_ONCE); + } v8::V8::ShutdownPlatform(); delete platform_; platform_ = nullptr; + CHECK_EQ(0, uv_loop_close(¤t_loop)); } private: - v8::Platform* platform_ = nullptr; + node::NodePlatform* platform_ = nullptr; }; #endif // TEST_CCTEST_NODE_TEST_FIXTURE_H_ diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index aee8e795ecb6ab..4651e865a99e7d 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -31,7 +31,8 @@ class EnvironmentTest : public NodeTestFixture { const Argv& argv) { context_ = v8::Context::New(isolate); CHECK(!context_.IsEmpty()); - isolate_data_ = CreateIsolateData(isolate, uv_default_loop()); + isolate_data_ = CreateIsolateData(isolate, + NodeTestFixture::CurrentLoop()); CHECK_NE(nullptr, isolate_data_); environment_ = CreateEnvironment(isolate_data_, context_,