https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/102708
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. >From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Thu, 8 Aug 2024 08:58:52 -0700 Subject: [PATCH 1/3] Initial attempt at new classes Summary statistics in SummaryStatistics.h/cpp. --- lldb/include/lldb/Target/SummaryStatistics.h | 37 ++++++++++++++++++++ lldb/include/lldb/Target/Target.h | 5 +++ lldb/source/Core/ValueObject.cpp | 5 +++ lldb/source/Target/CMakeLists.txt | 1 + lldb/source/Target/SummaryStatistics.cpp | 26 ++++++++++++++ lldb/source/Target/Target.cpp | 9 +++++ 6 files changed, 83 insertions(+) create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h create mode 100644 lldb/source/Target/SummaryStatistics.cpp diff --git a/lldb/include/lldb/Target/SummaryStatistics.h b/lldb/include/lldb/Target/SummaryStatistics.h new file mode 100644 index 00000000000000..0198249ba0b170 --- /dev/null +++ b/lldb/include/lldb/Target/SummaryStatistics.h @@ -0,0 +1,37 @@ +//===-- SummaryStatistics.h -----------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H +#define LLDB_TARGET_SUMMARYSTATISTICS_H + + +#include "lldb/Target/Statistics.h" +#include "llvm/ADT/StringRef.h" + +namespace lldb_private { + +class SummaryStatistics { +public: + SummaryStatistics(lldb_private::ConstString name) : + m_total_time(), m_name(name), m_summary_count(0) {} + + lldb_private::StatsDuration &GetDurationReference(); + + lldb_private::ConstString GetName() const; + + uint64_t GetSummaryCount() const; + +private: + lldb_private::StatsDuration m_total_time; + lldb_private::ConstString m_name; + uint64_t m_summary_count; +}; + +} // namespace lldb_private + +#endif // LLDB_TARGET_SUMMARYSTATISTICS_H diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 119dff4d498199..ee6a009b0af95d 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -30,6 +30,7 @@ #include "lldb/Target/PathMappingList.h" #include "lldb/Target/SectionLoadHistory.h" #include "lldb/Target/Statistics.h" +#include "lldb/Target/SummaryStatistics.h" #include "lldb/Target/ThreadSpec.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/Broadcaster.h" @@ -258,6 +259,7 @@ class TargetProperties : public Properties { bool GetDebugUtilityExpression() const; + private: std::optional<bool> GetExperimentalPropertyValue(size_t prop_idx, @@ -1221,6 +1223,8 @@ class Target : public std::enable_shared_from_this<Target>, void ClearAllLoadedSections(); + lldb_private::StatsDuration& GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name); + /// 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). + std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> m_summary_stats_map; 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..bed4ab8d69cbda 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -615,6 +615,11 @@ 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%#}) + StatsDuration &summary_duration = GetExecutionContextRef() + .GetProcessSP() + ->GetTarget() + .GetSummaryProviderDuration(GetTypeName()); + ElapsedTime elapsed(summary_duration); summary_ptr->FormatObject(this, destination, actual_options); } m_flags.m_is_getting_summary = false; diff --git a/lldb/source/Target/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt index a42c44b761dc56..e51da37cd84db3 100644 --- a/lldb/source/Target/CMakeLists.txt +++ b/lldb/source/Target/CMakeLists.txt @@ -46,6 +46,7 @@ add_lldb_library(lldbTarget Statistics.cpp StopInfo.cpp StructuredDataPlugin.cpp + SummaryStatistics.cpp SystemRuntime.cpp Target.cpp TargetList.cpp diff --git a/lldb/source/Target/SummaryStatistics.cpp b/lldb/source/Target/SummaryStatistics.cpp new file mode 100644 index 00000000000000..62f9776d796341 --- /dev/null +++ b/lldb/source/Target/SummaryStatistics.cpp @@ -0,0 +1,26 @@ +//===-- SummaryStatistics.cpp -----------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Target/SummaryStatistics.h" + +using namespace lldb; +using namespace lldb_private; + + +StatsDuration& SummaryStatistics::GetDurationReference() { + m_summary_count++; + return m_total_time; +} + +ConstString SummaryStatistics::GetName() const { + return m_name; +} + +uint64_t SummaryStatistics::GetSummaryCount() const { + return m_summary_count; +} diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 129683c43f0c1a..0f213579ae6f53 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -3193,6 +3193,15 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP §ion_sp, void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); } +lldb_private::StatsDuration& Target::GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name) { + if (m_summary_stats_map.count(summary_provider_name) == 0) { + SummaryStatistics summary_stats(summary_provider_name); + m_summary_stats_map.emplace(summary_provider_name, SummaryStatistics(summary_provider_name)); + } + + return m_summary_stats_map[summary_provider_name].GetDurationReference(); +} + void Target::SaveScriptedLaunchInfo(lldb_private::ProcessInfo &process_info) { if (process_info.IsScriptedProcess()) { // Only copy scripted process launch options. >From a3d63fe12b1e7f16c95a9e8f8f4fbd96c9e93a5a Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 9 Aug 2024 16:14:24 -0700 Subject: [PATCH 2/3] Implement inovcation timing for summary providers --- lldb/include/lldb/Target/Statistics.h | 80 +++++++++++++++++++ lldb/include/lldb/Target/SummaryStatistics.h | 37 --------- lldb/include/lldb/Target/Target.h | 6 +- lldb/source/Core/ValueObject.cpp | 12 ++- lldb/source/Target/CMakeLists.txt | 1 - lldb/source/Target/Statistics.cpp | 18 +++++ lldb/source/Target/SummaryStatistics.cpp | 26 ------ lldb/source/Target/Target.cpp | 11 ++- .../API/commands/statistics/basic/Makefile | 2 +- .../commands/statistics/basic/TestStats.py | 1 + .../test/API/commands/statistics/basic/main.c | 2 - 11 files changed, 116 insertions(+), 80 deletions(-) delete mode 100644 lldb/include/lldb/Target/SummaryStatistics.h delete mode 100644 lldb/source/Target/SummaryStatistics.cpp diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 35bd7f8a66e055..3444c4673d9f15 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() { 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/SummaryStatistics.h b/lldb/include/lldb/Target/SummaryStatistics.h deleted file mode 100644 index 0198249ba0b170..00000000000000 --- a/lldb/include/lldb/Target/SummaryStatistics.h +++ /dev/null @@ -1,37 +0,0 @@ -//===-- SummaryStatistics.h -----------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H -#define LLDB_TARGET_SUMMARYSTATISTICS_H - - -#include "lldb/Target/Statistics.h" -#include "llvm/ADT/StringRef.h" - -namespace lldb_private { - -class SummaryStatistics { -public: - SummaryStatistics(lldb_private::ConstString name) : - m_total_time(), m_name(name), m_summary_count(0) {} - - lldb_private::StatsDuration &GetDurationReference(); - - lldb_private::ConstString GetName() const; - - uint64_t GetSummaryCount() const; - -private: - lldb_private::StatsDuration m_total_time; - lldb_private::ConstString m_name; - uint64_t m_summary_count; -}; - -} // namespace lldb_private - -#endif // LLDB_TARGET_SUMMARYSTATISTICS_H diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index ee6a009b0af95d..ae1ea43c01003b 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -30,7 +30,6 @@ #include "lldb/Target/PathMappingList.h" #include "lldb/Target/SectionLoadHistory.h" #include "lldb/Target/Statistics.h" -#include "lldb/Target/SummaryStatistics.h" #include "lldb/Target/ThreadSpec.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/Broadcaster.h" @@ -1223,7 +1222,8 @@ class Target : public std::enable_shared_from_this<Target>, void ClearAllLoadedSections(); - lldb_private::StatsDuration& GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name); + 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. @@ -1558,7 +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). - std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> m_summary_stats_map; + 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 bed4ab8d69cbda..8e6ff41c08539f 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -615,12 +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%#}) - StatsDuration &summary_duration = GetExecutionContextRef() + SummaryStatistics &summary_stats = GetExecutionContextRef() .GetProcessSP() ->GetTarget() - .GetSummaryProviderDuration(GetTypeName()); - ElapsedTime elapsed(summary_duration); - summary_ptr->FormatObject(this, destination, actual_options); + .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/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt index e51da37cd84db3..a42c44b761dc56 100644 --- a/lldb/source/Target/CMakeLists.txt +++ b/lldb/source/Target/CMakeLists.txt @@ -46,7 +46,6 @@ add_lldb_library(lldbTarget Statistics.cpp StopInfo.cpp StructuredDataPlugin.cpp - SummaryStatistics.cpp SystemRuntime.cpp Target.cpp TargetList.cpp diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 583d1524881fc3..626214cb7a3187 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -354,6 +354,7 @@ llvm::json::Value DebuggerStats::ReportStatistics( if (include_targets) { if (target) { json_targets.emplace_back(target->ReportStatistics(options)); + json_targets.emplace_back(target->GetSummaryStatisticsCache().ToJSON()); } else { for (const auto &target : debugger.GetTargetList().Targets()) json_targets.emplace_back(target->ReportStatistics(options)); @@ -408,3 +409,20 @@ llvm::json::Value DebuggerStats::ReportStatistics( return std::move(global_stats); } + +llvm::json::Value SummaryStatistics::ToJSON() const { + return json::Object { + {"invocationCount", GetSummaryCount()}, + {"totalTime", GetTotalTime()}, + {"averageTime", GetAverageTime()} + }; +} + +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::Object{{"summaryProviderStatistics", std::move(json_summary_stats)}}; +} diff --git a/lldb/source/Target/SummaryStatistics.cpp b/lldb/source/Target/SummaryStatistics.cpp deleted file mode 100644 index 62f9776d796341..00000000000000 --- a/lldb/source/Target/SummaryStatistics.cpp +++ /dev/null @@ -1,26 +0,0 @@ -//===-- SummaryStatistics.cpp -----------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "lldb/Target/SummaryStatistics.h" - -using namespace lldb; -using namespace lldb_private; - - -StatsDuration& SummaryStatistics::GetDurationReference() { - m_summary_count++; - return m_total_time; -} - -ConstString SummaryStatistics::GetName() const { - return m_name; -} - -uint64_t SummaryStatistics::GetSummaryCount() const { - return m_summary_count; -} diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 0f213579ae6f53..834d48763bdff8 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -3193,13 +3193,12 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP §ion_sp, void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); } -lldb_private::StatsDuration& Target::GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name) { - if (m_summary_stats_map.count(summary_provider_name) == 0) { - SummaryStatistics summary_stats(summary_provider_name); - m_summary_stats_map.emplace(summary_provider_name, SummaryStatistics(summary_provider_name)); - } +lldb_private::SummaryStatistics& Target::GetSummaryStatisticsForProvider(lldb_private::ConstString summary_provider_name) { + return m_summary_statistics_cache.GetSummaryStatisticsForProviderName(summary_provider_name); +} - return m_summary_stats_map[summary_provider_name].GetDurationReference(); +lldb_private::SummaryStatisticsCache& Target::GetSummaryStatisticsCache() { + return m_summary_statistics_cache; } void Target::SaveScriptedLaunchInfo(lldb_private::ProcessInfo &process_info) { 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..0a435efbf0a1a0 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -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) diff --git a/lldb/test/API/commands/statistics/basic/main.c b/lldb/test/API/commands/statistics/basic/main.c index 2c5b4f5f098ee4..85c5654d38ff74 100644 --- a/lldb/test/API/commands/statistics/basic/main.c +++ b/lldb/test/API/commands/statistics/basic/main.c @@ -1,7 +1,5 @@ // Test that the lldb command `statistics` works. - int main(void) { int patatino = 27; - return 0; // break here } >From ad4b20d2074203fce9e4c47fd6045699b1528d86 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Fri, 9 Aug 2024 16:54:37 -0700 Subject: [PATCH 3/3] Sprinkle new entry into tests where there is nothing in the array, and add a dedicated test for strings. Migrate main.c to main.cpp to facilitate this. --- lldb/include/lldb/Target/Statistics.h | 2 +- lldb/source/Target/Statistics.cpp | 18 ++++++++----- .../commands/statistics/basic/TestStats.py | 26 +++++++++++++++++-- .../statistics/basic/{main.c => main.cpp} | 8 ++++++ 4 files changed, 44 insertions(+), 10 deletions(-) rename lldb/test/API/commands/statistics/basic/{main.c => main.cpp} (52%) diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 3444c4673d9f15..06ca5c7923f747 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -186,7 +186,7 @@ class SummaryStatistics { 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() { return m_name; }; + 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); } diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 626214cb7a3187..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; } @@ -354,7 +356,6 @@ llvm::json::Value DebuggerStats::ReportStatistics( if (include_targets) { if (target) { json_targets.emplace_back(target->ReportStatistics(options)); - json_targets.emplace_back(target->GetSummaryStatisticsCache().ToJSON()); } else { for (const auto &target : debugger.GetTargetList().Targets()) json_targets.emplace_back(target->ReportStatistics(options)); @@ -411,18 +412,21 @@ llvm::json::Value DebuggerStats::ReportStatistics( } llvm::json::Value SummaryStatistics::ToJSON() const { - return json::Object { - {"invocationCount", GetSummaryCount()}, - {"totalTime", GetTotalTime()}, - {"averageTime", GetAverageTime()} - }; + 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::Object{{"summaryProviderStatistics", std::move(json_summary_stats)}}; + return json_summary_stats; } diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index 0a435efbf0a1a0..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 = [ @@ -448,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) @@ -919,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 85c5654d38ff74..3e3f34421eca30 100644 --- a/lldb/test/API/commands/statistics/basic/main.c +++ b/lldb/test/API/commands/statistics/basic/main.cpp @@ -1,5 +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 } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits