[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Tue Aug 13 17:32:29 PDT 2024


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 <jalalonde at fb.com>
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/6] 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 &section_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 <jalalonde at fb.com>
Date: Fri, 9 Aug 2024 16:14:24 -0700
Subject: [PATCH 2/6] 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 &section_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 <jalalonde at fb.com>
Date: Fri, 9 Aug 2024 16:54:37 -0700
Subject: [PATCH 3/6] 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 <jalalonde at fb.com>
Date: Tue, 13 Aug 2024 17:24:49 -0700
Subject: [PATCH 4/6] 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 &section_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 <jalalonde at fb.com>
Date: Tue, 13 Aug 2024 17:26:16 -0700
Subject: [PATCH 5/6] 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 <jalalonde at fb.com>
Date: Tue, 13 Aug 2024 17:32:17 -0700
Subject: [PATCH 6/6] 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,



More information about the lldb-commits mailing list