Skip to content

Commit ededfc9

Browse files
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
1 parent 23b9ace commit ededfc9

File tree

7 files changed

+43
-18
lines changed

7 files changed

+43
-18
lines changed

src/api/environment.cc

+9-1
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,14 @@ MultiIsolatePlatform* GetMultiIsolatePlatform(IsolateData* env) {
496496
MultiIsolatePlatform* CreatePlatform(
497497
int thread_pool_size,
498498
node::tracing::TracingController* tracing_controller) {
499+
return CreatePlatform(
500+
thread_pool_size,
501+
static_cast<v8::TracingController*>(tracing_controller));
502+
}
503+
504+
MultiIsolatePlatform* CreatePlatform(
505+
int thread_pool_size,
506+
v8::TracingController* tracing_controller) {
499507
return MultiIsolatePlatform::Create(thread_pool_size, tracing_controller)
500508
.release();
501509
}
@@ -506,7 +514,7 @@ void FreePlatform(MultiIsolatePlatform* platform) {
506514

507515
std::unique_ptr<MultiIsolatePlatform> MultiIsolatePlatform::Create(
508516
int thread_pool_size,
509-
node::tracing::TracingController* tracing_controller) {
517+
v8::TracingController* tracing_controller) {
510518
return std::make_unique<NodePlatform>(thread_pool_size, tracing_controller);
511519
}
512520

src/node.h

+5-1
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

+8-5
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
}
@@ -519,7 +522,7 @@ double NodePlatform::CurrentClockTimeMillis() {
519522
return SystemClockTimeMillis();
520523
}
521524

522-
TracingController* NodePlatform::GetTracingController() {
525+
v8::TracingController* NodePlatform::GetTracingController() {
523526
CHECK_NOT_NULL(tracing_controller_);
524527
return tracing_controller_;
525528
}

src/node_platform.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class WorkerThreadsTaskRunner {
138138
class NodePlatform : public MultiIsolatePlatform {
139139
public:
140140
NodePlatform(int thread_pool_size,
141-
node::tracing::TracingController* tracing_controller);
141+
v8::TracingController* tracing_controller);
142142
~NodePlatform() override = default;
143143

144144
void DrainTasks(v8::Isolate* isolate) override;
@@ -160,7 +160,7 @@ class NodePlatform : public MultiIsolatePlatform {
160160
bool IdleTasksEnabled(v8::Isolate* isolate) override;
161161
double MonotonicallyIncreasingTime() override;
162162
double CurrentClockTimeMillis() override;
163-
node::tracing::TracingController* GetTracingController() override;
163+
v8::TracingController* GetTracingController() override;
164164
bool FlushForegroundTasks(v8::Isolate* isolate) override;
165165

166166
void RegisterIsolate(v8::Isolate* isolate, uv_loop_t* loop) override;
@@ -185,7 +185,7 @@ class NodePlatform : public MultiIsolatePlatform {
185185
IsolatePlatformDelegate*, std::shared_ptr<PerIsolatePlatformData>>;
186186
std::unordered_map<v8::Isolate*, DelegatePair> per_isolate_;
187187

188-
node::tracing::TracingController* tracing_controller_;
188+
v8::TracingController* tracing_controller_;
189189
std::shared_ptr<WorkerThreadsTaskRunner> worker_thread_task_runner_;
190190
};
191191

src/tracing/agent.cc

+3-2
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

+8-2
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

+7-4
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)