[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 9 17:36:50 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jacob Lalonde (Jlalond)
<details>
<summary>Changes</summary>
This PR adds a statistics provider cache, which allows an individual target to keep a rolling tally of it's total time and number of invocations for a given summary provider. This information is then available in statistics dump to help slow summary providers, and gleam more into insight into LLDB's time use.
---
Full diff: https://github.com/llvm/llvm-project/pull/102708.diff
8 Files Affected:
- (modified) lldb/include/lldb/Target/Statistics.h (+80)
- (modified) lldb/include/lldb/Target/Target.h (+5)
- (modified) lldb/source/Core/ValueObject.cpp (+10-1)
- (modified) lldb/source/Target/Statistics.cpp (+22)
- (modified) lldb/source/Target/Target.cpp (+8)
- (modified) lldb/test/API/commands/statistics/basic/Makefile (+1-1)
- (modified) lldb/test/API/commands/statistics/basic/TestStats.py (+25-2)
- (renamed) lldb/test/API/commands/statistics/basic/main.cpp (+7-1)
``````````diff
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 35bd7f8a66e055..06ca5c7923f747 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -16,6 +16,7 @@
#include "llvm/Support/JSON.h"
#include <atomic>
#include <chrono>
+#include <mutex>
#include <optional>
#include <ratio>
#include <string>
@@ -25,6 +26,7 @@ namespace lldb_private {
using StatsClock = std::chrono::high_resolution_clock;
using StatsTimepoint = std::chrono::time_point<StatsClock>;
+using Duration = std::chrono::duration<double>;
class StatsDuration {
public:
@@ -174,6 +176,83 @@ struct StatisticsOptions {
std::optional<bool> m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ SummaryStatistics() = default;
+ SummaryStatistics(lldb_private::ConstString name) :
+ m_total_time(), m_name(name), m_summary_count(0) {}
+
+ SummaryStatistics(const SummaryStatistics &&rhs)
+ : m_total_time(), m_name(rhs.m_name), m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+ lldb_private::ConstString GetName() const { return m_name; };
+ double GetAverageTime() const {
+ return m_total_time.get().count() / m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ double GetTotalTime() const {
+ return m_total_time.get().count();
+ }
+
+ uint64_t GetSummaryCount() const {
+ return m_summary_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration& GetDurationReference() {
+ return m_total_time;
+ }
+
+ llvm::json::Value ToJSON() const;
+
+ // Basic RAII class to increment the summary count when the call is complete.
+ // In the future this can be extended to collect information about the
+ // elapsed time for a single request.
+ class SummaryInvocation {
+ public:
+ SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+ ~SummaryInvocation() {
+ m_summary.OnInvoked();
+ }
+ private:
+ SummaryStatistics &m_summary;
+ };
+
+private:
+ /// Called when Summary Invocation is destructed.
+ void OnInvoked() {
+ m_summary_count.fetch_add(1, std::memory_order_relaxed);
+ }
+
+ lldb_private::StatsDuration m_total_time;
+ lldb_private::ConstString m_name;
+ std::atomic<uint64_t> m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+ SummaryStatisticsCache() = default;
+ /// Get the SummaryStatistics object for a given provider name, or insert
+ /// if statistics for that provider is not in the map.
+ lldb_private::SummaryStatistics &GetSummaryStatisticsForProviderName(lldb_private::ConstString summary_provider_name) {
+ m_map_mutex.lock();
+ if (m_summary_stats_map.count(summary_provider_name) == 0) {
+ m_summary_stats_map.emplace(summary_provider_name, SummaryStatistics(summary_provider_name));
+ }
+
+ SummaryStatistics &summary_stats = m_summary_stats_map.at(summary_provider_name);
+ m_map_mutex.unlock();
+ return summary_stats;
+ }
+
+ llvm::json::Value ToJSON();
+
+private:
+ std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> m_summary_stats_map;
+ std::mutex m_map_mutex;
+};
+
/// A class that represents statistics for a since lldb_private::Target.
class TargetStats {
public:
@@ -198,6 +277,7 @@ class TargetStats {
StatsSuccessFail m_frame_var{"frameVariable"};
std::vector<intptr_t> m_module_identifiers;
uint32_t m_source_map_deduce_count = 0;
+ SummaryStatisticsCache m_summary_stats_cache;
void CollectStats(Target &target);
};
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ae1ea43c01003b 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,7 @@ class TargetProperties : public Properties {
bool GetDebugUtilityExpression() const;
+
private:
std::optional<bool>
GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1222,9 @@ class Target : public std::enable_shared_from_this<Target>,
void ClearAllLoadedSections();
+ lldb_private::SummaryStatistics& GetSummaryStatisticsForProvider(lldb_private::ConstString summary_provider_name);
+ lldb_private::SummaryStatisticsCache& GetSummaryStatisticsCache();
+
/// Set the \a Trace object containing processor trace information of this
/// target.
///
@@ -1554,6 +1558,7 @@ class Target : public std::enable_shared_from_this<Target>,
std::string m_label;
ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+ SummaryStatisticsCache m_summary_statistics_cache;
SectionLoadHistory m_section_load_history;
BreakpointList m_breakpoint_list;
BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..8e6ff41c08539f 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,7 +615,16 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
// the synthetic children being
// up-to-date (e.g. ${svar%#})
- summary_ptr->FormatObject(this, destination, actual_options);
+ SummaryStatistics &summary_stats = GetExecutionContextRef()
+ .GetProcessSP()
+ ->GetTarget()
+ .GetSummaryStatisticsForProvider(GetTypeName());
+ /// Construct RAII types to time and collect data on summary creation.
+ SummaryStatistics::SummaryInvocation summary_inv(summary_stats);
+ {
+ ElapsedTime elapsed(summary_stats.GetDurationReference());
+ summary_ptr->FormatObject(this, destination, actual_options);
+ }
}
m_flags.m_is_getting_summary = false;
return !destination.empty();
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 583d1524881fc3..bcb8fdbca42a3c 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -192,6 +192,8 @@ TargetStats::ToJSON(Target &target,
}
target_metrics_json.try_emplace("sourceMapDeduceCount",
m_source_map_deduce_count);
+ target_metrics_json.try_emplace("summaryProviderStatistics",
+ target.GetSummaryStatisticsCache().ToJSON());
return target_metrics_json;
}
@@ -408,3 +410,23 @@ llvm::json::Value DebuggerStats::ReportStatistics(
return std::move(global_stats);
}
+
+llvm::json::Value SummaryStatistics::ToJSON() const {
+ json::Object body {{
+ {"invocationCount", GetSummaryCount()},
+ {"totalTime", GetTotalTime()},
+ {"averageTime", GetAverageTime()}
+ }};
+ return json::Object{{GetName().AsCString(), std::move(body)}};
+}
+
+
+json::Value SummaryStatisticsCache::ToJSON() {
+ m_map_mutex.lock();
+ json::Array json_summary_stats;
+ for (const auto &summary_stat : m_summary_stats_map)
+ json_summary_stats.emplace_back(summary_stat.second.ToJSON());
+
+ m_map_mutex.unlock();
+ return json_summary_stats;
+}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 129683c43f0c1a..834d48763bdff8 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3193,6 +3193,14 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP §ion_sp,
void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); }
+lldb_private::SummaryStatistics& Target::GetSummaryStatisticsForProvider(lldb_private::ConstString summary_provider_name) {
+ return m_summary_statistics_cache.GetSummaryStatisticsForProviderName(summary_provider_name);
+}
+
+lldb_private::SummaryStatisticsCache& Target::GetSummaryStatisticsCache() {
+ return m_summary_statistics_cache;
+}
+
void Target::SaveScriptedLaunchInfo(lldb_private::ProcessInfo &process_info) {
if (process_info.IsScriptedProcess()) {
// Only copy scripted process launch options.
diff --git a/lldb/test/API/commands/statistics/basic/Makefile b/lldb/test/API/commands/statistics/basic/Makefile
index c9319d6e6888a4..3d0b98f13f3d7b 100644
--- a/lldb/test/API/commands/statistics/basic/Makefile
+++ b/lldb/test/API/commands/statistics/basic/Makefile
@@ -1,2 +1,2 @@
-C_SOURCES := main.c
+CXX_SOURCES := main.cpp
include Makefile.rules
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index a8ac60090a760d..85e76e526849ab 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -81,7 +81,7 @@ def get_command_stats(self, debug_stats):
def test_expressions_frame_var_counts(self):
self.build()
lldbutil.run_to_source_breakpoint(
- self, "// break here", lldb.SBFileSpec("main.c")
+ self, "// break here", lldb.SBFileSpec("main.cpp")
)
self.expect("expr patatino", substrs=["27"])
@@ -224,7 +224,7 @@ def test_default_with_run(self):
self.build()
target = self.createTestTarget()
lldbutil.run_to_source_breakpoint(
- self, "// break here", lldb.SBFileSpec("main.c")
+ self, "// break here", lldb.SBFileSpec("main.cpp")
)
debug_stats = self.get_stats()
debug_stat_keys = [
@@ -250,6 +250,7 @@ def test_default_with_run(self):
"launchOrAttachTime",
"moduleIdentifiers",
"targetCreateTime",
+ "summaryProviderStatistics"
]
self.verify_keys(stats, '"stats"', keys_exist, None)
self.assertGreater(stats["firstStopTime"], 0.0)
@@ -447,6 +448,7 @@ def test_breakpoints(self):
"targetCreateTime",
"moduleIdentifiers",
"totalBreakpointResolveTime",
+ "summaryProviderStatistics"
]
self.verify_keys(target_stats, '"stats"', keys_exist, None)
self.assertGreater(target_stats["totalBreakpointResolveTime"], 0.0)
@@ -918,3 +920,24 @@ def test_order_of_options_do_not_matter(self):
debug_stats_1,
f"The order of options '{options[0]}' and '{options[1]}' should not matter",
)
+
+ def test_summary_statistics_providers(self):
+ """
+ Test summary timing statistics is included in statistics dump when
+ a type with a summary provider exists, and is evaluated.
+ """
+
+ self.build()
+ target = self.createTestTarget()
+ lldbutil.run_to_source_breakpoint(
+ self, "// stop here", lldb.SBFileSpec("main.cpp")
+ )
+ self.expect("frame var", substrs=["hello world"])
+ stats = self.get_target_stats(self.get_stats())
+ self.assertIn("summaryProviderStatistics", stats)
+ summary_providers = stats["summaryProviderStatistics"]
+ # We don't want to take a dependency on the type name, so we just look
+ # for string and that it was called once.
+ summary_provider_str = str(summary_providers)
+ self.assertIn("string", summary_provider_str)
+ self.assertIn("'invocationCount': 1", summary_provider_str)
diff --git a/lldb/test/API/commands/statistics/basic/main.c b/lldb/test/API/commands/statistics/basic/main.cpp
similarity index 52%
rename from lldb/test/API/commands/statistics/basic/main.c
rename to lldb/test/API/commands/statistics/basic/main.cpp
index 2c5b4f5f098ee4..3e3f34421eca30 100644
--- a/lldb/test/API/commands/statistics/basic/main.c
+++ b/lldb/test/API/commands/statistics/basic/main.cpp
@@ -1,7 +1,13 @@
// Test that the lldb command `statistics` works.
+#include <string>
+
+void foo() {
+ std::string str = "hello world";
+ str += "\n"; // stop here
+}
int main(void) {
int patatino = 27;
-
+ foo();
return 0; // break here
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/102708
More information about the lldb-commits
mailing list