use unique_ptr for TraceItem Also this guards event_ access by mutex. Change-Id: I71a67546c8b3474775e3eac4b33a6e0f188791c7 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/13320 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/import_manager.cc b/src/gn/import_manager.cc index a4928bf..69f44b4 100644 --- a/src/gn/import_manager.cc +++ b/src/gn/import_manager.cc
@@ -125,14 +125,14 @@ if (TracingEnabled() && TicksDelta(import_block_end, import_block_begin).InMilliseconds() > kImportBlockTraceThresholdMS) { - auto* import_block_trace = - new TraceItem(TraceItem::TRACE_IMPORT_BLOCK, file.value(), - std::this_thread::get_id()); + auto import_block_trace = std::make_unique<TraceItem>( + TraceItem::TRACE_IMPORT_BLOCK, file.value(), + std::this_thread::get_id()); import_block_trace->set_begin(import_block_begin); import_block_trace->set_end(import_block_end); import_block_trace->set_toolchain( scope->settings()->toolchain_label().GetUserVisibleName(false)); - AddTrace(import_block_trace); + AddTrace(std::move(import_block_trace)); } }
diff --git a/src/gn/trace.cc b/src/gn/trace.cc index c6eeed2..f812710 100644 --- a/src/gn/trace.cc +++ b/src/gn/trace.cc
@@ -30,18 +30,25 @@ TraceLog() { events_.reserve(16384); } // Trace items leaked intentionally. - void Add(TraceItem* item) { + void Add(std::unique_ptr<TraceItem> item) { std::lock_guard<std::mutex> lock(lock_); - events_.push_back(item); + events_.push_back(std::move(item)); } // Returns a copy for threadsafety. - std::vector<TraceItem*> events() const { return events_; } + std::vector<TraceItem*> events() const { + std::vector<TraceItem*> events; + std::lock_guard<std::mutex> lock(lock_); + events.reserve(events_.size()); + for (const auto& e : events_) + events.push_back(e.get()); + return events; + } private: - std::mutex lock_; + mutable std::mutex lock_; - std::vector<TraceItem*> events_; + std::vector<std::unique_ptr<TraceItem>> events_; TraceLog(const TraceLog&) = delete; TraceLog& operator=(const TraceLog&) = delete; @@ -120,18 +127,17 @@ TraceItem::~TraceItem() = default; ScopedTrace::ScopedTrace(TraceItem::Type t, const std::string& name) - : item_(nullptr), done_(false) { + : done_(false) { if (trace_log) { - item_ = new TraceItem(t, name, std::this_thread::get_id()); + item_ = std::make_unique<TraceItem>(t, name, std::this_thread::get_id()); item_->set_begin(TicksNow()); } } -ScopedTrace::ScopedTrace(TraceItem::Type t, const Label& label) - : item_(nullptr), done_(false) { +ScopedTrace::ScopedTrace(TraceItem::Type t, const Label& label) : done_(false) { if (trace_log) { - item_ = new TraceItem(t, label.GetUserVisibleName(false), - std::this_thread::get_id()); + item_ = std::make_unique<TraceItem>(t, label.GetUserVisibleName(false), + std::this_thread::get_id()); item_->set_begin(TicksNow()); } } @@ -155,7 +161,7 @@ done_ = true; if (trace_log) { item_->set_end(TicksNow()); - AddTrace(item_); + AddTrace(std::move(item_)); } } } @@ -169,8 +175,8 @@ return !!trace_log; } -void AddTrace(TraceItem* item) { - trace_log->Add(item); +void AddTrace(std::unique_ptr<TraceItem> item) { + trace_log->Add(std::move(item)); } std::string SummarizeTraces() {
diff --git a/src/gn/trace.h b/src/gn/trace.h index b89e82a..b59cd34 100644 --- a/src/gn/trace.h +++ b/src/gn/trace.h
@@ -5,6 +5,7 @@ #ifndef TOOLS_GN_TRACE_H_ #define TOOLS_GN_TRACE_H_ +#include <memory> #include <string> #include <thread> @@ -80,7 +81,7 @@ void Done(); private: - TraceItem* item_; + std::unique_ptr<TraceItem> item_; bool done_; }; @@ -90,8 +91,8 @@ // Returns whether tracing is enabled. bool TracingEnabled(); -// Adds a trace event to the log. Takes ownership of the pointer. -void AddTrace(TraceItem* item); +// Adds a trace event to the log. +void AddTrace(std::unique_ptr<TraceItem> item); // Returns a summary of the current traces, or the empty string if tracing is // not enabled.