[Lldb-commits] [lldb] 4f89157 - [lldb] Make StatsDuration thread-safe
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 19 07:43:03 PST 2022
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();
More information about the lldb-commits
mailing list