Author: Pavel Labath Date: 2022-01-19T16:42:53+01:00 New Revision: 4f89157b9d73711a7ce20b597f93eb17a3133adf
URL: https://github.com/llvm/llvm-project/commit/4f89157b9d73711a7ce20b597f93eb17a3133adf DIFF: https://github.com/llvm/llvm-project/commit/4f89157b9d73711a7ce20b597f93eb17a3133adf.diff LOG: [lldb] Make StatsDuration thread-safe std::chrono::duration types are not thread-safe, and they cannot be concurrently updated from multiple threads. Currently, we were doing such a thing (only) in the DWARF indexing code (DWARFUnit::ExtractDIEsRWLocked), but I think it can easily happen that someone else tries to update another statistic like this without bothering to check for thread safety. This patch changes the StatsDuration type from a simple typedef into a class in its own right. The class stores the duration internally as std::atomic<uint64_t> (so it can be updated atomically), but presents it to its users as the usual chrono type (duration<float>). Differential Revision: https://reviews.llvm.org/D117474 Added: Modified: lldb/include/lldb/Breakpoint/Breakpoint.h lldb/include/lldb/Core/Module.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Target/Statistics.h lldb/source/Breakpoint/Breakpoint.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Target/Statistics.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index 40435d5c3d0f..113f8c4905e1 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -581,7 +581,7 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, llvm::json::Value GetStatistics(); /// Get the time it took to resolve all locations in this breakpoint. - StatsDuration GetResolveTime() const { return m_resolve_time; } + StatsDuration::Duration GetResolveTime() const { return m_resolve_time; } protected: friend class Target; @@ -660,7 +660,7 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, BreakpointName::Permissions m_permissions; - StatsDuration m_resolve_time{0.0}; + StatsDuration m_resolve_time; void SendBreakpointChangedEvent(lldb::BreakpointEventType eventKind); diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h index d53481a1c720..f6c32586eda8 100644 --- a/lldb/include/lldb/Core/Module.h +++ b/lldb/include/lldb/Core/Module.h @@ -1047,11 +1047,11 @@ class Module : public std::enable_shared_from_this<Module>, /// We store a symbol table parse time duration here because we might have /// an object file and a symbol file which both have symbol tables. The parse /// time for the symbol tables can be aggregated here. - StatsDuration m_symtab_parse_time{0.0}; + StatsDuration m_symtab_parse_time; /// We store a symbol named index time duration here because we might have /// an object file and a symbol file which both have symbol tables. The parse /// time for the symbol tables can be aggregated here. - StatsDuration m_symtab_index_time{0.0}; + StatsDuration m_symtab_index_time; /// Resolve a file or load virtual address. /// diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index 288576b978a7..6cdb2bfb686b 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -316,14 +316,14 @@ class SymbolFile : public PluginInterface { /// /// \returns 0.0 if no information has been parsed or if there is /// no computational cost to parsing the debug information. - virtual StatsDuration GetDebugInfoParseTime() { return StatsDuration(0.0); } + virtual StatsDuration::Duration GetDebugInfoParseTime() { return {}; } /// Return the time it took to index the debug information in the object /// file. /// /// \returns 0.0 if the file doesn't need to be indexed or if it /// hasn't been indexed yet, or a valid duration if it has. - virtual StatsDuration GetDebugInfoIndexTime() { return StatsDuration(0.0); } + virtual StatsDuration::Duration GetDebugInfoIndexTime() { return {}; } /// Accessors for the bool that indicates if the debug info index was loaded /// from, or saved to the module index cache. diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index cf4fb83c816e..dbf3554986aa 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -9,20 +9,39 @@ #ifndef LLDB_TARGET_STATISTICS_H #define LLDB_TARGET_STATISTICS_H -#include <chrono> -#include <string> -#include <vector> - #include "lldb/Utility/Stream.h" #include "lldb/lldb-forward.h" #include "llvm/Support/JSON.h" +#include <atomic> +#include <chrono> +#include <string> +#include <vector> namespace lldb_private { using StatsClock = std::chrono::high_resolution_clock; -using StatsDuration = std::chrono::duration<double>; using StatsTimepoint = std::chrono::time_point<StatsClock>; +class StatsDuration { +public: + using Duration = std::chrono::duration<double>; + + Duration get() const { + return Duration(InternalDuration(value.load(std::memory_order_relaxed))); + } + operator Duration() const { return get(); } + + StatsDuration &operator+=(Duration dur) { + value.fetch_add(std::chrono::duration_cast<InternalDuration>(dur).count(), + std::memory_order_relaxed); + return *this; + } + +private: + using InternalDuration = std::chrono::duration<uint64_t, std::micro>; + std::atomic<uint64_t> value{0}; +}; + /// A class that measures elapsed time in an exception safe way. /// /// This is a RAII class is designed to help gather timing statistics within @@ -54,7 +73,7 @@ class ElapsedTime { m_start_time = StatsClock::now(); } ~ElapsedTime() { - StatsDuration elapsed = StatsClock::now() - m_start_time; + StatsClock::duration elapsed = StatsClock::now() - m_start_time; m_elapsed_time += elapsed; } }; @@ -104,7 +123,7 @@ class TargetStats { StatsSuccessFail &GetFrameVariableStats() { return m_frame_var; } protected: - StatsDuration m_create_time{0.0}; + StatsDuration m_create_time; llvm::Optional<StatsTimepoint> m_launch_or_attach_time; llvm::Optional<StatsTimepoint> m_first_private_stop_time; llvm::Optional<StatsTimepoint> m_first_public_stop_time; diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp index d6acf659e852..eeb8eac33567 100644 --- a/lldb/source/Breakpoint/Breakpoint.cpp +++ b/lldb/source/Breakpoint/Breakpoint.cpp @@ -1094,7 +1094,7 @@ Breakpoint::BreakpointEventData::GetBreakpointLocationAtIndexFromEvent( json::Value Breakpoint::GetStatistics() { json::Object bp; bp.try_emplace("id", GetID()); - bp.try_emplace("resolveTime", m_resolve_time.count()); + bp.try_emplace("resolveTime", m_resolve_time.get().count()); bp.try_emplace("numLocations", (int64_t)GetNumLocations()); bp.try_emplace("numResolvedLocations", (int64_t)GetNumResolvedLocations()); bp.try_emplace("internal", IsInternal()); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h index 1d3d70dfef01..c4995e721554 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h @@ -64,11 +64,11 @@ class DWARFIndex { virtual void Dump(Stream &s) = 0; - StatsDuration GetIndexTime() { return m_index_time; } + StatsDuration::Duration GetIndexTime() { return m_index_time; } protected: Module &m_module; - StatsDuration m_index_time{0.0}; + StatsDuration m_index_time; /// Helper function implementing common logic for processing function dies. If /// the function given by "ref" matches search criteria given by diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index ca701c6f2fcc..02d1a6a4a8be 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4086,8 +4086,8 @@ LanguageType SymbolFileDWARF::GetLanguageFamily(DWARFUnit &unit) { return LanguageTypeFromDWARF(lang); } -StatsDuration SymbolFileDWARF::GetDebugInfoIndexTime() { +StatsDuration::Duration SymbolFileDWARF::GetDebugInfoIndexTime() { if (m_index) return m_index->GetIndexTime(); - return StatsDuration(0.0); + return {}; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index e81ce28cb86e..f84a78620e17 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -319,10 +319,10 @@ class SymbolFileDWARF : public lldb_private::SymbolFile, /// Same as GetLanguage() but reports all C++ versions as C++ (no version). static lldb::LanguageType GetLanguageFamily(DWARFUnit &unit); - lldb_private::StatsDuration GetDebugInfoParseTime() override { + lldb_private::StatsDuration::Duration GetDebugInfoParseTime() override { return m_parse_time; } - lldb_private::StatsDuration GetDebugInfoIndexTime() override; + lldb_private::StatsDuration::Duration GetDebugInfoIndexTime() override; lldb_private::StatsDuration &GetDebugInfoParseTimeRef() { return m_parse_time; @@ -559,7 +559,7 @@ class SymbolFileDWARF : public lldb_private::SymbolFile, /// Try to filter out this debug info by comparing it to the lowest code /// address in the module. lldb::addr_t m_first_code_address = LLDB_INVALID_ADDRESS; - lldb_private::StatsDuration m_parse_time{0.0}; + lldb_private::StatsDuration m_parse_time; }; #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARF_H diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 2491f6af8c19..6ee189e04250 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -1447,8 +1447,8 @@ uint64_t SymbolFileDWARFDebugMap::GetDebugInfoSize() { return debug_info_size; } -StatsDuration SymbolFileDWARFDebugMap::GetDebugInfoParseTime() { - StatsDuration elapsed(0.0); +StatsDuration::Duration SymbolFileDWARFDebugMap::GetDebugInfoParseTime() { + StatsDuration::Duration elapsed(0.0); ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { ObjectFile *oso_objfile = oso_dwarf->GetObjectFile(); if (oso_objfile) { @@ -1464,8 +1464,8 @@ StatsDuration SymbolFileDWARFDebugMap::GetDebugInfoParseTime() { return elapsed; } -StatsDuration SymbolFileDWARFDebugMap::GetDebugInfoIndexTime() { - StatsDuration elapsed(0.0); +StatsDuration::Duration SymbolFileDWARFDebugMap::GetDebugInfoIndexTime() { + StatsDuration::Duration elapsed(0.0); ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { ObjectFile *oso_objfile = oso_dwarf->GetObjectFile(); if (oso_objfile) { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index 74f32442de2f..2a6232a501b4 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -143,8 +143,8 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFile { llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } uint64_t GetDebugInfoSize() override; - lldb_private::StatsDuration GetDebugInfoParseTime() override; - lldb_private::StatsDuration GetDebugInfoIndexTime() override; + lldb_private::StatsDuration::Duration GetDebugInfoParseTime() override; + lldb_private::StatsDuration::Duration GetDebugInfoIndexTime() override; protected: enum { kHaveInitializedOSOs = (1 << 0), kNumFlags }; diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index d50343fb5a43..8d1e982c3b98 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -34,7 +34,8 @@ json::Value StatsSuccessFail::ToJSON() const { } static double elapsed(const StatsTimepoint &start, const StatsTimepoint &end) { - StatsDuration elapsed = end.time_since_epoch() - start.time_since_epoch(); + StatsDuration::Duration elapsed = + end.time_since_epoch() - start.time_since_epoch(); return elapsed.count(); } @@ -86,7 +87,8 @@ json::Value TargetStats::ToJSON(Target &target) { elapsed(*m_launch_or_attach_time, *m_first_public_stop_time); target_metrics_json.try_emplace("firstStopTime", elapsed_time); } - target_metrics_json.try_emplace("targetCreateTime", m_create_time.count()); + target_metrics_json.try_emplace("targetCreateTime", + m_create_time.get().count()); json::Array breakpoints_array; double totalBreakpointResolveTime = 0.0; @@ -177,8 +179,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(Debugger &debugger, } module_stat.uuid = module->GetUUID().GetAsString(); module_stat.triple = module->GetArchitecture().GetTriple().str(); - module_stat.symtab_parse_time = module->GetSymtabParseTime().count(); - module_stat.symtab_index_time = module->GetSymtabIndexTime().count(); + module_stat.symtab_parse_time = module->GetSymtabParseTime().get().count(); + module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count(); Symtab *symtab = module->GetSymtab(); if (symtab) { module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits