Commit c101b39
src: refactor default trace writer out of agent
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>1 parent daafe6c commit c101b39
10 files changed
Lines changed: 130 additions & 87 deletions
File tree
- src
- inspector
- tracing
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -333,8 +333,8 @@ inline v8::Isolate* Environment::isolate() const { | |||
| 333 | 333 | return isolate_; | |
| 334 | 334 | } | |
| 335 | 335 | ||
| 336 | - inline tracing::Agent* Environment::tracing_agent() const { | ||
| 337 | - return tracing_agent_; | ||
| 336 | + inline tracing::AgentWriterHandle* Environment::tracing_agent_writer() const { | ||
| 337 | + return tracing_agent_writer_; | ||
| 338 | 338 | } | |
| 339 | 339 | ||
| 340 | 340 | inline Environment* Environment::from_immediate_check_handle( | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -103,10 +103,10 @@ void InitThreadLocalOnce() { | |||
| 103 | 103 | ||
| 104 | 104 | Environment::Environment(IsolateData* isolate_data, | |
| 105 | 105 | Local<Context> context, | |
| 106 | - tracing::Agent* tracing_agent) | ||
| 106 | + tracing::AgentWriterHandle* tracing_agent_writer) | ||
| 107 | 107 | : isolate_(context->GetIsolate()), | |
| 108 | 108 | isolate_data_(isolate_data), | |
| 109 | - tracing_agent_(tracing_agent), | ||
| 109 | + tracing_agent_writer_(tracing_agent_writer), | ||
| 110 | 110 | immediate_info_(context->GetIsolate()), | |
| 111 | 111 | tick_info_(context->GetIsolate()), | |
| 112 | 112 | timer_base_(uv_now(isolate_data->event_loop())), | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -55,7 +55,7 @@ class performance_state; | |||
| 55 | 55 | } | |
| 56 | 56 | ||
| 57 | 57 | namespace tracing { | |
| 58 | - class Agent; | ||
| 58 | + class AgentWriterHandle; | ||
| 59 | 59 | } | |
| 60 | 60 | ||
| 61 | 61 | namespace worker { | |
@@ -590,7 +590,7 @@ class Environment { | |||
| 590 | 590 | ||
| 591 | 591 | Environment(IsolateData* isolate_data, | |
| 592 | 592 | v8::Local<v8::Context> context, | |
| 593 | - tracing::Agent* tracing_agent); | ||
| 593 | + tracing::AgentWriterHandle* tracing_agent_writer); | ||
| 594 | 594 | ~Environment(); | |
| 595 | 595 | ||
| 596 | 596 | void Start(int argc, | |
@@ -628,7 +628,7 @@ class Environment { | |||
| 628 | 628 | inline bool profiler_idle_notifier_started() const; | |
| 629 | 629 | ||
| 630 | 630 | inline v8::Isolate* isolate() const; | |
| 631 | - inline tracing::Agent* tracing_agent() const; | ||
| 631 | + inline tracing::AgentWriterHandle* tracing_agent_writer() const; | ||
| 632 | 632 | inline uv_loop_t* event_loop() const; | |
| 633 | 633 | inline uint32_t watched_providers() const; | |
| 634 | 634 | ||
@@ -877,7 +877,7 @@ class Environment { | |||
| 877 | 877 | ||
| 878 | 878 | v8::Isolate* const isolate_; | |
| 879 | 879 | IsolateData* const isolate_data_; | |
| 880 | - tracing::Agent* const tracing_agent_; | ||
| 880 | + tracing::AgentWriterHandle* const tracing_agent_writer_; | ||
| 881 | 881 | uv_check_t immediate_check_handle_; | |
| 882 | 882 | uv_idle_t immediate_idle_handle_; | |
| 883 | 883 | uv_prepare_t idle_prepare_handle_; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -74,10 +74,11 @@ DispatchResponse TracingAgent::start( | |||
| 74 | 74 | if (categories_set.empty()) | |
| 75 | 75 | return DispatchResponse::Error("At least one category should be enabled"); | |
| 76 | 76 | ||
| 77 | - trace_writer_ = env_->tracing_agent()->AddClient( | ||
| 77 | + trace_writer_ = env_->tracing_agent_writer()->agent()->AddClient( | ||
| 78 | 78 | categories_set, | |
| 79 | 79 | std::unique_ptr<InspectorTraceWriter>( | |
| 80 | - new InspectorTraceWriter(frontend_.get()))); | ||
| 80 | + new InspectorTraceWriter(frontend_.get())), | ||
| 81 | + tracing::Agent::kIgnoreDefaultCategories); | ||
| 81 | 82 | return DispatchResponse::OK(); | |
| 82 | 83 | } | |
| 83 | 84 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -61,6 +61,7 @@ | |||
| 61 | 61 | #include "req_wrap-inl.h" | |
| 62 | 62 | #include "string_bytes.h" | |
| 63 | 63 | #include "tracing/agent.h" | |
| 64 | + #include "tracing/node_trace_writer.h" | ||
| 64 | 65 | #include "util.h" | |
| 65 | 66 | #include "uv.h" | |
| 66 | 67 | #if NODE_USE_V8_PLATFORM | |
@@ -427,17 +428,24 @@ static struct { | |||
| 427 | 428 | #endif // HAVE_INSPECTOR | |
| 428 | 429 | ||
| 429 | 430 | void StartTracingAgent() { | |
| 430 | - tracing_file_writer_ = tracing_agent_->AddClient( | ||
| 431 | - trace_enabled_categories, | ||
| 432 | - new tracing::NodeTraceWriter(trace_file_pattern)); | ||
| 431 | + if (trace_enabled_categories.empty()) { | ||
| 432 | + tracing_file_writer_ = tracing_agent_->DefaultHandle(); | ||
| 433 | + } else { | ||
| 434 | + tracing_file_writer_ = tracing_agent_->AddClient( | ||
| 435 | + ParseCommaSeparatedSet(trace_enabled_categories), | ||
| 436 | + std::unique_ptr<tracing::AsyncTraceWriter>( | ||
| 437 | + new tracing::NodeTraceWriter(trace_file_pattern, | ||
| 438 | + tracing_agent_->loop())), | ||
| 439 | + tracing::Agent::kUseDefaultCategories); | ||
| 440 | + } | ||
| 433 | 441 | } | |
| 434 | 442 | ||
| 435 | 443 | void StopTracingAgent() { | |
| 436 | 444 | tracing_file_writer_.reset(); | |
| 437 | 445 | } | |
| 438 | 446 | ||
| 439 | - tracing::Agent* GetTracingAgent() const { | ||
| 440 | - return tracing_agent_.get(); | ||
| 447 | + tracing::AgentWriterHandle* GetTracingAgentWriter() { | ||
| 448 | + return &tracing_file_writer_; | ||
| 441 | 449 | } | |
| 442 | 450 | ||
| 443 | 451 | NodePlatform* Platform() { | |
@@ -466,7 +474,9 @@ static struct { | |||
| 466 | 474 | } | |
| 467 | 475 | void StopTracingAgent() {} | |
| 468 | 476 | ||
| 469 | - tracing::Agent* GetTracingAgent() const { return nullptr; } | ||
| 477 | + tracing::AgentWriterHandle* GetTracingAgentWriter() { | ||
| 478 | + return nullptr; | ||
| 479 | + } | ||
| 470 | 480 | ||
| 471 | 481 | NodePlatform* Platform() { | |
| 472 | 482 | return nullptr; | |
@@ -3593,7 +3603,7 @@ Environment* CreateEnvironment(IsolateData* isolate_data, | |||
| 3593 | 3603 | HandleScope handle_scope(isolate); | |
| 3594 | 3604 | Context::Scope context_scope(context); | |
| 3595 | 3605 | auto env = new Environment(isolate_data, context, | |
| 3596 | - v8_platform.GetTracingAgent()); | ||
| 3606 | + v8_platform.GetTracingAgentWriter()); | ||
| 3597 | 3607 | env->Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); | |
| 3598 | 3608 | return env; | |
| 3599 | 3609 | } | |
@@ -3652,7 +3662,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, | |||
| 3652 | 3662 | HandleScope handle_scope(isolate); | |
| 3653 | 3663 | Local<Context> context = NewContext(isolate); | |
| 3654 | 3664 | Context::Scope context_scope(context); | |
| 3655 | - Environment env(isolate_data, context, v8_platform.GetTracingAgent()); | ||
| 3665 | + Environment env(isolate_data, context, v8_platform.GetTracingAgentWriter()); | ||
| 3656 | 3666 | env.Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); | |
| 3657 | 3667 | ||
| 3658 | 3668 | const char* path = argc > 1 ? argv[1] : nullptr; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -58,7 +58,7 @@ void NodeCategorySet::New(const FunctionCallbackInfo<Value>& args) { | |||
| 58 | 58 | if (!*val) return; | |
| 59 | 59 | categories.emplace(*val); | |
| 60 | 60 | } | |
| 61 | - CHECK_NOT_NULL(env->tracing_agent()); | ||
| 61 | + CHECK_NOT_NULL(env->tracing_agent_writer()); | ||
| 62 | 62 | new NodeCategorySet(env, args.This(), std::move(categories)); | |
| 63 | 63 | } | |
| 64 | 64 | ||
@@ -69,7 +69,7 @@ void NodeCategorySet::Enable(const FunctionCallbackInfo<Value>& args) { | |||
| 69 | 69 | CHECK_NOT_NULL(category_set); | |
| 70 | 70 | const auto& categories = category_set->GetCategories(); | |
| 71 | 71 | if (!category_set->enabled_ && !categories.empty()) { | |
| 72 | - env->tracing_agent()->Enable(categories); | ||
| 72 | + env->tracing_agent_writer()->Enable(categories); | ||
| 73 | 73 | category_set->enabled_ = true; | |
| 74 | 74 | } | |
| 75 | 75 | } | |
@@ -81,14 +81,15 @@ void NodeCategorySet::Disable(const FunctionCallbackInfo<Value>& args) { | |||
| 81 | 81 | CHECK_NOT_NULL(category_set); | |
| 82 | 82 | const auto& categories = category_set->GetCategories(); | |
| 83 | 83 | if (category_set->enabled_ && !categories.empty()) { | |
| 84 | - env->tracing_agent()->Disable(categories); | ||
| 84 | + env->tracing_agent_writer()->Disable(categories); | ||
| 85 | 85 | category_set->enabled_ = false; | |
| 86 | 86 | } | |
| 87 | 87 | } | |
| 88 | 88 | ||
| 89 | 89 | void GetEnabledCategories(const FunctionCallbackInfo<Value>& args) { | |
| 90 | 90 | Environment* env = Environment::GetCurrent(args); | |
| 91 | - std::string categories = env->tracing_agent()->GetEnabledCategories(); | ||
| 91 | + std::string categories = | ||
| 92 | + env->tracing_agent_writer()->agent()->GetEnabledCategories(); | ||
| 92 | 93 | if (!categories.empty()) { | |
| 93 | 94 | args.GetReturnValue().Set( | |
| 94 | 95 | String::NewFromUtf8(env->isolate(), | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,23 +1,24 @@ | |||
| 1 | 1 | #include "tracing/agent.h" | |
| 2 | 2 | ||
| 3 | - #include <sstream> | ||
| 4 | 3 | #include <string> | |
| 5 | 4 | #include "tracing/node_trace_buffer.h" | |
| 6 | - #include "tracing/node_trace_writer.h" | ||
| 7 | 5 | ||
| 8 | 6 | namespace node { | |
| 9 | 7 | namespace tracing { | |
| 10 | 8 | ||
| 11 | - namespace { | ||
| 12 | - | ||
| 13 | - class ScopedSuspendTracing { | ||
| 9 | + class Agent::ScopedSuspendTracing { | ||
| 14 | 10 | public: | |
| 15 | - ScopedSuspendTracing(TracingController* controller, Agent* agent) | ||
| 16 | - : controller_(controller), agent_(agent) { | ||
| 17 | - controller->StopTracing(); | ||
| 11 | + ScopedSuspendTracing(TracingController* controller, Agent* agent, | ||
| 12 | + bool do_suspend = true) | ||
| 13 | + : controller_(controller), agent_(do_suspend ? agent : nullptr) { | ||
| 14 | + if (do_suspend) { | ||
| 15 | + CHECK(agent_->started_); | ||
| 16 | + controller->StopTracing(); | ||
| 17 | + } | ||
| 18 | 18 | } | |
| 19 | 19 | ||
| 20 | 20 | ~ScopedSuspendTracing() { | |
| 21 | + if (agent_ == nullptr) return; | ||
| 21 | 22 | TraceConfig* config = agent_->CreateTraceConfig(); | |
| 22 | 23 | if (config != nullptr) { | |
| 23 | 24 | controller_->StartTracing(config); | |
@@ -29,8 +30,10 @@ class ScopedSuspendTracing { | |||
| 29 | 30 | Agent* agent_; | |
| 30 | 31 | }; | |
| 31 | 32 | ||
| 33 | + namespace { | ||
| 34 | + | ||
| 32 | 35 | std::set<std::string> flatten( | |
| 33 | - const std::unordered_map<int, std::set<std::string>>& map) { | ||
| 36 | + const std::unordered_map<int, std::multiset<std::string>>& map) { | ||
| 34 | 37 | std::set<std::string> result; | |
| 35 | 38 | for (const auto& id_value : map) | |
| 36 | 39 | result.insert(id_value.second.begin(), id_value.second.end()); | |
@@ -43,18 +46,17 @@ using v8::platform::tracing::TraceConfig; | |||
| 43 | 46 | using v8::platform::tracing::TraceWriter; | |
| 44 | 47 | using std::string; | |
| 45 | 48 | ||
| 46 | - Agent::Agent(const std::string& log_file_pattern) | ||
| 47 | - : log_file_pattern_(log_file_pattern) { | ||
| 49 | + Agent::Agent() { | ||
| 48 | 50 | tracing_controller_ = new TracingController(); | |
| 49 | 51 | tracing_controller_->Initialize(nullptr); | |
| 52 | + | ||
| 53 | + CHECK_EQ(uv_loop_init(&tracing_loop_), 0); | ||
| 50 | 54 | } | |
| 51 | 55 | ||
| 52 | 56 | void Agent::Start() { | |
| 53 | 57 | if (started_) | |
| 54 | 58 | return; | |
| 55 | 59 | ||
| 56 | - CHECK_EQ(uv_loop_init(&tracing_loop_), 0); | ||
| 57 | - | ||
| 58 | 60 | NodeTraceBuffer* trace_buffer_ = new NodeTraceBuffer( | |
| 59 | 61 | NodeTraceBuffer::kBufferChunks, this, &tracing_loop_); | |
| 60 | 62 | tracing_controller_->Initialize(trace_buffer_); | |
@@ -71,18 +73,30 @@ void Agent::Start() { | |||
| 71 | 73 | ||
| 72 | 74 | AgentWriterHandle Agent::AddClient( | |
| 73 | 75 | const std::set<std::string>& categories, | |
| 74 | - std::unique_ptr<AsyncTraceWriter> writer) { | ||
| 76 | + std::unique_ptr<AsyncTraceWriter> writer, | ||
| 77 | + enum UseDefaultCategoryMode mode) { | ||
| 75 | 78 | Start(); | |
| 79 | + | ||
| 80 | + const std::set<std::string>* use_categories = &categories; | ||
| 81 | + | ||
| 82 | + std::set<std::string> categories_with_default; | ||
| 83 | + if (mode == kUseDefaultCategories) { | ||
| 84 | + categories_with_default.insert(categories.begin(), categories.end()); | ||
| 85 | + categories_with_default.insert(categories_[kDefaultHandleId].begin(), | ||
| 86 | + categories_[kDefaultHandleId].end()); | ||
| 87 | + use_categories = &categories_with_default; | ||
| 88 | + } | ||
| 89 | + | ||
| 76 | 90 | ScopedSuspendTracing suspend(tracing_controller_, this); | |
| 77 | 91 | int id = next_writer_id_++; | |
| 78 | 92 | writers_[id] = std::move(writer); | |
| 79 | - categories_[id] = categories; | ||
| 93 | + categories_[id] = { use_categories->begin(), use_categories->end() }; | ||
| 80 | 94 | ||
| 81 | 95 | return AgentWriterHandle(this, id); | |
| 82 | 96 | } | |
| 83 | 97 | ||
| 84 | - void Agent::Stop() { | ||
| 85 | - file_writer_.reset(); | ||
| 98 | + AgentWriterHandle Agent::DefaultHandle() { | ||
| 99 | + return AgentWriterHandle(this, kDefaultHandleId); | ||
| 86 | 100 | } | |
| 87 | 101 | ||
| 88 | 102 | void Agent::StopTracing() { | |
@@ -99,54 +113,30 @@ void Agent::StopTracing() { | |||
| 99 | 113 | } | |
| 100 | 114 | ||
| 101 | 115 | void Agent::Disconnect(int client) { | |
| 116 | + if (client == kDefaultHandleId) return; | ||
| 102 | 117 | ScopedSuspendTracing suspend(tracing_controller_, this); | |
| 103 | 118 | writers_.erase(client); | |
| 104 | 119 | categories_.erase(client); | |
| 105 | 120 | } | |
| 106 | 121 | ||
| 107 | - void Agent::Enable(const std::string& categories) { | ||
| 122 | + void Agent::Enable(int id, const std::set<std::string>& categories) { | ||
| 108 | 123 | if (categories.empty()) | |
| 109 | 124 | return; | |
| 110 | - std::set<std::string> categories_set; | ||
| 111 | - std::istringstream category_list(categories); | ||
| 112 | - while (category_list.good()) { | ||
| 113 | - std::string category; | ||
| 114 | - getline(category_list, category, ','); | ||
| 115 | - categories_set.emplace(std::move(category)); | ||
| 116 | - } | ||
| 117 | - Enable(categories_set); | ||
| 118 | - } | ||
| 119 | 125 | ||
| 120 | - void Agent::Enable(const std::set<std::string>& categories) { | ||
| 121 | - if (categories.empty()) | ||
| 122 | - return; | ||
| 123 | - | ||
| 124 | - file_writer_categories_.insert(categories.begin(), categories.end()); | ||
| 125 | - std::set<std::string> full_list(file_writer_categories_.begin(), | ||
| 126 | - file_writer_categories_.end()); | ||
| 127 | - if (file_writer_.empty()) { | ||
| 128 | - // Ensure background thread is running | ||
| 129 | - Start(); | ||
| 130 | - std::unique_ptr<NodeTraceWriter> writer( | ||
| 131 | - new NodeTraceWriter(log_file_pattern_, &tracing_loop_)); | ||
| 132 | - file_writer_ = AddClient(full_list, std::move(writer)); | ||
| 133 | - } else { | ||
| 134 | - ScopedSuspendTracing suspend(tracing_controller_, this); | ||
| 135 | - categories_[file_writer_.id_] = full_list; | ||
| 136 | - } | ||
| 126 | + ScopedSuspendTracing suspend(tracing_controller_, this, | ||
| 127 | + id != kDefaultHandleId); | ||
| 128 | + categories_[id].insert(categories.begin(), categories.end()); | ||
| 137 | 129 | } | |
| 138 | 130 | ||
| 139 | - void Agent::Disable(const std::set<std::string>& categories) { | ||
| 131 | + void Agent::Disable(int id, const std::set<std::string>& categories) { | ||
| 132 | + ScopedSuspendTracing suspend(tracing_controller_, this, | ||
| 133 | + id != kDefaultHandleId); | ||
| 134 | + std::multiset<std::string>& writer_categories = categories_[id]; | ||
| 140 | 135 | for (const std::string& category : categories) { | |
| 141 | - auto it = file_writer_categories_.find(category); | ||
| 142 | - if (it != file_writer_categories_.end()) | ||
| 143 | - file_writer_categories_.erase(it); | ||
| 136 | + auto it = writer_categories.find(category); | ||
| 137 | + if (it != writer_categories.end()) | ||
| 138 | + writer_categories.erase(it); | ||
| 144 | 139 | } | |
| 145 | - if (file_writer_.empty()) | ||
| 146 | - return; | ||
| 147 | - ScopedSuspendTracing suspend(tracing_controller_, this); | ||
| 148 | - categories_[file_writer_.id_] = { file_writer_categories_.begin(), | ||
| 149 | - file_writer_categories_.end() }; | ||
| 150 | 140 | } | |
| 151 | 141 | ||
| 152 | 142 | TraceConfig* Agent::CreateTraceConfig() const { | |
@@ -178,5 +168,6 @@ void Agent::Flush(bool blocking) { | |||
| 178 | 168 | for (const auto& id_writer : writers_) | |
| 179 | 169 | id_writer.second->Flush(blocking); | |
| 180 | 170 | } | |
| 171 | + | ||
| 181 | 172 | } // namespace tracing | |
| 182 | 173 | } // namespace node | |
0 commit comments