Skip to content

Commit

Permalink
src: allow non-Node.js TracingControllers
Browse files Browse the repository at this point in the history
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: #30467
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
  • Loading branch information
addaleax committed Mar 21, 2020
1 parent 7e0264d commit 887b6a1
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 18 deletions.
10 changes: 9 additions & 1 deletion src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<v8::TracingController*>(tracing_controller));
}

MultiIsolatePlatform* CreatePlatform(
int thread_pool_size,
v8::TracingController* tracing_controller) {
return MultiIsolatePlatform::Create(thread_pool_size, tracing_controller)
.release();
}
Expand All @@ -508,7 +516,7 @@ void FreePlatform(MultiIsolatePlatform* platform) {

std::unique_ptr<MultiIsolatePlatform> MultiIsolatePlatform::Create(
int thread_pool_size,
node::tracing::TracingController* tracing_controller) {
v8::TracingController* tracing_controller) {
return std::make_unique<NodePlatform>(thread_pool_size, tracing_controller);
}

Expand Down
6 changes: 5 additions & 1 deletion src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {

static std::unique_ptr<MultiIsolatePlatform> Create(
int thread_pool_size,
node::tracing::TracingController* tracing_controller = nullptr);
v8::TracingController* tracing_controller = nullptr);
};

enum IsolateSettingsFlags {
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 8 additions & 5 deletions src/node_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ using v8::Isolate;
using v8::Object;
using v8::Platform;
using v8::Task;
using node::tracing::TracingController;

namespace {

Expand Down Expand Up @@ -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<WorkerThreadsTaskRunner>(thread_pool_size);
}
Expand Down Expand Up @@ -518,7 +521,7 @@ double NodePlatform::CurrentClockTimeMillis() {
return SystemClockTimeMillis();
}

TracingController* NodePlatform::GetTracingController() {
v8::TracingController* NodePlatform::GetTracingController() {
CHECK_NOT_NULL(tracing_controller_);
return tracing_controller_;
}
Expand Down
6 changes: 3 additions & 3 deletions src/node_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -178,7 +178,7 @@ class NodePlatform : public MultiIsolatePlatform {
IsolatePlatformDelegate*, std::shared_ptr<PerIsolatePlatformData>>;
std::unordered_map<v8::Isolate*, DelegatePair> per_isolate_;

node::tracing::TracingController* tracing_controller_;
v8::TracingController* tracing_controller_;
std::shared_ptr<WorkerThreadsTaskRunner> worker_thread_task_runner_;
};

Expand Down
5 changes: 3 additions & 2 deletions src/tracing/agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions src/tracing/trace_event.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions src/tracing/trace_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down Expand Up @@ -505,9 +507,10 @@ static V8_INLINE void AddMetadataEventImpl(
arg_convertibles[1].reset(reinterpret_cast<v8::ConvertableToTraceFormat*>(
static_cast<intptr_t>(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);
}
Expand Down

0 comments on commit 887b6a1

Please sign in to comment.