labath created this revision.
labath added a reviewer: clayborg.
Herald added a subscriber: arphaman.
labath requested review of this revision.
Herald added a project: LLDB.
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>).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D117474
Files:
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
Index: lldb/source/Target/Statistics.cpp
===================================================================
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -34,7 +34,8 @@
}
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 @@
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 @@
}
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();
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -143,8 +143,8 @@
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 };
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1447,8 +1447,8 @@
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 @@
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) {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -319,10 +319,10 @@
/// 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 @@
/// 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
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4086,8 +4086,8 @@
return LanguageTypeFromDWARF(lang);
}
-StatsDuration SymbolFileDWARF::GetDebugInfoIndexTime() {
+StatsDuration::Duration SymbolFileDWARF::GetDebugInfoIndexTime() {
if (m_index)
return m_index->GetIndexTime();
- return StatsDuration(0.0);
+ return {};
}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -64,11 +64,11 @@
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
Index: lldb/source/Breakpoint/Breakpoint.cpp
===================================================================
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1094,7 +1094,7 @@
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());
Index: lldb/include/lldb/Target/Statistics.h
===================================================================
--- lldb/include/lldb/Target/Statistics.h
+++ 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 @@
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 @@
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;
Index: lldb/include/lldb/Symbol/SymbolFile.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFile.h
+++ lldb/include/lldb/Symbol/SymbolFile.h
@@ -316,14 +316,14 @@
///
/// \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.
Index: lldb/include/lldb/Core/Module.h
===================================================================
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -1047,11 +1047,11 @@
/// 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.
///
Index: lldb/include/lldb/Breakpoint/Breakpoint.h
===================================================================
--- lldb/include/lldb/Breakpoint/Breakpoint.h
+++ lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -581,7 +581,7 @@
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 @@
BreakpointName::Permissions m_permissions;
- StatsDuration m_resolve_time{0.0};
+ StatsDuration m_resolve_time;
void SendBreakpointChangedEvent(lldb::BreakpointEventType eventKind);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
