Skip to content

Commit

Permalink
src: refactor default trace writer out of agent
Browse files Browse the repository at this point in the history
The agent code is supposed to manage multiple writers/clients.
Adding the default writer into the mix breaks that encapsulation.
Instead, manage default options through a separate "virtual"
default client handle, and keep the file writer management
all to the actual users.

PR-URL: #21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
  • Loading branch information
addaleax authored and targos committed Aug 1, 2018
1 parent daafe6c commit c101b39
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 87 deletions.
4 changes: 2 additions & 2 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ inline v8::Isolate* Environment::isolate() const {
return isolate_;
}

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

inline Environment* Environment::from_immediate_check_handle(
Expand Down
4 changes: 2 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ void InitThreadLocalOnce() {

Environment::Environment(IsolateData* isolate_data,
Local<Context> context,
tracing::Agent* tracing_agent)
tracing::AgentWriterHandle* tracing_agent_writer)
: isolate_(context->GetIsolate()),
isolate_data_(isolate_data),
tracing_agent_(tracing_agent),
tracing_agent_writer_(tracing_agent_writer),
immediate_info_(context->GetIsolate()),
tick_info_(context->GetIsolate()),
timer_base_(uv_now(isolate_data->event_loop())),
Expand Down
8 changes: 4 additions & 4 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class performance_state;
}

namespace tracing {
class Agent;
class AgentWriterHandle;
}

namespace worker {
Expand Down Expand Up @@ -590,7 +590,7 @@ class Environment {

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

void Start(int argc,
Expand Down Expand Up @@ -628,7 +628,7 @@ class Environment {
inline bool profiler_idle_notifier_started() const;

inline v8::Isolate* isolate() const;
inline tracing::Agent* tracing_agent() 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 @@ -877,7 +877,7 @@ class Environment {

v8::Isolate* const isolate_;
IsolateData* const isolate_data_;
tracing::Agent* const tracing_agent_;
tracing::AgentWriterHandle* const tracing_agent_writer_;
uv_check_t immediate_check_handle_;
uv_idle_t immediate_idle_handle_;
uv_prepare_t idle_prepare_handle_;
Expand Down
5 changes: 3 additions & 2 deletions src/inspector/tracing_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ DispatchResponse TracingAgent::start(
if (categories_set.empty())
return DispatchResponse::Error("At least one category should be enabled");

trace_writer_ = env_->tracing_agent()->AddClient(
trace_writer_ = env_->tracing_agent_writer()->agent()->AddClient(
categories_set,
std::unique_ptr<InspectorTraceWriter>(
new InspectorTraceWriter(frontend_.get())));
new InspectorTraceWriter(frontend_.get())),
tracing::Agent::kIgnoreDefaultCategories);
return DispatchResponse::OK();
}

Expand Down
26 changes: 18 additions & 8 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include "req_wrap-inl.h"
#include "string_bytes.h"
#include "tracing/agent.h"
#include "tracing/node_trace_writer.h"
#include "util.h"
#include "uv.h"
#if NODE_USE_V8_PLATFORM
Expand Down Expand Up @@ -427,17 +428,24 @@ static struct {
#endif // HAVE_INSPECTOR

void StartTracingAgent() {
tracing_file_writer_ = tracing_agent_->AddClient(
trace_enabled_categories,
new tracing::NodeTraceWriter(trace_file_pattern));
if (trace_enabled_categories.empty()) {
tracing_file_writer_ = tracing_agent_->DefaultHandle();
} else {
tracing_file_writer_ = tracing_agent_->AddClient(
ParseCommaSeparatedSet(trace_enabled_categories),
std::unique_ptr<tracing::AsyncTraceWriter>(
new tracing::NodeTraceWriter(trace_file_pattern,
tracing_agent_->loop())),
tracing::Agent::kUseDefaultCategories);
}
}

void StopTracingAgent() {
tracing_file_writer_.reset();
}

tracing::Agent* GetTracingAgent() const {
return tracing_agent_.get();
tracing::AgentWriterHandle* GetTracingAgentWriter() {
return &tracing_file_writer_;
}

NodePlatform* Platform() {
Expand Down Expand Up @@ -466,7 +474,9 @@ static struct {
}
void StopTracingAgent() {}

tracing::Agent* GetTracingAgent() const { return nullptr; }
tracing::AgentWriterHandle* GetTracingAgentWriter() {
return nullptr;
}

NodePlatform* Platform() {
return nullptr;
Expand Down Expand Up @@ -3593,7 +3603,7 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
HandleScope handle_scope(isolate);
Context::Scope context_scope(context);
auto env = new Environment(isolate_data, context,
v8_platform.GetTracingAgent());
v8_platform.GetTracingAgentWriter());
env->Start(argc, argv, exec_argc, exec_argv, v8_is_profiling);
return env;
}
Expand Down Expand Up @@ -3652,7 +3662,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.GetTracingAgent());
Environment env(isolate_data, context, v8_platform.GetTracingAgentWriter());
env.Start(argc, argv, exec_argc, exec_argv, v8_is_profiling);

const char* path = argc > 1 ? argv[1] : nullptr;
Expand Down
9 changes: 5 additions & 4 deletions src/node_trace_events.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void NodeCategorySet::New(const FunctionCallbackInfo<Value>& args) {
if (!*val) return;
categories.emplace(*val);
}
CHECK_NOT_NULL(env->tracing_agent());
CHECK_NOT_NULL(env->tracing_agent_writer());
new NodeCategorySet(env, args.This(), std::move(categories));
}

Expand All @@ -69,7 +69,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()->Enable(categories);
env->tracing_agent_writer()->Enable(categories);
category_set->enabled_ = true;
}
}
Expand All @@ -81,14 +81,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()->Disable(categories);
env->tracing_agent_writer()->Disable(categories);
category_set->enabled_ = false;
}
}

