Skip to content

Commit 887b6a1

Browse files
addaleaxaddaleax
committed
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: electron/electron@9c36576 PR-URL: nodejs#30467 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
1 parent 7e0264d commit 887b6a1

File tree

7 files changed

+43
-18
lines changed

7 files changed

+43
-18
lines changed

‎src/api/environment.cc‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,14 @@ MultiIsolatePlatform* GetMultiIsolatePlatform(IsolateData* env) {
498498
MultiIsolatePlatform* CreatePlatform(
499499
int thread_pool_size,
500500
node::tracing::TracingController* tracing_controller) {
501+
return CreatePlatform(
502+
thread_pool_size,
503+
static_cast<v8::TracingController*>(tracing_controller));
504+
}
505+
506+
MultiIsolatePlatform* CreatePlatform(
507+
int thread_pool_size,
508+
v8::TracingController* tracing_controller) {
501509
return MultiIsolatePlatform::Create(thread_pool_size, tracing_controller)
502510
.release();
503511
}
@@ -508,7 +516,7 @@ void FreePlatform(MultiIsolatePlatform* platform) {
508516

509517
std::unique_ptr<MultiIsolatePlatform> MultiIsolatePlatform::Create(
510518
int thread_pool_size,
511-
node::tracing::TracingController* tracing_controller) {
519+
v8::TracingController* tracing_controller) {
512520
return std::make_unique<NodePlatform>(thread_pool_size, tracing_controller);
513521
}
514522

‎src/node.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
329329

330330
static std::unique_ptr<MultiIsolatePlatform> Create(
331331
int thread_pool_size,
332-
node::tracing::TracingController* tracing_controller = nullptr);
332+
v8::TracingController* tracing_controller = nullptr);
333333
};
334334

