https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/102708
>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/9] 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/9] 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/9] 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 } >From c304b60f14b519d01b4544f129503a2002b9b6d5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 13 Aug 2024 17:24:49 -0700 Subject: [PATCH 4/9] Added more tests for python script provider and added vector as a test case. Added a type field to the typesummaryimplmpl. Made the cache use lock guards, and take a reference to the typeimpl instead of the conststring name. Drop average time --- .../include/lldb/DataFormatters/TypeSummary.h | 15 ++++ lldb/include/lldb/Target/Statistics.h | 68 ++++++++++--------- lldb/include/lldb/Target/Target.h | 6 +- lldb/source/Core/ValueObject.cpp | 21 +++--- lldb/source/DataFormatters/TypeSummary.cpp | 35 +++++++++- lldb/source/Target/Statistics.cpp | 16 ++--- lldb/source/Target/Target.cpp | 8 ++- .../commands/statistics/basic/BoxFormatter.py | 15 ++++ .../commands/statistics/basic/TestStats.py | 45 ++++++++++-- .../API/commands/statistics/basic/main.cpp | 22 ++++++ 10 files changed, 187 insertions(+), 64 deletions(-) create mode 100644 lldb/test/API/commands/statistics/basic/BoxFormatter.py diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h index a82641021dad36..4fb3d78a456218 100644 --- a/lldb/include/lldb/DataFormatters/TypeSummary.h +++ b/lldb/include/lldb/DataFormatters/TypeSummary.h @@ -258,6 +258,9 @@ class TypeSummaryImpl { virtual std::string GetDescription() = 0; + virtual ConstString GetName() = 0; + virtual ConstString GetImplType() = 0; + uint32_t &GetRevision() { return m_my_revision; } typedef std::shared_ptr<TypeSummaryImpl> SharedPointer; @@ -293,13 +296,18 @@ struct StringSummaryFormat : public TypeSummaryImpl { std::string GetDescription() override; + ConstString GetName() override; + ConstString GetImplType() override; + static bool classof(const TypeSummaryImpl *S) { return S->GetKind() == Kind::eSummaryString; } private: + void SetProviderName(const char *f); StringSummaryFormat(const StringSummaryFormat &) = delete; const StringSummaryFormat &operator=(const StringSummaryFormat &) = delete; + ConstString m_provider_name; }; // summaries implemented via a C++ function @@ -340,6 +348,9 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl { return S->GetKind() == Kind::eCallback; } + ConstString GetName() override; + ConstString GetImplType() override; + typedef std::shared_ptr<CXXFunctionSummaryFormat> SharedPointer; private: @@ -352,6 +363,7 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl { struct ScriptSummaryFormat : public TypeSummaryImpl { std::string m_function_name; std::string m_python_script; + ConstString m_script_formatter_name; StructuredData::ObjectSP m_script_function_sp; ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, @@ -384,6 +396,9 @@ struct ScriptSummaryFormat : public TypeSummaryImpl { std::string GetDescription() override; + ConstString GetName() override; + ConstString GetImplType() override; + static bool classof(const TypeSummaryImpl *S) { return S->GetKind() == Kind::eScript; } diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 06ca5c7923f747..310f7ee13657e9 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -9,6 +9,7 @@ #ifndef LLDB_TARGET_STATISTICS_H #define LLDB_TARGET_STATISTICS_H +#include "lldb/DataFormatters/TypeSummary.h" #include "lldb/Utility/ConstString.h" #include "lldb/Utility/Stream.h" #include "lldb/lldb-forward.h" @@ -179,52 +180,53 @@ struct StatisticsOptions { /// 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)) {} + explicit SummaryStatistics(lldb_private::ConstString name, + lldb_private::ConstString impl_type) + : m_total_time(), m_impl_type(impl_type), m_name(name), + m_summary_count(0) {} 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(); - } + 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; - } + StatsDuration &GetDurationReference() { return m_total_time; } + + ConstString GetImplType() const { return m_impl_type; } 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 + // 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(); - } + SummaryInvocation(SummaryStatistics &summary) + : m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {} + ~SummaryInvocation() { m_summary.OnInvoked(); } + + // Delete the copy constructor and assignment operator to prevent + // accidental double counting. + SummaryInvocation(const SummaryInvocation &) = delete; + SummaryInvocation &operator=(const SummaryInvocation &) = delete; + private: SummaryStatistics &m_summary; + ElapsedTime m_elapsed_time; }; + SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); } + private: /// Called when Summary Invocation is destructed. - void OnInvoked() { + void OnInvoked() noexcept { m_summary_count.fetch_add(1, std::memory_order_relaxed); } lldb_private::StatsDuration m_total_time; + lldb_private::ConstString m_impl_type; lldb_private::ConstString m_name; std::atomic<uint64_t> m_summary_count; }; @@ -232,25 +234,25 @@ class SummaryStatistics { /// 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(); + lldb_private::SummaryStatistics & + GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) { + std::lock_guard<std::recursive_mutex> guard(m_map_mutex); + m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(), + provider.GetImplType()); + + SummaryStatistics &summary_stats = + m_summary_stats_map.at(provider.GetName()); 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; + std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> + m_summary_stats_map; + std::recursive_mutex m_map_mutex; }; /// A class that represents statistics for a since lldb_private::Target. diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index ae1ea43c01003b..8305897bfb084e 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -258,7 +258,6 @@ class TargetProperties : public Properties { bool GetDebugUtilityExpression() const; - private: std::optional<bool> GetExperimentalPropertyValue(size_t prop_idx, @@ -1222,8 +1221,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(); + lldb_private::SummaryStatistics & + GetSummaryStatisticsForProvider(lldb_private::TypeSummaryImpl &provider); + lldb_private::SummaryStatisticsCache &GetSummaryStatisticsCache(); /// Set the \a Trace object containing processor trace information of this /// target. diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index 8e6ff41c08539f..288d055be26576 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -602,7 +602,7 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr, actual_options.SetLanguage(GetPreferredDisplayLanguage()); // this is a hot path in code and we prefer to avoid setting this string all - // too often also clearing out other information that we might care to see in + // too often also clearing out other information that we might care` to see in // a crash log. might be useful in very specific situations though. /*Host::SetCrashDescriptionWithFormat("Trying to fetch a summary for %s %s. Summary provider's description is %s", @@ -615,16 +615,17 @@ 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%#}) - 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()); + + TargetSP target = GetExecutionContextRef().GetTargetSP(); + if (target) { + SummaryStatistics &summary_stats = + target->GetSummaryStatisticsForProvider(*summary_ptr); + /// Construct RAII types to time and collect data on summary creation. + SummaryStatistics::SummaryInvocation summary_inv = + summary_stats.GetSummaryInvocation(); + summary_ptr->FormatObject(this, destination, actual_options); + } else summary_ptr->FormatObject(this, destination, actual_options); - } } m_flags.m_is_getting_summary = false; return !destination.empty(); diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index 3707d2d879d33a..6d8c9ea302d9f2 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -116,6 +116,9 @@ std::string StringSummaryFormat::GetDescription() { return std::string(sstr.GetString()); } +ConstString StringSummaryFormat::GetName() { return ConstString(m_format_str); } +ConstString StringSummaryFormat::GetImplType() { return ConstString("string"); } + CXXFunctionSummaryFormat::CXXFunctionSummaryFormat( const TypeSummaryImpl::Flags &flags, Callback impl, const char *description) : TypeSummaryImpl(Kind::eCallback, flags), m_impl(impl), @@ -145,15 +148,34 @@ std::string CXXFunctionSummaryFormat::GetDescription() { return std::string(sstr.GetString()); } +ConstString CXXFunctionSummaryFormat::GetName() { + return ConstString(m_description); +} + +ConstString CXXFunctionSummaryFormat::GetImplType() { + return ConstString("c++"); +} + ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *function_name, const char *python_script) : TypeSummaryImpl(Kind::eScript, flags), m_function_name(), m_python_script(), m_script_function_sp() { - if (function_name) + std::string sstring; + // Take preference in the python script name over the function name. + if (function_name) { + sstring.assign(function_name); m_function_name.assign(function_name); - if (python_script) + } + if (python_script) { + sstring.assign(python_script); m_python_script.assign(python_script); + } + + // Python scripts include their leading spacing, so we remove it so we don't + // save extra spaces in the const string forever. + sstring.erase(0, sstring.find_first_not_of('0')); + m_script_formatter_name = ConstString(sstring); } bool ScriptSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, @@ -201,3 +223,12 @@ std::string ScriptSummaryFormat::GetDescription() { } return std::string(sstr.GetString()); } + +ConstString ScriptSummaryFormat::GetName() { return m_script_formatter_name; } + +ConstString ScriptSummaryFormat::GetImplType() { + if (!m_python_script.empty()) + return ConstString("python"); + + return ConstString("script"); +} diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index bcb8fdbca42a3c..a0f242698802f4 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -192,7 +192,7 @@ TargetStats::ToJSON(Target &target, } target_metrics_json.try_emplace("sourceMapDeduceCount", m_source_map_deduce_count); - target_metrics_json.try_emplace("summaryProviderStatistics", + target_metrics_json.try_emplace("summaryProviderStatistics", target.GetSummaryStatisticsCache().ToJSON()); return target_metrics_json; } @@ -412,21 +412,19 @@ llvm::json::Value DebuggerStats::ReportStatistics( } llvm::json::Value SummaryStatistics::ToJSON() const { - json::Object body {{ + return json::Object{{ + {"name", GetName().AsCString()}, + {"type", GetImplType().AsCString()}, {"invocationCount", GetSummaryCount()}, {"totalTime", GetTotalTime()}, - {"averageTime", GetAverageTime()} - }}; - return json::Object{{GetName().AsCString(), std::move(body)}}; + }}; } - json::Value SummaryStatisticsCache::ToJSON() { - m_map_mutex.lock(); + std::lock_guard<std::recursive_mutex> guard(m_map_mutex); 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 834d48763bdff8..4358f032024a1c 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -3193,11 +3193,13 @@ 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); +SummaryStatistics & +Target::GetSummaryStatisticsForProvider(TypeSummaryImpl &provider) { + return m_summary_statistics_cache.GetSummaryStatisticsForProviderName( + provider); } -lldb_private::SummaryStatisticsCache& Target::GetSummaryStatisticsCache() { +SummaryStatisticsCache &Target::GetSummaryStatisticsCache() { return m_summary_statistics_cache; } diff --git a/lldb/test/API/commands/statistics/basic/BoxFormatter.py b/lldb/test/API/commands/statistics/basic/BoxFormatter.py new file mode 100644 index 00000000000000..07681b32dfd090 --- /dev/null +++ b/lldb/test/API/commands/statistics/basic/BoxFormatter.py @@ -0,0 +1,15 @@ +import lldb + + +def summary(valobj, dict): + return f"[{valobj.GetChildAtIndex(0).GetValue()}]" + + +def __lldb_init_module(debugger, dict): + typeName = "Box<.*$" + debugger.HandleCommand( + 'type summary add -x "' + + typeName + + '" --python-function ' + + f"{__name__}.summary" + ) diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index 85e76e526849ab..715219d9790104 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -250,7 +250,7 @@ def test_default_with_run(self): "launchOrAttachTime", "moduleIdentifiers", "targetCreateTime", - "summaryProviderStatistics" + "summaryProviderStatistics", ] self.verify_keys(stats, '"stats"', keys_exist, None) self.assertGreater(stats["firstStopTime"], 0.0) @@ -448,7 +448,7 @@ def test_breakpoints(self): "targetCreateTime", "moduleIdentifiers", "totalBreakpointResolveTime", - "summaryProviderStatistics" + "summaryProviderStatistics", ] self.verify_keys(target_stats, '"stats"', keys_exist, None) self.assertGreater(target_stats["totalBreakpointResolveTime"], 0.0) @@ -920,10 +920,10 @@ 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 + Test summary timing statistics is included in statistics dump when a type with a summary provider exists, and is evaluated. """ @@ -941,3 +941,40 @@ def test_summary_statistics_providers(self): summary_provider_str = str(summary_providers) self.assertIn("string", summary_provider_str) self.assertIn("'invocationCount': 1", summary_provider_str) + self.assertIn("'totalTime':", summary_provider_str) + self.assertIn("'type': 'c++'", summary_provider_str) + + self.runCmd("continue") + self.runCmd("command script import BoxFormatter.py") + self.expect("frame var", substrs=["box = [27]"]) + stats = self.get_target_stats(self.get_stats()) + self.assertIn("summaryProviderStatistics", stats) + summary_providers = stats["summaryProviderStatistics"] + summary_provider_str = str(summary_providers) + self.assertRegex("'name': 'BoxFormatter.summary'", summary_provider_str) + self.assertIn("'invocationCount': 1", summary_provider_str) + self.assertIn("'totalTime':", summary_provider_str) + self.assertIn("'type': 'python'", summary_provider_str) + + def test_summary_statistics_providers_vec(self): + """ + Test summary timing statistics is included in statistics dump when + a type with a summary provider exists, and is evaluated. This variation + tests that vector recurses into it's child type. + """ + self.build() + target = self.createTestTarget() + lldbutil.run_to_source_breakpoint( + self, "// stop vector", lldb.SBFileSpec("main.cpp") + ) + self.expect( + "frame var", substrs=["int_vec", "double_vec", "[0] = 1", "[7] = 8"] + ) + stats = self.get_target_stats(self.get_stats()) + self.assertIn("summaryProviderStatistics", stats) + summary_providers = stats["summaryProviderStatistics"] + summary_provider_str = str(summary_providers) + self.assertIn("std::vector", summary_provider_str) + self.assertIn("'invocationCount': 2", summary_provider_str) + self.assertIn("'totalTime':", summary_provider_str) + self.assertIn("'type': 'c++'", summary_provider_str) diff --git a/lldb/test/API/commands/statistics/basic/main.cpp b/lldb/test/API/commands/statistics/basic/main.cpp index 3e3f34421eca30..321d4b5034393e 100644 --- a/lldb/test/API/commands/statistics/basic/main.cpp +++ b/lldb/test/API/commands/statistics/basic/main.cpp @@ -1,13 +1,35 @@ // Test that the lldb command `statistics` works. #include <string> +#include <vector> + +template <typename T> class Box { + T m_value; + +public: + Box(T value) : m_value(value) {} +}; void foo() { std::string str = "hello world"; str += "\n"; // stop here } +void bar(int x) { + auto box = Box<int>(x); + // stop here +} + +void vec() { + std::vector<int> int_vec = {1, 2, 3, 4, 5, 6, 7, 8}; + std::vector<double> double_vec = {1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0}; + // stop vector + int x = int_vec.size(); +} + int main(void) { int patatino = 27; foo(); + bar(patatino); + vec(); return 0; // break here } >From f0ed5b0dcb00729ff86b93d4a5d039561ce6ef78 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 13 Aug 2024 17:26:16 -0700 Subject: [PATCH 5/9] Include cleanup of unused method in typesummary.h --- lldb/include/lldb/DataFormatters/TypeSummary.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h index 4fb3d78a456218..b59ed424d8ebd5 100644 --- a/lldb/include/lldb/DataFormatters/TypeSummary.h +++ b/lldb/include/lldb/DataFormatters/TypeSummary.h @@ -304,7 +304,6 @@ struct StringSummaryFormat : public TypeSummaryImpl { } private: - void SetProviderName(const char *f); StringSummaryFormat(const StringSummaryFormat &) = delete; const StringSummaryFormat &operator=(const StringSummaryFormat &) = delete; ConstString m_provider_name; >From bc592c659cdbe2d2500786ed088715095ae4594d Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 13 Aug 2024 17:32:17 -0700 Subject: [PATCH 6/9] Remove the empty line that keeps getting included --- lldb/include/lldb/Target/Target.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 8305897bfb084e..fe547b1552470b 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -257,7 +257,7 @@ class TargetProperties : public Properties { void SetDebugUtilityExpression(bool debug); bool GetDebugUtilityExpression() const; - + private: std::optional<bool> GetExperimentalPropertyValue(size_t prop_idx, >From 7b4b7fcf2fc43479bf601db99e6117c5c7731706 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 14 Aug 2024 09:53:57 -0700 Subject: [PATCH 7/9] Move to std::string vs const string, and have the cache directly give out RAII objects --- .../include/lldb/DataFormatters/TypeSummary.h | 19 ++++++------- lldb/include/lldb/Target/Statistics.h | 28 +++++++++---------- lldb/include/lldb/Target/Target.h | 2 +- lldb/source/Core/ValueObject.cpp | 6 ++-- lldb/source/DataFormatters/TypeSummary.cpp | 24 ++++++++-------- lldb/source/Target/Statistics.cpp | 6 ++-- lldb/source/Target/Target.cpp | 2 +- 7 files changed, 41 insertions(+), 46 deletions(-) diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h index b59ed424d8ebd5..8a3562cedf8fe5 100644 --- a/lldb/include/lldb/DataFormatters/TypeSummary.h +++ b/lldb/include/lldb/DataFormatters/TypeSummary.h @@ -258,8 +258,8 @@ class TypeSummaryImpl { virtual std::string GetDescription() = 0; - virtual ConstString GetName() = 0; - virtual ConstString GetImplType() = 0; + virtual std::string GetName() = 0; + virtual std::string GetSummaryKindName() = 0; uint32_t &GetRevision() { return m_my_revision; } @@ -296,8 +296,8 @@ struct StringSummaryFormat : public TypeSummaryImpl { std::string GetDescription() override; - ConstString GetName() override; - ConstString GetImplType() override; + std::string GetName() override; + std::string GetSummaryKindName() override; static bool classof(const TypeSummaryImpl *S) { return S->GetKind() == Kind::eSummaryString; @@ -306,7 +306,6 @@ struct StringSummaryFormat : public TypeSummaryImpl { private: StringSummaryFormat(const StringSummaryFormat &) = delete; const StringSummaryFormat &operator=(const StringSummaryFormat &) = delete; - ConstString m_provider_name; }; // summaries implemented via a C++ function @@ -347,8 +346,8 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl { return S->GetKind() == Kind::eCallback; } - ConstString GetName() override; - ConstString GetImplType() override; + std::string GetName() override; + std::string GetSummaryKindName() override; typedef std::shared_ptr<CXXFunctionSummaryFormat> SharedPointer; @@ -362,7 +361,7 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl { struct ScriptSummaryFormat : public TypeSummaryImpl { std::string m_function_name; std::string m_python_script; - ConstString m_script_formatter_name; + std::string m_script_formatter_name; StructuredData::ObjectSP m_script_function_sp; ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, @@ -395,8 +394,8 @@ struct ScriptSummaryFormat : public TypeSummaryImpl { std::string GetDescription() override; - ConstString GetName() override; - ConstString GetImplType() override; + std::string GetName() override; + std::string GetSummaryKindName() override; static bool classof(const TypeSummaryImpl *S) { return S->GetKind() == Kind::eScript; diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 310f7ee13657e9..05e7ad7704d316 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -180,12 +180,12 @@ struct StatisticsOptions { /// A class that represents statistics about a TypeSummaryProviders invocations class SummaryStatistics { public: - explicit SummaryStatistics(lldb_private::ConstString name, - lldb_private::ConstString impl_type) + explicit SummaryStatistics(std::string name, + std::string impl_type) : m_total_time(), m_impl_type(impl_type), m_name(name), m_summary_count(0) {} - lldb_private::ConstString GetName() const { return m_name; }; + std::string GetName() const { return m_name; }; double GetTotalTime() const { return m_total_time.get().count(); } uint64_t GetSummaryCount() const { @@ -194,7 +194,7 @@ class SummaryStatistics { StatsDuration &GetDurationReference() { return m_total_time; } - ConstString GetImplType() const { return m_impl_type; } + std::string GetSummaryKindName() const { return m_impl_type; } llvm::json::Value ToJSON() const; @@ -226,8 +226,8 @@ class SummaryStatistics { } lldb_private::StatsDuration m_total_time; - lldb_private::ConstString m_impl_type; - lldb_private::ConstString m_name; + std::string m_impl_type; + std::string m_name; std::atomic<uint64_t> m_summary_count; }; @@ -236,23 +236,21 @@ class SummaryStatisticsCache { public: /// Get the SummaryStatistics object for a given provider name, or insert /// if statistics for that provider is not in the map. - lldb_private::SummaryStatistics & + SummaryStatistics::SummaryInvocation GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) { - std::lock_guard<std::recursive_mutex> guard(m_map_mutex); - m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(), - provider.GetImplType()); + std::lock_guard<std::mutex> guard(m_map_mutex); + auto pair = m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(), + provider.GetSummaryKindName()); - SummaryStatistics &summary_stats = - m_summary_stats_map.at(provider.GetName()); - return summary_stats; + return pair.first->second.GetSummaryInvocation(); } llvm::json::Value ToJSON(); private: - std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> + std::map<std::string, lldb_private::SummaryStatistics> m_summary_stats_map; - std::recursive_mutex m_map_mutex; + std::mutex m_map_mutex; }; /// A class that represents statistics for a since lldb_private::Target. diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index fe547b1552470b..e9d4456da3ff7a 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1221,7 +1221,7 @@ class Target : public std::enable_shared_from_this<Target>, void ClearAllLoadedSections(); - lldb_private::SummaryStatistics & + lldb_private::SummaryStatistics::SummaryInvocation GetSummaryStatisticsForProvider(lldb_private::TypeSummaryImpl &provider); lldb_private::SummaryStatisticsCache &GetSummaryStatisticsCache(); diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index 288d055be26576..c991018bbc05c6 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -602,7 +602,7 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr, actual_options.SetLanguage(GetPreferredDisplayLanguage()); // this is a hot path in code and we prefer to avoid setting this string all - // too often also clearing out other information that we might care` to see in + // too often also clearing out other information that we might care to see in // a crash log. might be useful in very specific situations though. /*Host::SetCrashDescriptionWithFormat("Trying to fetch a summary for %s %s. Summary provider's description is %s", @@ -618,11 +618,9 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr, TargetSP target = GetExecutionContextRef().GetTargetSP(); if (target) { - SummaryStatistics &summary_stats = - target->GetSummaryStatisticsForProvider(*summary_ptr); /// Construct RAII types to time and collect data on summary creation. SummaryStatistics::SummaryInvocation summary_inv = - summary_stats.GetSummaryInvocation(); + target->GetSummaryStatisticsForProvider(*summary_ptr); summary_ptr->FormatObject(this, destination, actual_options); } else summary_ptr->FormatObject(this, destination, actual_options); diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index 6d8c9ea302d9f2..4778688e94b5cf 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -116,8 +116,8 @@ std::string StringSummaryFormat::GetDescription() { return std::string(sstr.GetString()); } -ConstString StringSummaryFormat::GetName() { return ConstString(m_format_str); } -ConstString StringSummaryFormat::GetImplType() { return ConstString("string"); } +std::string StringSummaryFormat::GetName() { return m_format_str; } +std::string StringSummaryFormat::GetSummaryKindName() { return "string"; } CXXFunctionSummaryFormat::CXXFunctionSummaryFormat( const TypeSummaryImpl::Flags &flags, Callback impl, const char *description) @@ -148,12 +148,12 @@ std::string CXXFunctionSummaryFormat::GetDescription() { return std::string(sstr.GetString()); } -ConstString CXXFunctionSummaryFormat::GetName() { - return ConstString(m_description); +std::string CXXFunctionSummaryFormat::GetName() { + return m_description; } -ConstString CXXFunctionSummaryFormat::GetImplType() { - return ConstString("c++"); +std::string CXXFunctionSummaryFormat::GetSummaryKindName() { + return "c++"; } ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, @@ -174,8 +174,8 @@ ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, // Python scripts include their leading spacing, so we remove it so we don't // save extra spaces in the const string forever. - sstring.erase(0, sstring.find_first_not_of('0')); - m_script_formatter_name = ConstString(sstring); + sstring.erase(0, sstring.find_first_not_of(' ')); + m_script_formatter_name = sstring; } bool ScriptSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, @@ -224,11 +224,11 @@ std::string ScriptSummaryFormat::GetDescription() { return std::string(sstr.GetString()); } -ConstString ScriptSummaryFormat::GetName() { return m_script_formatter_name; } +std::string ScriptSummaryFormat::GetName() { return m_script_formatter_name; } -ConstString ScriptSummaryFormat::GetImplType() { +std::string ScriptSummaryFormat::GetSummaryKindName() { if (!m_python_script.empty()) - return ConstString("python"); + return "python"; - return ConstString("script"); + return "script"; } diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index a0f242698802f4..cf36a1c90d38a7 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -413,15 +413,15 @@ llvm::json::Value DebuggerStats::ReportStatistics( llvm::json::Value SummaryStatistics::ToJSON() const { return json::Object{{ - {"name", GetName().AsCString()}, - {"type", GetImplType().AsCString()}, + {"name", GetName()}, + {"type", GetSummaryKindName()}, {"invocationCount", GetSummaryCount()}, {"totalTime", GetTotalTime()}, }}; } json::Value SummaryStatisticsCache::ToJSON() { - std::lock_guard<std::recursive_mutex> guard(m_map_mutex); + std::lock_guard<std::mutex> guard(m_map_mutex); json::Array json_summary_stats; for (const auto &summary_stat : m_summary_stats_map) json_summary_stats.emplace_back(summary_stat.second.ToJSON()); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 4358f032024a1c..c1ca442b6c07b7 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -3193,7 +3193,7 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP §ion_sp, void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); } -SummaryStatistics & +SummaryStatistics::SummaryInvocation Target::GetSummaryStatisticsForProvider(TypeSummaryImpl &provider) { return m_summary_statistics_cache.GetSummaryStatisticsForProviderName( provider); >From 08f9ecbb0d1772d56db93d4b0048fe7016e5eec3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 14 Aug 2024 14:50:05 -0700 Subject: [PATCH 8/9] Switch to string map, set the summary string directly for script summarizers, remove extra lines and change the target method to just return a cache reference --- lldb/include/lldb/Target/Statistics.h | 65 ++++++++++--------- lldb/include/lldb/Target/Target.h | 4 +- lldb/source/Core/ValueObject.cpp | 4 +- lldb/source/DataFormatters/TypeSummary.cpp | 12 ++-- lldb/source/Target/Target.cpp | 6 -- .../commands/statistics/basic/BoxFormatter.py | 2 - 6 files changed, 42 insertions(+), 51 deletions(-) diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 05e7ad7704d316..80609599559652 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -192,39 +192,17 @@ class SummaryStatistics { return m_summary_count.load(std::memory_order_relaxed); } - StatsDuration &GetDurationReference() { return m_total_time; } + StatsDuration &GetDurationReference() { return m_total_time; }; std::string GetSummaryKindName() const { return m_impl_type; } 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), m_elapsed_time(summary.GetDurationReference()) {} - ~SummaryInvocation() { m_summary.OnInvoked(); } - - // Delete the copy constructor and assignment operator to prevent - // accidental double counting. - SummaryInvocation(const SummaryInvocation &) = delete; - SummaryInvocation &operator=(const SummaryInvocation &) = delete; - - private: - SummaryStatistics &m_summary; - ElapsedTime m_elapsed_time; - }; - - SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); } - -private: - /// Called when Summary Invocation is destructed. - void OnInvoked() noexcept { - m_summary_count.fetch_add(1, std::memory_order_relaxed); + void IncrementSummaryCount() { + m_summary_count.fetch_add(1, std::memory_order_relaxed); } +private: lldb_private::StatsDuration m_total_time; std::string m_impl_type; std::string m_name; @@ -234,25 +212,52 @@ class SummaryStatistics { /// A class that wraps a std::map of SummaryStatistics objects behind a mutex. class SummaryStatisticsCache { public: + /// 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, SummaryStatisticsCache &cache) + : m_provider_key(summary.GetName()), m_cache(cache), m_elapsed_time(summary.GetDurationReference()) {} + ~SummaryInvocation() { m_cache.OnInvoked(m_provider_key); } + + /// Delete the copy constructor and assignment operator to prevent + /// accidental double counting. + /// @{ + SummaryInvocation(const SummaryInvocation &) = delete; + SummaryInvocation &operator=(const SummaryInvocation &) = delete; + /// @} + + private: + std::string m_provider_key; + SummaryStatisticsCache &m_cache; + ElapsedTime m_elapsed_time; + }; + /// Get the SummaryStatistics object for a given provider name, or insert /// if statistics for that provider is not in the map. - SummaryStatistics::SummaryInvocation + SummaryStatisticsCache::SummaryInvocation GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) { std::lock_guard<std::mutex> guard(m_map_mutex); auto pair = m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(), provider.GetSummaryKindName()); - return pair.first->second.GetSummaryInvocation(); + return SummaryInvocation(pair.first->second, *this); } llvm::json::Value ToJSON(); private: - std::map<std::string, lldb_private::SummaryStatistics> - m_summary_stats_map; + /// Called when Summary Invocation is destructed. + void OnInvoked(std::string provider_name) noexcept { + // .at give us a const reference, and [provider_name] = will give us a copy + m_summary_stats_map.find(provider_name)->second.IncrementSummaryCount(); + } + llvm::StringMap<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: diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index e9d4456da3ff7a..d8c428d58a59f6 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -257,7 +257,7 @@ class TargetProperties : public Properties { void SetDebugUtilityExpression(bool debug); bool GetDebugUtilityExpression() const; - + private: std::optional<bool> GetExperimentalPropertyValue(size_t prop_idx, @@ -1221,8 +1221,6 @@ class Target : public std::enable_shared_from_this<Target>, void ClearAllLoadedSections(); - lldb_private::SummaryStatistics::SummaryInvocation - GetSummaryStatisticsForProvider(lldb_private::TypeSummaryImpl &provider); lldb_private::SummaryStatisticsCache &GetSummaryStatisticsCache(); /// Set the \a Trace object containing processor trace information of this diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index c991018bbc05c6..33f9df89cca7f8 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -619,8 +619,8 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr, TargetSP target = GetExecutionContextRef().GetTargetSP(); if (target) { /// Construct RAII types to time and collect data on summary creation. - SummaryStatistics::SummaryInvocation summary_inv = - target->GetSummaryStatisticsForProvider(*summary_ptr); + SummaryStatisticsCache::SummaryInvocation summary_inv = + target->GetSummaryStatisticsCache().GetSummaryStatisticsForProviderName(*summary_ptr); summary_ptr->FormatObject(this, destination, actual_options); } else summary_ptr->FormatObject(this, destination, actual_options); diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index 4778688e94b5cf..f98c9dc41b6f92 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -161,21 +161,17 @@ ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *python_script) : TypeSummaryImpl(Kind::eScript, flags), m_function_name(), m_python_script(), m_script_function_sp() { - std::string sstring; - // Take preference in the python script name over the function name. + // Take preference in the python script name over the function name.; if (function_name) { - sstring.assign(function_name); m_function_name.assign(function_name); + m_script_formatter_name = function_name; } if (python_script) { - sstring.assign(python_script); m_python_script.assign(python_script); + m_script_formatter_name = python_script; } - // Python scripts include their leading spacing, so we remove it so we don't - // save extra spaces in the const string forever. - sstring.erase(0, sstring.find_first_not_of(' ')); - m_script_formatter_name = sstring; + m_script_formatter_name = m_script_formatter_name.erase(0, m_script_formatter_name.find_first_not_of(' ')); } bool ScriptSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index c1ca442b6c07b7..84d102c1f85e43 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -3193,12 +3193,6 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP §ion_sp, void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); } -SummaryStatistics::SummaryInvocation -Target::GetSummaryStatisticsForProvider(TypeSummaryImpl &provider) { - return m_summary_statistics_cache.GetSummaryStatisticsForProviderName( - provider); -} - SummaryStatisticsCache &Target::GetSummaryStatisticsCache() { return m_summary_statistics_cache; } diff --git a/lldb/test/API/commands/statistics/basic/BoxFormatter.py b/lldb/test/API/commands/statistics/basic/BoxFormatter.py index 07681b32dfd090..763427ae22976a 100644 --- a/lldb/test/API/commands/statistics/basic/BoxFormatter.py +++ b/lldb/test/API/commands/statistics/basic/BoxFormatter.py @@ -1,10 +1,8 @@ import lldb - def summary(valobj, dict): return f"[{valobj.GetChildAtIndex(0).GetValue()}]" - def __lldb_init_module(debugger, dict): typeName = "Box<.*$" debugger.HandleCommand( >From 1a984fb8b1ca01206e223e2872ecd499ddac0bd0 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 14 Aug 2024 14:58:06 -0700 Subject: [PATCH 9/9] Run formatters --- lldb/include/lldb/Target/Statistics.h | 45 +++++++++---------- lldb/source/Core/ValueObject.cpp | 3 +- lldb/source/DataFormatters/TypeSummary.cpp | 11 ++--- .../commands/statistics/basic/BoxFormatter.py | 2 + 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index 80609599559652..0cf02f5ec78855 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -180,8 +180,7 @@ struct StatisticsOptions { /// A class that represents statistics about a TypeSummaryProviders invocations class SummaryStatistics { public: - explicit SummaryStatistics(std::string name, - std::string impl_type) + explicit SummaryStatistics(std::string name, std::string impl_type) : m_total_time(), m_impl_type(impl_type), m_name(name), m_summary_count(0) {} @@ -198,8 +197,8 @@ class SummaryStatistics { llvm::json::Value ToJSON() const; - void IncrementSummaryCount() { - m_summary_count.fetch_add(1, std::memory_order_relaxed); + void IncrementSummaryCount() { + m_summary_count.fetch_add(1, std::memory_order_relaxed); } private: @@ -216,22 +215,23 @@ class SummaryStatisticsCache { /// In the future this can be extended to collect information about the /// elapsed time for a single request. class SummaryInvocation { - public: - SummaryInvocation(SummaryStatistics &summary, SummaryStatisticsCache &cache) - : m_provider_key(summary.GetName()), m_cache(cache), m_elapsed_time(summary.GetDurationReference()) {} - ~SummaryInvocation() { m_cache.OnInvoked(m_provider_key); } - - /// Delete the copy constructor and assignment operator to prevent - /// accidental double counting. - /// @{ - SummaryInvocation(const SummaryInvocation &) = delete; - SummaryInvocation &operator=(const SummaryInvocation &) = delete; - /// @} - - private: - std::string m_provider_key; - SummaryStatisticsCache &m_cache; - ElapsedTime m_elapsed_time; + public: + SummaryInvocation(SummaryStatistics &summary, SummaryStatisticsCache &cache) + : m_provider_key(summary.GetName()), m_cache(cache), + m_elapsed_time(summary.GetDurationReference()) {} + ~SummaryInvocation() { m_cache.OnInvoked(m_provider_key); } + + /// Delete the copy constructor and assignment operator to prevent + /// accidental double counting. + /// @{ + SummaryInvocation(const SummaryInvocation &) = delete; + SummaryInvocation &operator=(const SummaryInvocation &) = delete; + /// @} + + private: + std::string m_provider_key; + SummaryStatisticsCache &m_cache; + ElapsedTime m_elapsed_time; }; /// Get the SummaryStatistics object for a given provider name, or insert @@ -239,8 +239,8 @@ class SummaryStatisticsCache { SummaryStatisticsCache::SummaryInvocation GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) { std::lock_guard<std::mutex> guard(m_map_mutex); - auto pair = m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(), - provider.GetSummaryKindName()); + auto pair = m_summary_stats_map.try_emplace( + provider.GetName(), provider.GetName(), provider.GetSummaryKindName()); return SummaryInvocation(pair.first->second, *this); } @@ -257,7 +257,6 @@ class SummaryStatisticsCache { std::mutex m_map_mutex; }; - /// A class that represents statistics for a since lldb_private::Target. class TargetStats { public: diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index 33f9df89cca7f8..744e718e37d7a8 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -620,7 +620,8 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr, if (target) { /// Construct RAII types to time and collect data on summary creation. SummaryStatisticsCache::SummaryInvocation summary_inv = - target->GetSummaryStatisticsCache().GetSummaryStatisticsForProviderName(*summary_ptr); + target->GetSummaryStatisticsCache() + .GetSummaryStatisticsForProviderName(*summary_ptr); summary_ptr->FormatObject(this, destination, actual_options); } else summary_ptr->FormatObject(this, destination, actual_options); diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp index f98c9dc41b6f92..04d2c58ed4ce29 100644 --- a/lldb/source/DataFormatters/TypeSummary.cpp +++ b/lldb/source/DataFormatters/TypeSummary.cpp @@ -148,13 +148,9 @@ std::string CXXFunctionSummaryFormat::GetDescription() { return std::string(sstr.GetString()); } -std::string CXXFunctionSummaryFormat::GetName() { - return m_description; -} +std::string CXXFunctionSummaryFormat::GetName() { return m_description; } -std::string CXXFunctionSummaryFormat::GetSummaryKindName() { - return "c++"; -} +std::string CXXFunctionSummaryFormat::GetSummaryKindName() { return "c++"; } ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, const char *function_name, @@ -171,7 +167,8 @@ ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags, m_script_formatter_name = python_script; } - m_script_formatter_name = m_script_formatter_name.erase(0, m_script_formatter_name.find_first_not_of(' ')); + m_script_formatter_name = m_script_formatter_name.erase( + 0, m_script_formatter_name.find_first_not_of(' ')); } bool ScriptSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval, diff --git a/lldb/test/API/commands/statistics/basic/BoxFormatter.py b/lldb/test/API/commands/statistics/basic/BoxFormatter.py index 763427ae22976a..07681b32dfd090 100644 --- a/lldb/test/API/commands/statistics/basic/BoxFormatter.py +++ b/lldb/test/API/commands/statistics/basic/BoxFormatter.py @@ -1,8 +1,10 @@ import lldb + def summary(valobj, dict): return f"[{valobj.GetChildAtIndex(0).GetValue()}]" + def __lldb_init_module(debugger, dict): typeName = "Box<.*$" debugger.HandleCommand( _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
