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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