335335
enum IsolateSettingsFlags {
@@ -487,9 +487,13 @@ NODE_EXTERN MultiIsolatePlatform* GetMultiIsolatePlatform(Environment* env);
487487
NODE_EXTERN MultiIsolatePlatform* GetMultiIsolatePlatform(IsolateData* env);
488488

489489
// Legacy variants of MultiIsolatePlatform::Create().
490+
// TODO(addaleax): Deprecate in favour of the v8::TracingController variant.
490491
NODE_EXTERN MultiIsolatePlatform* CreatePlatform(
491492
int thread_pool_size,
492493
node::tracing::TracingController* tracing_controller);
494+
NODE_EXTERN MultiIsolatePlatform* CreatePlatform(
495+
int thread_pool_size,
496+
v8::TracingController* tracing_controller);
493497
NODE_EXTERN void FreePlatform(MultiIsolatePlatform* platform);
494498

495499
NODE_EXTERN void EmitBeforeExit(Environment* env);

‎src/node_platform.cc‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ using v8::Isolate;
1313
using v8::Object;
1414
using v8::Platform;
1515
using v8::Task;
16-
using node::tracing::TracingController;
1716

1817
namespace {
1918

@@ -325,12 +324,16 @@ void PerIsolatePlatformData::DecreaseHandleCount() {
325324
}
326325

327326
NodePlatform::NodePlatform(int thread_pool_size,
328-
TracingController* tracing_controller) {
329-
if (tracing_controller) {
327+
v8::TracingController* tracing_controller) {
328+
if (tracing_controller != nullptr) {
330329
tracing_controller_ = tracing_controller;
331330
} else {
332-
tracing_controller_ = new TracingController();
331+
tracing_controller_ = new v8::TracingController();
333332
}
333+
// TODO(addaleax): It's a bit icky that we use global state here, but we can't
334+
// really do anything about it unless V8 starts exposing a way to access the
335+
// current v8::Platform instance.
336+
tracing::TraceEventHelper::SetTracingController(tracing_controller_);
334337
worker_thread_task_runner_ =
335338
std::make_shared<WorkerThreadsTaskRunner>(thread_pool_size);
336339
}
@@ -518,7 +521,7 @@ double NodePlatform::CurrentClockTimeMillis() {
518521
return SystemClockTimeMillis();
519522
}
520523

521-
TracingController* NodePlatform::GetTracingController() {
524+
v8::TracingController* NodePlatform::GetTracingController() {
522525
CHECK_NOT_NULL(tracing_controller_);
523526
return tracing_controller_;
524527
}

‎src/node_platform.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class WorkerThreadsTaskRunner {
139139
class NodePlatform : public MultiIsolatePlatform {
140140
public:
141141
NodePlatform(int thread_pool_size,
142-
node::tracing::TracingController* tracing_controller);
142+
v8::TracingController* tracing_controller);
143143
~NodePlatform() override = default;
144144

145145
void DrainTasks(v8::Isolate* isolate) override;
@@ -153,7 +153,7 @@ class NodePlatform : public MultiIsolatePlatform {
153153
bool IdleTasksEnabled(v8::Isolate* isolate) override;
154154
double MonotonicallyIncreasingTime() override;
155155
double CurrentClockTimeMillis() override;
156-
node::tracing::TracingController* GetTracingController() override;
156+
v8::TracingController* GetTracingController() override;
157157
bool FlushForegroundTasks(v8::Isolate* isolate) override;
158158

159159
void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override;
@@ -178,7 +178,7 @@ class NodePlatform : public MultiIsolatePlatform {
178178
IsolatePlatformDelegate*, std::shared_ptr<PerIsolatePlatformData>>;
179179
std::unordered_map<v8::Isolate*, DelegatePair> per_isolate_;
180180

181-
node::tracing::TracingController* tracing_controller_;
181+
v8::TracingController* tracing_controller_;
182182
std::shared_ptr<WorkerThreadsTaskRunner> worker_thread_task_runner_;
183183
};
184184

‎src/tracing/agent.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,9 @@ void TracingController::AddMetadataEvent(
242242
TRACE_EVENT_FLAG_NONE,
243243
CurrentTimestampMicroseconds(),
244244
CurrentCpuTimestampMicroseconds());
245-
node::tracing::TraceEventHelper::GetAgent()->AddMetadataEvent(
246-
std::move(trace_event));
245+
Agent* node_agent = node::tracing::TraceEventHelper::GetAgent();
246+
if (node_agent != nullptr)
247+
node_agent->AddMetadataEvent(std::move(trace_event));
247248
}
248249

249250
} // namespace tracing

‎src/tracing/trace_event.cc‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,23 @@ namespace node {
44
namespace tracing {
55

66
Agent* g_agent = nullptr;
7+
v8::TracingController* g_controller = nullptr;
78

89
void TraceEventHelper::SetAgent(Agent* agent) {
910
g_agent = agent;
11+
g_controller = agent->GetTracingController();
1012
}
1113

1214
Agent* TraceEventHelper::GetAgent() {
1315
return g_agent;
1416
}
1517

16-
TracingController* TraceEventHelper::GetTracingController() {
17-
return g_agent->GetTracingController();
18+
v8::TracingController* TraceEventHelper::GetTracingController() {
19+
return g_controller;
20+
}
21+
22+
void TraceEventHelper::SetTracingController(v8::TracingController* controller) {
23+
g_controller = controller;
1824
}
1925

2026
} // namespace tracing

‎src/tracing/trace_event.h‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,9 @@ const uint64_t kNoId = 0;
314314
// Refs: https://github.com/nodejs/node/pull/28724
315315
class NODE_EXTERN TraceEventHelper {
316316
public:
317-
static TracingController* GetTracingController();
317+
static v8::TracingController* GetTracingController();
318+
static void SetTracingController(v8::TracingController* controller);
319+
318320
static Agent* GetAgent();
319321
static void SetAgent(Agent* agent);
320322
};
@@ -505,9 +507,10 @@ static V8_INLINE void AddMetadataEventImpl(
505507
arg_convertibles[1].reset(reinterpret_cast<v8::ConvertableToTraceFormat*>(
506508
static_cast<intptr_t>(arg_values[1])));
507509
}
508-
node::tracing::TracingController* controller =
509-
node::tracing::TraceEventHelper::GetTracingController();
510-
return controller->AddMetadataEvent(
510+
node::tracing::Agent* agent =
511+
node::tracing::TraceEventHelper::GetAgent();
512+
if (agent == nullptr) return;
513+
return agent->GetTracingController()->AddMetadataEvent(
511514
category_group_enabled, name, num_args, arg_names, arg_types, arg_values,
512515
arg_convertibles, flags);
513516
}

0 commit comments

Comments
 (0)