src: refactor tracing code

src/env-inl.h

@@ -32,7 +32,6 @@
 #include "v8.h"
 #include "node_perf_common.h"
 #include "node_context_data.h"
-#include "tracing/agent.h"
 #include "node_worker.h"
 
 #include <stddef.h>
@@ -326,8 +325,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_timer_handle(uv_timer_t* handle) {

src/env.cc

@@ -105,10 +105,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())),

src/env.h

@@ -35,7 +35,6 @@
 #include "v8.h"
 #include "node.h"
 #include "node_http2_state.h"
-#include "tracing/agent.h"
 
 #include <list>
 #include <stdint.h>
@@ -55,6 +54,10 @@ namespace performance {
 class performance_state;
 }
 
+namespace tracing {
+class AgentWriterHandle;
+}
+
 namespace worker {
 class Worker;
 }
@@ -584,7 +587,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,
@@ -622,7 +625,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;
 
@@ -876,7 +879,7 @@ class Environment {
 
   v8::Isolate* const isolate_;
   IsolateData* const isolate_data_;
-  tracing::Agent* const tracing_agent_;
+  tracing::AgentWriterHandle* const tracing_agent_writer_;
   uv_timer_t timer_handle_;
   uv_check_t immediate_check_handle_;
   uv_idle_t immediate_idle_handle_;

src/inspector/tracing_agent.cc

@@ -46,9 +46,7 @@ class InspectorTraceWriter : public node::tracing::AsyncTraceWriter {
 }  // namespace
 
 TracingAgent::TracingAgent(Environment* env)
-                           : env_(env),
-                             trace_writer_(
-                                  tracing::Agent::EmptyClientHandle()) {
+                           : env_(env) {
 }
 
 TracingAgent::~TracingAgent() {
@@ -62,7 +60,7 @@ void TracingAgent::Wire(UberDispatcher* dispatcher) {
 
 DispatchResponse TracingAgent::start(
     std::unique_ptr<protocol::NodeTracing::TraceConfig> traceConfig) {
-  if (trace_writer_ != nullptr) {
+  if (!trace_writer_.empty()) {
     return DispatchResponse::Error(
         "Call NodeTracing::end to stop tracing before updating the config");
   }
@@ -76,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();
 }
 

src/inspector/tracing_agent.h

@@ -2,16 +2,13 @@
 #define SRC_INSPECTOR_TRACING_AGENT_H_
 
 #include "node/inspector/protocol/NodeTracing.h"
+#include "tracing/agent.h"
 #include "v8.h"
 
 
 namespace node {
 class Environment;
 
-namespace tracing {
-class Agent;
-}  // namespace tracing
-
 namespace inspector {
 namespace protocol {
 
@@ -32,8 +29,7 @@ class TracingAgent : public NodeTracing::Backend {
   void DisconnectTraceClient();
 
   Environment* env_;
-  std::unique_ptr<std::pair<tracing::Agent*, int>,
-                  void (*)(std::pair<tracing::Agent*, int>*)> trace_writer_;
+  tracing::AgentWriterHandle trace_writer_;
   std::unique_ptr<NodeTracing::Frontend> frontend_;
 };
 

src/node.cc

@@ -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
@@ -387,7 +388,7 @@ class NodeTraceStateObserver :
 static struct {
 #if NODE_USE_V8_PLATFORM
   void Initialize(int thread_pool_size) {
-    tracing_agent_.reset(new tracing::Agent(trace_file_pattern));
+    tracing_agent_.reset(new tracing::Agent());
     auto controller = tracing_agent_->GetTracingController();
     controller->AddTraceStateObserver(new NodeTraceStateObserver(controller));
     tracing::TraceEventHelper::SetTracingController(controller);
@@ -397,10 +398,10 @@ static struct {
   }
 
   void Dispose() {
+    tracing_agent_.reset(nullptr);
     platform_->Shutdown();
     delete platform_;
     platform_ = nullptr;
-    tracing_agent_.reset(nullptr);
   }
 
   void DrainVMTasks(Isolate* isolate) {
@@ -427,23 +428,31 @@ static struct {
 #endif  // HAVE_INSPECTOR
 
   void StartTracingAgent() {
-    tracing_agent_->Enable(trace_enabled_categories);
+    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::kUseDefaultCategories);
+    }
   }
 
   void StopTracingAgent() {
-    if (tracing_agent_)
-      tracing_agent_->Stop();
+    tracing_file_writer_.reset();
   }
 
-  tracing::Agent* GetTracingAgent() const {
-    return tracing_agent_.get();
+  tracing::AgentWriterHandle* GetTracingAgentWriter() {
+    return &tracing_file_writer_;
   }
 
   NodePlatform* Platform() {
     return platform_;
   }
 
   std::unique_ptr<tracing::Agent> tracing_agent_;
+  tracing::AgentWriterHandle tracing_file_writer_;
   NodePlatform* platform_;
 #else  // !NODE_USE_V8_PLATFORM
   void Initialize(int thread_pool_size) {}
@@ -464,7 +473,9 @@ static struct {
   }
   void StopTracingAgent() {}
 
-  tracing::Agent* GetTracingAgent() const { return nullptr; }
+  tracing::AgentWriterHandle* GetTracingAgentWriter() {
+    return nullptr;
+  }
 
   NodePlatform* Platform() {
     return nullptr;
@@ -3588,7 +3599,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;
 }
@@ -3647,7 +3658,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;

src/node_trace_events.cc

@@ -25,7 +25,7 @@ class NodeCategorySet : public BaseObject {
   static void Enable(const FunctionCallbackInfo<Value>& args);
   static void Disable(const FunctionCallbackInfo<Value>& args);
 
-  const std::set<std::string>& GetCategories() { return categories_; }
+  const std::set<std::string>& GetCategories() const { return categories_; }
 
   void MemoryInfo(MemoryTracker* tracker) const override {
     tracker->TrackThis(this);
@@ -37,8 +37,8 @@ class NodeCategorySet : public BaseObject {
  private:
   NodeCategorySet(Environment* env,
                   Local<Object> wrap,
-                  std::set<std::string> categories) :
-        BaseObject(env, wrap), categories_(categories) {
+                  std::set<std::string>&& categories) :
+        BaseObject(env, wrap), categories_(std::move(categories)) {
     MakeWeak();
   }
 
@@ -52,12 +52,14 @@ void NodeCategorySet::New(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsArray());
   Local<Array> cats = args[0].As<Array>();
   for (size_t n = 0; n < cats->Length(); n++) {
-    Local<Value> category = cats->Get(env->context(), n).ToLocalChecked();
+    Local<Value> category;
+    if (!cats->Get(env->context(), n).ToLocal(&category)) return;
     Utf8Value val(env->isolate(), category);
+    if (!*val) return;
     categories.emplace(*val);
   }
-  CHECK_NOT_NULL(env->tracing_agent());
-  new NodeCategorySet(env, args.This(), categories);
+  CHECK_NOT_NULL(env->tracing_agent_writer());
+  new NodeCategorySet(env, args.This(), std::move(categories));
 }
 
 void NodeCategorySet::Enable(const FunctionCallbackInfo<Value>& args) {
@@ -67,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;
   }
 }
@@ -79,25 +81,28 @@ 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(),
                           categories.c_str(),
-                          v8::NewStringType::kNormal).ToLocalChecked());
+                          v8::NewStringType::kNormal,
+                          categories.size()).ToLocalChecked());
   }
 }
 
 // The tracing APIs require category groups to be pointers to long-lived
 // strings. Those strings are stored here.
-static std::unordered_set<std::string> categoryGroups;
+static std::unordered_set<std::string> category_groups;
+static Mutex category_groups_mutex;
 
 // Gets a pointer to the category-enabled flags for a tracing category group,
 // if tracing is enabled for it.
@@ -107,14 +112,15 @@ static const uint8_t* GetCategoryGroupEnabled(const char* category_group) {
 }
 
 static const char* GetCategoryGroup(Environment* env,
-                                    const Local<Value> categoryValue) {
-  CHECK(categoryValue->IsString());
+                                    const Local<Value> category_value) {
+  CHECK(category_value->IsString());
 
-  Utf8Value category(env->isolate(), categoryValue);
+  Utf8Value category(env->isolate(), category_value);
+  Mutex::ScopedLock lock(category_groups_mutex);
   // If the value already exists in the set, insertion.first will point
   // to the existing value. Thus, this will maintain a long lived pointer
   // to the category c-string.
-  auto insertion = categoryGroups.insert(category.out());
+  auto insertion = category_groups.insert(category.out());
 
   // The returned insertion is a pair whose first item is the object that was
   // inserted or that blocked the insertion and second item is a boolean
@@ -133,7 +139,7 @@ static void Emit(const FunctionCallbackInfo<Value>& args) {
   // enabled.
   const char* category_group = GetCategoryGroup(env, args[1]);
   const uint8_t* category_group_enabled =
-    GetCategoryGroupEnabled(category_group);
+      GetCategoryGroupEnabled(category_group);
   if (*category_group_enabled == 0) return;
 
   // get trace_event phase
@@ -142,8 +148,8 @@ static void Emit(const FunctionCallbackInfo<Value>& args) {
 
   // get trace_event name
   CHECK(args[2]->IsString());
-  Utf8Value nameValue(env->isolate(), args[2]);
-  const char* name = nameValue.out();
+  Utf8Value name_value(env->isolate(), args[2]);
+  const char* name = name_value.out();
 
   // get trace_event id
   int64_t id = 0;
@@ -212,7 +218,7 @@ static void CategoryGroupEnabled(const FunctionCallbackInfo<Value>& args) {
 
   const char* category_group = GetCategoryGroup(env, args[0]);
   const uint8_t* category_group_enabled =
-    GetCategoryGroupEnabled(category_group);
+      GetCategoryGroupEnabled(category_group);
   args.GetReturnValue().Set(*category_group_enabled > 0);
 }