Skip to content

Commit

Permalink
src: remove Environment::tracing_agent_writer()
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax authored and mmarchini committed Oct 24, 2018
1 parent d568b53 commit 036fbdb
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 29 deletions.
4 changes: 0 additions & 4 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,6 @@ inline v8::Isolate* Environment::isolate() const {
return isolate_;
}

inline tracing::AgentWriterHandle* Environment::tracing_agent_writer() const {
return tracing_agent_writer_;
}

inline Environment* Environment::from_timer_handle(uv_timer_t* handle) {
return ContainerOf(&Environment::timer_handle_, handle);
}
Expand Down
16 changes: 7 additions & 9 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,9 @@ void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
}

Environment::Environment(IsolateData* isolate_data,
Local<Context> context,
tracing::AgentWriterHandle* tracing_agent_writer)
Local<Context> context)
: isolate_(context->GetIsolate()),
isolate_data_(isolate_data),
tracing_agent_writer_(tracing_agent_writer),
immediate_info_(context->GetIsolate()),
tick_info_(context->GetIsolate()),
timer_base_(uv_now(isolate_data->event_loop())),
Expand Down Expand Up @@ -183,10 +181,9 @@ Environment::Environment(IsolateData* isolate_data,

AssignToContext(context, ContextInfo(""));

if (tracing_agent_writer_ != nullptr) {
if (tracing::AgentWriterHandle* writer = GetTracingAgentWriter()) {
trace_state_observer_.reset(new TrackingTraceStateObserver(this));
v8::TracingController* tracing_controller =
tracing_agent_writer_->GetTracingController();
v8::TracingController* tracing_controller = writer->GetTracingController();
if (tracing_controller != nullptr)
tracing_controller->AddTraceStateObserver(trace_state_observer_.get());
}
Expand Down Expand Up @@ -235,9 +232,10 @@ Environment::~Environment() {
context()->SetAlignedPointerInEmbedderData(
ContextEmbedderIndex::kEnvironment, nullptr);

if (tracing_agent_writer_ != nullptr) {
v8::TracingController* tracing_controller =
tracing_agent_writer_->GetTracingController();
if (trace_state_observer_) {
tracing::AgentWriterHandle* writer = GetTracingAgentWriter();
CHECK_NOT_NULL(writer);
v8::TracingController* tracing_controller = writer->GetTracingController();
if (tracing_controller != nullptr)
tracing_controller->RemoveTraceStateObserver(trace_state_observer_.get());
}
Expand Down
5 changes: 1 addition & 4 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -593,8 +593,7 @@ class Environment {
static inline Environment* GetThreadLocalEnv();

Environment(IsolateData* isolate_data,
v8::Local<v8::Context> context,
tracing::AgentWriterHandle* tracing_agent_writer);
v8::Local<v8::Context> context);
~Environment();

void Start(const std::vector<std::string>& args,
Expand Down Expand Up @@ -630,7 +629,6 @@ class Environment {
inline bool profiler_idle_notifier_started() const;

inline v8::Isolate* isolate() const;
inline tracing::AgentWriterHandle* tracing_agent_writer() const;
inline uv_loop_t* event_loop() const;
inline uint32_t watched_providers() const;

Expand Down Expand Up @@ -921,7 +919,6 @@ class Environment {

v8::Isolate* const isolate_;
IsolateData* const isolate_data_;
tracing::AgentWriterHandle* const tracing_agent_writer_;
uv_timer_t timer_handle_;
uv_check_t immediate_check_handle_;
uv_idle_t immediate_idle_handle_;
Expand Down
5 changes: 3 additions & 2 deletions src/inspector/tracing_agent.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "tracing_agent.h"
#include "node_internals.h"

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

auto* writer = env_->tracing_agent_writer();
tracing::AgentWriterHandle* writer = GetTracingAgentWriter();
if (writer != nullptr) {
trace_writer_ = env_->tracing_agent_writer()->agent()->AddClient(
trace_writer_ = writer->agent()->AddClient(
categories_set,
std::unique_ptr<InspectorTraceWriter>(
new InspectorTraceWriter(frontend_.get())),
Expand Down
9 changes: 6 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,10 @@ static struct {
#endif // !NODE_USE_V8_PLATFORM || !HAVE_INSPECTOR
} v8_platform;

tracing::AgentWriterHandle* GetTracingAgentWriter() {
return v8_platform.GetTracingAgentWriter();
}

#ifdef __POSIX__
static const unsigned kMaxSignal = 32;
#endif
Expand Down Expand Up @@ -2763,8 +2767,7 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
// options than the global parse call.
std::vector<std::string> args(argv, argv + argc);
std::vector<std::string> exec_args(exec_argv, exec_argv + exec_argc);
Environment* env = new Environment(isolate_data, context,
v8_platform.GetTracingAgentWriter());
Environment* env = new Environment(isolate_data, context);
env->Start(args, exec_args, v8_is_profiling);
return env;
}
Expand Down Expand Up @@ -2834,7 +2837,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
HandleScope handle_scope(isolate);
Local<Context> context = NewContext(isolate);
Context::Scope context_scope(context);
Environment env(isolate_data, context, v8_platform.GetTracingAgentWriter());
Environment env(isolate_data, context);
env.Start(args, exec_args, v8_is_profiling);

const char* path = args.size() > 1 ? args[1].c_str() : nullptr;
Expand Down
2 changes: 2 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,8 @@ int ThreadPoolWork::CancelWork() {
return uv_cancel(reinterpret_cast<uv_req_t*>(&work_req_));
}

tracing::AgentWriterHandle* GetTracingAgentWriter();

static inline const char* errno_string(int errorno) {
#define ERRNO_CASE(e) case e: return #e;
switch (errorno) {
Expand Down
8 changes: 4 additions & 4 deletions src/node_trace_events.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void NodeCategorySet::New(const FunctionCallbackInfo<Value>& args) {
if (!*val) return;
categories.emplace(*val);
}
CHECK_NOT_NULL(env->tracing_agent_writer());
CHECK_NOT_NULL(GetTracingAgentWriter());
new NodeCategorySet(env, args.This(), std::move(categories));
}

Expand All @@ -68,7 +68,7 @@ void NodeCategorySet::Enable(const FunctionCallbackInfo<Value>& args) {
CHECK_NOT_NULL(category_set);
const auto& categories = category_set->GetCategories();
if (!category_set->enabled_ && !categories.empty()) {
env->tracing_agent_writer()->Enable(categories);
GetTracingAgentWriter()->Enable(categories);
category_set->enabled_ = true;
}
}
Expand All @@ -80,15 +80,15 @@ void NodeCategorySet::Disable(const FunctionCallbackInfo<Value>& args) {
CHECK_NOT_NULL(category_set);
const auto& categories = category_set->GetCategories();
if (category_set->enabled_ && !categories.empty()) {
env->tracing_agent_writer()->Disable(categories);
GetTracingAgentWriter()->Disable(categories);
category_set->enabled_ = false;
}
}

void GetEnabledCategories(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
std::string categories =
env->tracing_agent_writer()->agent()->GetEnabledCategories();
GetTracingAgentWriter()->agent()->GetEnabledCategories();
if (!categories.empty()) {
args.GetReturnValue().Set(
String::NewFromUtf8(env->isolate(),
Expand Down
4 changes: 1 addition & 3 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ Worker::Worker(Environment* env, Local<Object> wrap, const std::string& url)
Context::Scope context_scope(context);

// TODO(addaleax): Use CreateEnvironment(), or generally another public API.
env_.reset(new Environment(isolate_data_.get(),
context,
nullptr));
env_.reset(new Environment(isolate_data_.get(), context));
CHECK_NE(env_, nullptr);
env_->set_abort_on_uncaught_exception(false);
env_->set_worker_context(this);
Expand Down

0 comments on commit 036fbdb

Please sign in to comment.