diff options
author | Anna Henningsen <anna@addaleax.net> | 2018-07-17 03:50:07 +0300 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2018-08-01 18:16:51 +0300 |
commit | 620e46c8b59b6269784ebba42b23f23897eeedb9 (patch) | |
tree | 6b396fdec4964d0305bbbb62195ebf56e636c78e /src/tracing | |
parent | 3ac94dc977b232a4502307733af2b3af60e7b102 (diff) |
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: https://github.com/nodejs/node/pull/21867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Diffstat (limited to 'src/tracing')
-rw-r--r-- | src/tracing/agent.cc | 97 | ||||
-rw-r--r-- | src/tracing/agent.h | 47 |
2 files changed, 79 insertions, 65 deletions
diff --git a/src/tracing/agent.cc b/src/tracing/agent.cc index 9cc21863e3e..641c476c1d8 100644 --- a/src/tracing/agent.cc +++ b/src/tracing/agent.cc @@ -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); @@ -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()); @@ -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_); @@ -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() { @@ -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 { @@ -178,5 +168,6 @@ void Agent::Flush(bool blocking) { for (const auto& id_writer : writers_) id_writer.second->Flush(blocking); } + } // namespace tracing } // namespace node diff --git a/src/tracing/agent.h b/src/tracing/agent.h index 022e86f11af..b62dc5fe5bc 100644 --- a/src/tracing/agent.h +++ b/src/tracing/agent.h @@ -44,6 +44,11 @@ class AgentWriterHandle { inline bool empty() const { return agent_ == nullptr; } inline void reset(); + inline void Enable(const std::set<std::string>& categories); + inline void Disable(const std::set<std::string>& categories); + + inline Agent* agent() { return agent_; } + private: inline AgentWriterHandle(Agent* agent, int id) : agent_(agent), id_(id) {} @@ -58,19 +63,23 @@ class AgentWriterHandle { class Agent { public: - explicit Agent(const std::string& log_file_pattern); - void Stop(); + Agent(); TracingController* GetTracingController() { return tracing_controller_; } + enum UseDefaultCategoryMode { + kUseDefaultCategories, + kIgnoreDefaultCategories + }; + // Destroying the handle disconnects the client AgentWriterHandle AddClient(const std::set<std::string>& categories, - std::unique_ptr<AsyncTraceWriter> writer); - - // These 3 methods operate on a "default" client, e.g. the file writer - void Enable(const std::string& categories); - void Enable(const std::set<std::string>& categories); - void Disable(const std::set<std::string>& categories); + std::unique_ptr<AsyncTraceWriter> writer, + enum UseDefaultCategoryMode mode); + // A handle that is only used for managing the default categories + // (which can then implicitly be used through using `USE_DEFAULT_CATEGORIES` + // when adding a client later). + AgentWriterHandle DefaultHandle(); // Returns a comma-separated list of enabled categories. std::string GetEnabledCategories() const; @@ -82,6 +91,9 @@ class Agent { TraceConfig* CreateTraceConfig() const; + // TODO(addaleax): This design is broken and inherently thread-unsafe. + inline uv_loop_t* loop() { return &tracing_loop_; } + private: friend class AgentWriterHandle; @@ -91,19 +103,22 @@ class Agent { void StopTracing(); void Disconnect(int client); - const std::string& log_file_pattern_; + void Enable(int id, const std::set<std::string>& categories); + void Disable(int id, const std::set<std::string>& categories); + uv_thread_t thread_; uv_loop_t tracing_loop_; + bool started_ = false; + class ScopedSuspendTracing; // Each individual Writer has one id. int next_writer_id_ = 1; + enum { kDefaultHandleId = -1 }; // These maps store the original arguments to AddClient(), by id. - std::unordered_map<int, std::set<std::string>> categories_; + std::unordered_map<int, std::multiset<std::string>> categories_; std::unordered_map<int, std::unique_ptr<AsyncTraceWriter>> writers_; TracingController* tracing_controller_ = nullptr; - AgentWriterHandle file_writer_; - std::multiset<std::string> file_writer_categories_; }; void AgentWriterHandle::reset() { @@ -124,6 +139,14 @@ AgentWriterHandle::AgentWriterHandle(AgentWriterHandle&& other) { *this = std::move(other); } +void AgentWriterHandle::Enable(const std::set<std::string>& categories) { + if (agent_ != nullptr) agent_->Enable(id_, categories); +} + +void AgentWriterHandle::Disable(const std::set<std::string>& categories) { + if (agent_ != nullptr) agent_->Disable(id_, categories); +} + } // namespace tracing } // namespace node |