From 887b6a143b132b1820d161c50d439db38d09aeb1 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 27 Feb 2020 21:43:02 -0800 Subject: [PATCH] src: allow non-Node.js TracingControllers We do not need a Node.js-provided `v8::TracingController`, generally. Loosen that restriction in order to make it easier for embedders to provide their own subclass of `v8::TracingController`, or none at all. Refs: https://github.com/electron/electron/commit/9c36576dddfaecde1298ff3e089d21a6e54fe67f PR-URL: https://github.com/nodejs/node/pull/30467 Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil --- src/api/environment.cc | 10 +++++++++- src/node.h | 6 +++++- src/node_platform.cc | 13 ++++++++----- src/node_platform.h | 6 +++--- src/tracing/agent.cc | 5 +++-- src/tracing/trace_event.cc | 10 ++++++++-- src/tracing/trace_event.h | 11 +++++++---- 7 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index c44a882537016c..128bfc972a0556 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -498,6 +498,14 @@ MultiIsolatePlatform* GetMultiIsolatePlatform(IsolateData* env) { MultiIsolatePlatform* CreatePlatform( int thread_pool_size, node::tracing::TracingController* tracing_controller) { + return CreatePlatform( + thread_pool_size, + static_cast(tracing_controller)); +} + +MultiIsolatePlatform* CreatePlatform( + int thread_pool_size, + v8::TracingController* tracing_controller) { return MultiIsolatePlatform::Create(thread_pool_size, tracing_controller) .release(); } @@ -508,7 +516,7 @@ void FreePlatform(MultiIsolatePlatform* platform) { std::unique_ptr MultiIsolatePlatform::Create( int thread_pool_size, - node::tracing::TracingController* tracing_controller) { + v8::TracingController* tracing_controller) { return std::make_unique(thread_pool_size, tracing_controller); } diff --git a/src/node.h b/src/node.h index fcd5d4c1319d52..94ef91f5332121 100644 --- a/src/node.h +++ b/src/node.h @@ -329,7 +329,7 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform { static std::unique_ptr Create( int thread_pool_size, - node::tracing::TracingController* tracing_controller = nullptr); + v8::TracingController* tracing_controller = nullptr); }; enum IsolateSettingsFlags { @@ -487,9 +487,13 @@ NODE_EXTERN MultiIsolatePlatform* GetMultiIsolatePlatform(Environment* env); NODE_EXTERN MultiIsolatePlatform* GetMultiIsolatePlatform(IsolateData* env); // Legacy variants of MultiIsolatePlatform::Create(). +// TODO(addaleax): Deprecate in favour of the v8::TracingController variant. NODE_EXTERN MultiIsolatePlatform* CreatePlatform( int thread_pool_size, node::tracing::TracingController* tracing_controller); +NODE_EXTERN MultiIsolatePlatform* CreatePlatform( + int thread_pool_size, + v8::TracingController* tracing_controller); NODE_EXTERN void FreePlatform(MultiIsolatePlatform* platform); NODE_EXTERN void EmitBeforeExit(Environment* env); diff --git a/src/node_platform.cc b/src/node_platform.cc index 7628cf5339cdb3..e7c8eadf8f6842 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -13,7 +13,6 @@ using v8::Isolate; using v8::Object; using v8::Platform; using v8::Task; -using node::tracing::TracingController; namespace { @@ -325,12 +324,16 @@ void PerIsolatePlatformData::DecreaseHandleCount() { } NodePlatform::NodePlatform(int thread_pool_size, - TracingController* tracing_controller) { - if (tracing_controller) { + v8::TracingController* tracing_controller) { + if (tracing_controller != nullptr) { tracing_controller_ = tracing_controller; } else { - tracing_controller_ = new TracingController(); + tracing_controller_ = new v8::TracingController(); } + // TODO(addaleax): It's a bit icky that we use global state here, but we can't + // really do anything about it unless V8 starts exposing a way to access the + // current v8::Platform instance. + tracing::TraceEventHelper::SetTracingController(tracing_controller_); worker_thread_task_runner_ = std::make_shared(thread_pool_size); } @@ -518,7 +521,7 @@ double NodePlatform::CurrentClockTimeMillis() { return SystemClockTimeMillis(); } -TracingController* NodePlatform::GetTracingController() { +v8::TracingController* NodePlatform::GetTracingController() { CHECK_NOT_NULL(tracing_controller_); return tracing_controller_; } diff --git a/src/node_platform.h b/src/node_platform.h index 2ae95e3dae21d6..7b15f032fc423d 100644 --- a/src/node_platform.h +++ b/src/node_platform.h @@ -139,7 +139,7 @@ class WorkerThreadsTaskRunner { class NodePlatform : public MultiIsolatePlatform { public: NodePlatform(int thread_pool_size, - node::tracing::TracingController* tracing_controller); + v8::TracingController* tracing_controller); ~NodePlatform() override = default; void DrainTasks(v8::Isolate* isolate) override; @@ -153,7 +153,7 @@ class NodePlatform : public MultiIsolatePlatform { bool IdleTasksEnabled(v8::Isolate* isolate) override; double MonotonicallyIncreasingTime() override; double CurrentClockTimeMillis() override; - node::tracing::TracingController* GetTracingController() override; + v8::TracingController* GetTracingController() override; bool FlushForegroundTasks(v8::Isolate* isolate) override; void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override; @@ -178,7 +178,7 @@ class NodePlatform : public MultiIsolatePlatform { IsolatePlatformDelegate*, std::shared_ptr>; std::unordered_map per_isolate_; - node::tracing::TracingController* tracing_controller_; + v8::TracingController* tracing_controller_; std::shared_ptr worker_thread_task_runner_; }; diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc index 7d265dcb0c4c3b..7ce59674356f97 100644 --- a/src/tracing/agent.cc +++ b/src/tracing/agent.cc @@ -242,8 +242,9 @@ void TracingController::AddMetadataEvent( TRACE_EVENT_FLAG_NONE, CurrentTimestampMicroseconds(), CurrentCpuTimestampMicroseconds()); - node::tracing::TraceEventHelper::GetAgent()->AddMetadataEvent( - std::move(trace_event)); + Agent* node_agent = node::tracing::TraceEventHelper::GetAgent(); + if (node_agent != nullptr) + node_agent->AddMetadataEvent(std::move(trace_event)); } } // namespace tracing diff --git a/src/tracing/trace_event.cc b/src/tracing/trace_event.cc index 9232c34c4ca322..da562c1bf7f8ff 100644 --- a/src/tracing/trace_event.cc +++ b/src/tracing/trace_event.cc @@ -4,17 +4,23 @@ namespace node { namespace tracing { Agent* g_agent = nullptr; +v8::TracingController* g_controller = nullptr; void TraceEventHelper::SetAgent(Agent* agent) { g_agent = agent; + g_controller = agent->GetTracingController(); } Agent* TraceEventHelper::GetAgent() { return g_agent; } -TracingController* TraceEventHelper::GetTracingController() { - return g_agent->GetTracingController(); +v8::TracingController* TraceEventHelper::GetTracingController() { + return g_controller; +} + +void TraceEventHelper::SetTracingController(v8::TracingController* controller) { + g_controller = controller; } } // namespace tracing diff --git a/src/tracing/trace_event.h b/src/tracing/trace_event.h index 27408eafd832ae..be2276a9e27670 100644 --- a/src/tracing/trace_event.h +++ b/src/tracing/trace_event.h @@ -314,7 +314,9 @@ const uint64_t kNoId = 0; // Refs: https://github.com/nodejs/node/pull/28724 class NODE_EXTERN TraceEventHelper { public: - static TracingController* GetTracingController(); + static v8::TracingController* GetTracingController(); + static void SetTracingController(v8::TracingController* controller); + static Agent* GetAgent(); static void SetAgent(Agent* agent); }; @@ -505,9 +507,10 @@ static V8_INLINE void AddMetadataEventImpl( arg_convertibles[1].reset(reinterpret_cast( static_cast(arg_values[1]))); } - node::tracing::TracingController* controller = - node::tracing::TraceEventHelper::GetTracingController(); - return controller->AddMetadataEvent( + node::tracing::Agent* agent = + node::tracing::TraceEventHelper::GetAgent(); + if (agent == nullptr) return; + return agent->GetTracingController()->AddMetadataEvent( category_group_enabled, name, num_args, arg_names, arg_types, arg_values, arg_convertibles, flags); }