void GetEnabledCategories(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
std::string categories = env->tracing_agent()->GetEnabledCategories();
std::string categories =
env->tracing_agent_writer()->agent()->GetEnabledCategories();
if (!categories.empty()) {
args.GetReturnValue().Set(
String::NewFromUtf8(env->isolate(),
Expand Down
97 changes: 44 additions & 53 deletions src/tracing/agent.cc
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
#include "tracing/agent.h"

#include <sstream>
#include <string>
#include "tracing/node_trace_buffer.h"
#include "tracing/node_trace_writer.h"

namespace node {
namespace tracing {

namespace {

class ScopedSuspendTracing {
class Agent::ScopedSuspendTracing {
public:
ScopedSuspendTracing(TracingController* controller, Agent* agent)
: controller_(controller), agent_(agent) {
controller->StopTracing();
ScopedSuspendTracing(TracingController* controller, Agent* agent,
bool do_suspend = true)
: controller_(controller), agent_(do_suspend ? agent : nullptr) {
if (do_suspend) {
CHECK(agent_->started_);
controller->StopTracing();
}
}

~ScopedSuspendTracing() {
if (agent_ == nullptr) return;
TraceConfig* config = agent_->CreateTraceConfig();
if (config != nullptr) {
controller_->StartTracing(config);
Expand All @@ -29,8 +30,10 @@ class ScopedSuspendTracing {
Agent* agent_;
};

namespace {

std::set<std::string> flatten(
const std::unordered_map<int, std::set<std::string>>& map) {
const std::unordered_map<int, std::multiset<std::string>>& map) {
std::set<std::string> result;
for (const auto& id_value : map)
result.insert(id_value.second.begin(), id_value.second.end());
Expand All @@ -43,18 +46,17 @@ using v8::platform::tracing::TraceConfig;
using v8::platform::tracing::TraceWriter;
using std::string;

Agent::Agent(const std::string& log_file_pattern)
: log_file_pattern_(log_file_pattern) {
Agent::Agent() {
tracing_controller_ = new TracingController();
tracing_controller_->Initialize(nullptr);

CHECK_EQ(uv_loop_init(&tracing_loop_), 0);
}

void Agent::Start() {
if (started_)
return;

CHECK_EQ(uv_loop_init(&tracing_loop_), 0);

NodeTraceBuffer* trace_buffer_ = new NodeTraceBuffer(
NodeTraceBuffer::kBufferChunks, this, &tracing_loop_);
tracing_controller_->Initialize(trace_buffer_);
Expand All @@ -71,18 +73,30 @@ void Agent::Start() {

AgentWriterHandle Agent::AddClient(
const std::set<std::string>& categories,
std::unique_ptr<AsyncTraceWriter> writer) {
std::unique_ptr<AsyncTraceWriter> writer,
enum UseDefaultCategoryMode mode) {
Start();

const std::set<std::string>* use_categories = &categories;

std::set<std::string> categories_with_default;
if (mode == kUseDefaultCategories) {
categories_with_default.insert(categories.begin(), categories.end());
categories_with_default.insert(categories_[kDefaultHandleId].begin(),
categories_[kDefaultHandleId].end());
use_categories = &categories_with_default;
}

ScopedSuspendTracing suspend(tracing_controller_, this);
int id = next_writer_id_++;
writers_[id] = std::move(writer);
categories_[id] = categories;
categories_[id] = { use_categories->begin(), use_categories->end() };

return AgentWriterHandle(this, id);
}

void Agent::Stop() {
file_writer_.reset();
AgentWriterHandle Agent::DefaultHandle() {
return AgentWriterHandle(this, kDefaultHandleId);
}

void Agent::StopTracing() {
Expand All @@ -99,54 +113,30 @@ void Agent::StopTracing() {
}

void Agent::Disconnect(int client) {
if (client == kDefaultHandleId) return;
ScopedSuspendTracing suspend(tracing_controller_, this);
writers_.erase(client);
categories_.erase(client);
}

void Agent::Enable(const std::string& categories) {
void Agent::Enable(int id, const std::set<std::string>& categories) {
if (categories.empty())
return;
std::set<std::string> categories_set;
std::istringstream category_list(categories);
while (category_list.good()) {
std::string category;
getline(category_list, category, ',');
categories_set.emplace(std::move(category));
}
Enable(categories_set);
}

void Agent::Enable(const std::set<std::string>& categories) {
if (categories.empty())
return;

file_writer_categories_.insert(categories.begin(), categories.end());
std::set<std::string> full_list(file_writer_categories_.begin(),
file_writer_categories_.end());
if (file_writer_.empty()) {
// Ensure background thread is running
Start();
std::unique_ptr<NodeTraceWriter> writer(
new NodeTraceWriter(log_file_pattern_, &tracing_loop_));
file_writer_ = AddClient(full_list, std::move(writer));
} else {
ScopedSuspendTracing suspend(tracing_controller_, this);
categories_[file_writer_.id_] = full_list;
}
ScopedSuspendTracing suspend(tracing_controller_, this,
id != kDefaultHandleId);
categories_[id].insert(categories.begin(), categories.end());
}

void Agent::Disable(const std::set<std::string>& categories) {
void Agent::Disable(int id, const std::set<std::string>& categories) {
ScopedSuspendTracing suspend(tracing_controller_, this,
id != kDefaultHandleId);
std::multiset<std::string>& writer_categories = categories_[id];
for (const std::string& category : categories) {
auto it = file_writer_categories_.find(category);
if (it != file_writer_categories_.end())
file_writer_categories_.erase(it);
auto it = writer_categories.find(category);
if (it != writer_categories.end())
writer_categories.erase(it);
}
if (file_writer_.empty())
return;
ScopedSuspendTracing suspend(tracing_controller_, this);
categories_[file_writer_.id_] = { file_writer_categories_.begin(),
file_writer_categories_.end() };
}

TraceConfig* Agent::CreateTraceConfig() const {
Expand Down Expand Up @@ -178,5 +168,6 @@ void Agent::Flush(bool blocking) {
for (const auto& id_writer : writers_)
id_writer.second->Flush(blocking);
}

} // namespace tracing
} // namespace node
Loading

0 comments on commit c101b39

Please sign in to comment.