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

[Jacob Lalonde via lldb-commits]

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 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 <jalalonde at fb.com>
Date: Wed, 14 Aug 2024 09:53:57 -0700
Subject: [PATCH 07/10] 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 &section_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 <jalalonde at fb.com>
Date: Wed, 14 Aug 2024 14:50:05 -0700
Subject: [PATCH 08/10] 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 &section_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 <jalalonde at fb.com>
Date: Wed, 14 Aug 2024 14:58:06 -0700
Subject: [PATCH 09/10] 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(

>From 64d586a31943f2412d455df69afb3c15c08b1e83 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 14 Aug 2024 19:21:25 -0700
Subject: [PATCH 10/10] Talked with Greg and we decided to move back to not
 having the cache give out RAII objects and keep it scoped to the object. In
 addition, to free the stats object from the underlying container we instead
 hand out shared ptrs now

---
 .../include/lldb/DataFormatters/TypeSummary.h |  3 +
 lldb/include/lldb/Target/Statistics.h         | 59 +++++++++----------
 lldb/include/lldb/Target/Target.h             |  1 +
 lldb/source/Core/ValueObject.cpp              |  7 ++-
 lldb/source/Target/Statistics.cpp             |  2 +-
 lldb/source/Target/Target.cpp                 |  4 ++
 6 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h
index 8a3562cedf8fe5..7d1b9e1cbd7d91 100644
--- a/lldb/include/lldb/DataFormatters/TypeSummary.h
+++ b/lldb/include/lldb/DataFormatters/TypeSummary.h
@@ -258,7 +258,10 @@ class TypeSummaryImpl {
 
   virtual std::string GetDescription() = 0;
 
+  /// Get the name of the Type Summary Provider
   virtual std::string GetName() = 0;
+
+  /// Get the name of the kind of Summary Provider
   virtual std::string GetSummaryKindName() = 0;
 
   uint32_t &GetRevision() { return m_my_revision; }
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 0cf02f5ec78855..4e9fc44b65da58 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -182,13 +182,13 @@ class SummaryStatistics {
 public:
   explicit SummaryStatistics(std::string name, std::string impl_type)
       : m_total_time(), m_impl_type(impl_type), m_name(name),
-        m_summary_count(0) {}
+        m_count(0) {}
 
   std::string GetName() const { return m_name; };
   double GetTotalTime() const { return m_total_time.get().count(); }
 
   uint64_t GetSummaryCount() const {
-    return m_summary_count.load(std::memory_order_relaxed);
+    return m_count.load(std::memory_order_relaxed);
   }
 
   StatsDuration &GetDurationReference() { return m_total_time; };
@@ -197,29 +197,16 @@ class SummaryStatistics {
 
   llvm::json::Value ToJSON() const;
 
-  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;
-  std::atomic<uint64_t> m_summary_count;
-};
 
-/// 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 {
+  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); }
+    SummaryInvocation(std::shared_ptr<SummaryStatistics> summary_stats)
+        : m_stats(summary_stats),
+          m_elapsed_time(summary_stats->GetDurationReference()) {}
+    ~SummaryInvocation() { m_stats->OnInvoked(); }
 
     /// Delete the copy constructor and assignment operator to prevent
     /// accidental double counting.
@@ -229,31 +216,41 @@ class SummaryStatisticsCache {
     /// @}
 
   private:
-    std::string m_provider_key;
-    SummaryStatisticsCache &m_cache;
+    std::shared_ptr<SummaryStatistics> m_stats;
     ElapsedTime m_elapsed_time;
   };
 
+private:
+  void OnInvoked() {
+    m_count.fetch_add(1, std::memory_order_relaxed);
+  }
+  lldb_private::StatsDuration m_total_time;
+  std::string m_impl_type;
+  std::string m_name;
+  std::atomic<uint64_t> m_count;
+};
+
+typedef std::shared_ptr<SummaryStatistics> SummaryStatisticsSP;
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+
   /// Get the SummaryStatistics object for a given provider name, or insert
   /// if statistics for that provider is not in the map.
-  SummaryStatisticsCache::SummaryInvocation
+  SummaryStatisticsSP
   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());
+        provider.GetName(), std::make_shared<SummaryStatistics>(provider.GetName(), provider.GetSummaryKindName()));
 
-    return SummaryInvocation(pair.first->second, *this);
+    return pair.first->second;
   }
 
   llvm::json::Value ToJSON();
 
 private:
-  /// 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;
+  llvm::StringMap<SummaryStatisticsSP> m_summary_stats_map;
   std::mutex m_map_mutex;
 };
 
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index d8c428d58a59f6..8fad01b003f747 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1221,6 +1221,7 @@ class Target : public std::enable_shared_from_this<Target>,
 
   void ClearAllLoadedSections();
 
+  lldb_private::SummaryStatisticsSP GetSummaryStatisticsSPForProviderName(lldb_private::TypeSummaryImpl &summary_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 744e718e37d7a8..0441c27e0d8f66 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -618,10 +618,13 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
 
     TargetSP target = GetExecutionContextRef().GetTargetSP();
     if (target) {
-      /// Construct RAII types to time and collect data on summary creation.
-      SummaryStatisticsCache::SummaryInvocation summary_inv =
+      // Get Shared pointer to the summary statistics container
+      SummaryStatisticsSP stats_sp =
           target->GetSummaryStatisticsCache()
               .GetSummaryStatisticsForProviderName(*summary_ptr);
+              
+      // Construct RAII types to time and collect data on summary creation.
+      SummaryStatistics::SummaryInvocation invocation(stats_sp);
       summary_ptr->FormatObject(this, destination, actual_options);
     } else
       summary_ptr->FormatObject(this, destination, actual_options);
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index cf36a1c90d38a7..ee13b3c028c5c2 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -424,7 +424,7 @@ json::Value SummaryStatisticsCache::ToJSON() {
   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());
+    json_summary_stats.emplace_back(summary_stat.second->ToJSON());
 
   return json_summary_stats;
 }
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 84d102c1f85e43..1b2985573cec5b 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3193,6 +3193,10 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP &section_sp,
 
 void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); }
 
+lldb_private::SummaryStatisticsSP Target::GetSummaryStatisticsSPForProviderName(lldb_private::TypeSummaryImpl &summary_provider) {
+  return m_summary_statistics_cache.GetSummaryStatisticsForProviderName(summary_provider);
+}
+
 SummaryStatisticsCache &Target::GetSummaryStatisticsCache() {
   return m_summary_statistics_cache;
 }