Skip to content

Commit 5061610

Browse files
addaleaxrvagg
authored andcommitted
src: remove Environment::tracing_agent_writer()
As per the conversation in #22513, this is essentially global, and adding this on the Environment is generally just confusing. Refs: #22513 Fixes: #22767 PR-URL: #23781 Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Matheus Marchini <mat@mmarchini.me>
1 parent c177799 commit 5061610

8 files changed

+24
-29
lines changed

src/env-inl.h

-4
Original file line numberDiff line numberDiff line change
@@ -334,10 +334,6 @@ inline v8::Isolate* Environment::isolate() const {
334334
return isolate_;
335335
}
336336

337-
inline tracing::AgentWriterHandle* Environment::tracing_agent_writer() const {
338-
return tracing_agent_writer_;
339-
}
340-
341337
inline Environment* Environment::from_timer_handle(uv_timer_t* handle) {
342338
return ContainerOf(&Environment::timer_handle_, handle);
343339
}

src/env.cc

+7-9
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,9 @@ void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
143143
}
144144

145145
Environment::Environment(IsolateData* isolate_data,
146-
Local<Context> context,
147-
tracing::AgentWriterHandle* tracing_agent_writer)
146+
Local<Context> context)
148147
: isolate_(context->GetIsolate()),
149148
isolate_data_(isolate_data),
150-
tracing_agent_writer_(tracing_agent_writer),
151149
immediate_info_(context->GetIsolate()),
152150
tick_info_(context->GetIsolate()),
153151
timer_base_(uv_now(isolate_data->event_loop())),
@@ -183,10 +181,9 @@ Environment::Environment(IsolateData* isolate_data,
183181

184182
AssignToContext(context, ContextInfo(""));
185183

186-
if (tracing_agent_writer_ != nullptr) {
184+
if (tracing::AgentWriterHandle* writer = GetTracingAgentWriter()) {
187185
trace_state_observer_.reset(new TrackingTraceStateObserver(this));
188-
v8::TracingController* tracing_controller =
189-
tracing_agent_writer_->GetTracingController();
186+
v8::TracingController* tracing_controller = writer->GetTracingController();
190187
if (tracing_controller != nullptr)
191188
tracing_controller->AddTraceStateObserver(trace_state_observer_.get());
192189
}
@@ -235,9 +232,10 @@ Environment::~Environment() {
235232
context()->SetAlignedPointerInEmbedderData(
236233
ContextEmbedderIndex::kEnvironment, nullptr);
237234

238-
if (tracing_agent_writer_ != nullptr) {
239-
v8::TracingController* tracing_controller =
240-
tracing_agent_writer_->GetTracingController();
235+
if (trace_state_observer_) {
236+
tracing::AgentWriterHandle* writer = GetTracingAgentWriter();
237+
CHECK_NOT_NULL(writer);
238+
v8::TracingController* tracing_controller = writer->GetTracingController();
241239
if (tracing_controller != nullptr)
242240
tracing_controller->RemoveTraceStateObserver(trace_state_observer_.get());
243241
}

src/env.h

+1-4
Original file line numberDiff line numberDiff line change
@@ -595,8 +595,7 @@ class Environment {
595595
static inline Environment* GetThreadLocalEnv();
596596

597597
Environment(IsolateData* isolate_data,
598-
v8::Local<v8::Context> context,
599-
tracing::AgentWriterHandle* tracing_agent_writer);
598+
v8::Local<v8::Context> context);
600599
~Environment();
601600

602601
void Start(const std::vector<std::string>& args,
@@ -632,7 +631,6 @@ class Environment {
632631
inline bool profiler_idle_notifier_started() const;
633632

634633
inline v8::Isolate* isolate() const;
635-
inline tracing::AgentWriterHandle* tracing_agent_writer() const;
636634
inline uv_loop_t* event_loop() const;
637635
inline uint32_t watched_providers() const;
638636

@@ -923,7 +921,6 @@ class Environment {
923921

924922
v8::Isolate* const isolate_;
925923
IsolateData* const isolate_data_;
926-
tracing::AgentWriterHandle* const tracing_agent_writer_;
927924
uv_timer_t timer_handle_;
928925
uv_check_t immediate_check_handle_;
929926
uv_idle_t immediate_idle_handle_;

src/inspector/tracing_agent.cc

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "tracing_agent.h"
2+
#include "node_internals.h"
23

34
#include "env-inl.h"
45
#include "v8.h"
@@ -74,9 +75,9 @@ DispatchResponse TracingAgent::start(
7475
if (categories_set.empty())
7576
return DispatchResponse::Error("At least one category should be enabled");
7677

77-
auto* writer = env_->tracing_agent_writer();
78+
tracing::AgentWriterHandle* writer = GetTracingAgentWriter();
7879
if (writer != nullptr) {
79-
trace_writer_ = env_->tracing_agent_writer()->agent()->AddClient(
80+
trace_writer_ = writer->agent()->AddClient(
8081
categories_set,
8182
std::unique_ptr<InspectorTraceWriter>(
8283
new InspectorTraceWriter(frontend_.get())),

src/node.cc

+6-3
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,10 @@ static struct {
399399
#endif // !NODE_USE_V8_PLATFORM || !HAVE_INSPECTOR
400400
} v8_platform;
401401

402+
tracing::AgentWriterHandle* GetTracingAgentWriter() {
403+
return v8_platform.GetTracingAgentWriter();
404+
}
405+
402406
#ifdef __POSIX__
403407
static const unsigned kMaxSignal = 32;
404408
#endif
@@ -2442,8 +2446,7 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
24422446
// options than the global parse call.
24432447
std::vector<std::string> args(argv, argv + argc);
24442448
std::vector<std::string> exec_args(exec_argv, exec_argv + exec_argc);
2445-
Environment* env = new Environment(isolate_data, context,
2446-
v8_platform.GetTracingAgentWriter());
2449+
Environment* env = new Environment(isolate_data, context);
24472450
env->Start(args, exec_args, v8_is_profiling);
24482451
return env;
24492452
}
@@ -2515,7 +2518,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
25152518
HandleScope handle_scope(isolate);
25162519
Local<Context> context = NewContext(isolate);
25172520
Context::Scope context_scope(context);
2518-
Environment env(isolate_data, context, v8_platform.GetTracingAgentWriter());
2521+
Environment env(isolate_data, context);
25192522
env.Start(args, exec_args, v8_is_profiling);
25202523

25212524
const char* path = args.size() > 1 ? args[1].c_str() : nullptr;

src/node_internals.h

+2
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,8 @@ int ThreadPoolWork::CancelWork() {
501501
return uv_cancel(reinterpret_cast<uv_req_t*>(&work_req_));
502502
}
503503

504+
tracing::AgentWriterHandle* GetTracingAgentWriter();
505+
504506
static inline const char* errno_string(int errorno) {
505507
#define ERRNO_CASE(e) case e: return #e;
506508
switch (errorno) {

src/node_trace_events.cc

+4-4
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ void NodeCategorySet::New(const FunctionCallbackInfo<Value>& args) {
5757
if (!*val) return;
5858
categories.emplace(*val);
5959
}
60-
CHECK_NOT_NULL(env->tracing_agent_writer());
60+
CHECK_NOT_NULL(GetTracingAgentWriter());
6161
new NodeCategorySet(env, args.This(), std::move(categories));
6262
}
6363

@@ -68,7 +68,7 @@ void NodeCategorySet::Enable(const FunctionCallbackInfo<Value>& args) {
6868
CHECK_NOT_NULL(category_set);
6969
const auto& categories = category_set->GetCategories();
7070
if (!category_set->enabled_ && !categories.empty()) {
71-
env->tracing_agent_writer()->Enable(categories);
71+
GetTracingAgentWriter()->Enable(categories);
7272
category_set->enabled_ = true;
7373
}
7474
}
@@ -80,15 +80,15 @@ void NodeCategorySet::Disable(const FunctionCallbackInfo<Value>& args) {
8080
CHECK_NOT_NULL(category_set);
8181
const auto& categories = category_set->GetCategories();
8282
if (category_set->enabled_ && !categories.empty()) {
83-
env->tracing_agent_writer()->Disable(categories);
83+
GetTracingAgentWriter()->Disable(categories);
8484
category_set->enabled_ = false;
8585
}
8686
}
8787

8888
void GetEnabledCategories(const FunctionCallbackInfo<Value>& args) {
8989
Environment* env = Environment::GetCurrent(args);
9090
std::string categories =
91-
env->tracing_agent_writer()->agent()->GetEnabledCategories();
91+
GetTracingAgentWriter()->agent()->GetEnabledCategories();
9292
if (!categories.empty()) {
9393
args.GetReturnValue().Set(
9494
String::NewFromUtf8(env->isolate(),

src/node_worker.cc

+1-3
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,7 @@ Worker::Worker(Environment* env, Local<Object> wrap, const std::string& url)
116116
Context::Scope context_scope(context);
117117

118118
// TODO(addaleax): Use CreateEnvironment(), or generally another public API.
119-
env_.reset(new Environment(isolate_data_.get(),
120-
context,
121-
nullptr));
119+
env_.reset(new Environment(isolate_data_.get(), context));
122120
CHECK_NE(env_, nullptr);
123121
env_->set_abort_on_uncaught_exception(false);
124122
env_->set_worker_context(this);

0 commit comments

Comments
 (0)