[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 b451c1630a9799180a3a976a4ecd86c8d4ec35da 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/22] 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 7f4d607f5427df..94ce505231e084 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"
@@ -261,6 +262,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional<bool>
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1224,6 +1226,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.
   ///
@@ -1557,6 +1561,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 5a5d689e03fbc0..c415b808c6ca8f 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3194,6 +3194,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 d69df7cc181cfc187ba806d178956211cbbeaf94 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/22] Implement inovcation timing for summary providers

---
 lldb/include/lldb/Target/Statistics.h         | 79 +++++++++++++++++++
 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, 115 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 5193d099a5494d..48b0f3ff5f4efb 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -17,6 +17,7 @@
 #include "llvm/Support/JSON.h"
 #include <atomic>
 #include <chrono>
+#include <mutex>
 #include <optional>
 #include <ratio>
 #include <string>
@@ -26,6 +27,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:
@@ -175,6 +177,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:
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 94ce505231e084..e9eb4049a41c2c 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"
@@ -1226,7 +1225,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.
@@ -1561,7 +1561,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 390e04cebf6be6..5aed455ce90b9e 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -366,6 +366,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));
@@ -420,3 +421,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 c415b808c6ca8f..4568c5432a6eb2 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3194,13 +3194,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 48f373ffd26826b9452ecf7bbbaaea0837c063da 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/22] 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 48b0f3ff5f4efb..8a230f4252b848 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -187,7 +187,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 5aed455ce90b9e..0ff0e63df65d7f 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -196,6 +196,8 @@ TargetStats::ToJSON(Target &target,
                                   m_source_realpath_attempt_count);
   target_metrics_json.try_emplace("sourceRealpathCompatibleCount",
                                   m_source_realpath_compatible_count);
+  target_metrics_json.try_emplace("summaryProviderStatistics", 
+                                  target.GetSummaryStatisticsCache().ToJSON());
   return target_metrics_json;
 }
 
@@ -366,7 +368,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));
@@ -423,18 +424,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 0e0828ce0418571827daa85afd2c6d5859d8a470 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/22] 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 8a230f4252b848..3a41619439bd10 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/RealpathPrefixes.h"
 #include "lldb/Utility/Stream.h"
@@ -180,52 +181,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;
 };
@@ -233,25 +235,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 e9eb4049a41c2c..de494034e5080e 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -261,7 +261,6 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
-
 private:
   std::optional<bool>
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1225,8 +1224,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 0ff0e63df65d7f..9a5f4a4b7c6606 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -196,7 +196,7 @@ TargetStats::ToJSON(Target &target,
                                   m_source_realpath_attempt_count);
   target_metrics_json.try_emplace("sourceRealpathCompatibleCount",
                                   m_source_realpath_compatible_count);
-  target_metrics_json.try_emplace("summaryProviderStatistics", 
+  target_metrics_json.try_emplace("summaryProviderStatistics",
                                   target.GetSummaryStatisticsCache().ToJSON());
   return target_metrics_json;
 }
@@ -424,21 +424,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 4568c5432a6eb2..129d939e6042eb 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3194,11 +3194,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 16be36370df9722249f53b6c5deaee98db13bd4c 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/22] 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 62a162219373b425fafb31369997f0aff2d24511 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/22] 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 de494034e5080e..04d9c88a5f9ea7 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -260,7 +260,7 @@ class TargetProperties : public Properties {
   void SetDebugUtilityExpression(bool debug);
 
   bool GetDebugUtilityExpression() const;
-
+  
 private:
   std::optional<bool>
   GetExperimentalPropertyValue(size_t prop_idx,

>From 19ab63fbf8d99cd93150193da077019ad5270e75 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/22] 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 3a41619439bd10..d43cdf859a0b05 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -181,12 +181,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 {
@@ -195,7 +195,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;
 
@@ -227,8 +227,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;
 };
 
@@ -237,23 +237,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 04d9c88a5f9ea7..1cb8ae0121e058 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1224,7 +1224,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 9a5f4a4b7c6606..e5a9c24d2782a3 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -425,15 +425,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 129d939e6042eb..d39efd7781d05d 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3194,7 +3194,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 8ae8b029b0c6f0393261bd4394ea4d1495b4086e 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/22] 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 d43cdf859a0b05..7600696c51df27 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -193,39 +193,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;
@@ -235,25 +213,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 1cb8ae0121e058..54994848ceda53 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -260,7 +260,7 @@ class TargetProperties : public Properties {
   void SetDebugUtilityExpression(bool debug);
 
   bool GetDebugUtilityExpression() const;
-  
+
 private:
   std::optional<bool>
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1224,8 +1224,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 d39efd7781d05d..3aacfee5d26497 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3194,12 +3194,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 c96b8b769612099aa34e499d542b9ef02a3fe391 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/22] 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 7600696c51df27..c9c35a4ebbed8a 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -181,8 +181,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) {}
 
@@ -199,8 +198,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:
@@ -217,22 +216,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
@@ -240,8 +240,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);
   }
@@ -258,7 +258,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 faa5ba7697c6149140cbe08a7be2448d5961047b 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/22] 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 c9c35a4ebbed8a..ccb29db5c89d39 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -183,13 +183,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; };
@@ -198,29 +198,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.
@@ -230,31 +217,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 54994848ceda53..4348197db3c318 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1224,6 +1224,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 e5a9c24d2782a3..2026827bbd4b76 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -436,7 +436,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 3aacfee5d26497..bf893cca05f4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3194,6 +3194,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;
 }

>From 7aaf218ce41f0b85c571016fd2bf729722e59330 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 15 Aug 2024 10:43:27 -0700
Subject: [PATCH 11/22] Stop always creating a new SP to summary stats per try
 insert. Remove unused using and trim up comment. Rename 'invocationCount' to
 count in json and _sp suffix to used shared pointers.

---
 .../include/lldb/DataFormatters/TypeSummary.h |  6 ++++--
 lldb/include/lldb/Target/Statistics.h         | 21 ++++++++++---------
 lldb/source/Core/ValueObject.cpp              |  6 +++---
 lldb/source/Target/Statistics.cpp             |  2 +-
 4 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h
index 7d1b9e1cbd7d91..1e5c3f3e92daa3 100644
--- a/lldb/include/lldb/DataFormatters/TypeSummary.h
+++ b/lldb/include/lldb/DataFormatters/TypeSummary.h
@@ -258,10 +258,12 @@ class TypeSummaryImpl {
 
   virtual std::string GetDescription() = 0;
 
-  /// Get the name of the Type Summary Provider
+  /// Get the name of the Type Summary Provider, either a C++ class, a summary
+  /// string, or a script function name.
   virtual std::string GetName() = 0;
 
-  /// Get the name of the kind of Summary Provider
+  /// Get the name of the kind of Summary Provider, either c++, summary string,
+  /// script or python.
   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 ccb29db5c89d39..e8ca466d978f0f 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -28,7 +28,6 @@ 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:
@@ -200,8 +199,6 @@ class SummaryStatistics {
 
 
   /// 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(std::shared_ptr<SummaryStatistics> summary_stats)
@@ -222,12 +219,12 @@ class SummaryStatistics {
   };
 
 private:
-  void OnInvoked() {
+  void OnInvoked() noexcept {
     m_count.fetch_add(1, std::memory_order_relaxed);
   }
   lldb_private::StatsDuration m_total_time;
-  std::string m_impl_type;
-  std::string m_name;
+  const std::string m_impl_type;
+  const std::string m_name;
   std::atomic<uint64_t> m_count;
 };
 
@@ -242,10 +239,14 @@ class SummaryStatisticsCache {
   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(), std::make_shared<SummaryStatistics>(provider.GetName(), provider.GetSummaryKindName()));
-
-    return pair.first->second;
+    auto iterator = m_summary_stats_map.find(provider.GetName());
+    if (iterator != m_summary_stats_map.end()) 
+        return iterator->second;
+    else {
+      auto it = m_summary_stats_map.try_emplace(provider.GetName(), 
+                                                std::make_shared<SummaryStatistics>(provider.GetName(), provider.GetSummaryKindName()));
+      return it.first->second;
+    }
   }
 
   llvm::json::Value ToJSON();
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 0441c27e0d8f66..df34e808c6e280 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -616,11 +616,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
                                                 // the synthetic children being
                                                 // up-to-date (e.g. ${svar%#})
 
-    TargetSP target = GetExecutionContextRef().GetTargetSP();
-    if (target) {
+    TargetSP target_sp = GetExecutionContextRef().GetTargetSP();
+    if (target_sp) {
       // Get Shared pointer to the summary statistics container
       SummaryStatisticsSP stats_sp =
-          target->GetSummaryStatisticsCache()
+          target_sp->GetSummaryStatisticsCache()
               .GetSummaryStatisticsForProviderName(*summary_ptr);
               
       // Construct RAII types to time and collect data on summary creation.
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 2026827bbd4b76..d619f92122cb9d 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -427,7 +427,7 @@ llvm::json::Value SummaryStatistics::ToJSON() const {
   return json::Object{{
       {"name", GetName()},
       {"type", GetSummaryKindName()},
-      {"invocationCount", GetSummaryCount()},
+      {"count", GetSummaryCount()},
       {"totalTime", GetTotalTime()},
   }};
 }

>From 28fb346ad7b9601def1d36f8846c23cc2b3b66c3 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 15 Aug 2024 11:05:27 -0700
Subject: [PATCH 12/22] Run GCF

---
 lldb/include/lldb/Target/Statistics.h | 21 +++++++++------------
 lldb/include/lldb/Target/Target.h     |  3 ++-
 lldb/source/Core/ValueObject.cpp      |  2 +-
 lldb/source/Target/Target.cpp         |  6 ++++--
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index e8ca466d978f0f..c802cace0eb422 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -181,8 +181,7 @@ struct StatisticsOptions {
 class SummaryStatistics {
 public:
   explicit SummaryStatistics(std::string name, std::string impl_type)
-      : m_total_time(), m_impl_type(impl_type), m_name(name),
-        m_count(0) {}
+      : m_total_time(), m_impl_type(impl_type), m_name(name), m_count(0) {}
 
   std::string GetName() const { return m_name; };
   double GetTotalTime() const { return m_total_time.get().count(); }
@@ -197,9 +196,8 @@ class SummaryStatistics {
 
   llvm::json::Value ToJSON() const;
 
-
   /// Basic RAII class to increment the summary count when the call is complete.
-  class SummaryInvocation{
+  class SummaryInvocation {
   public:
     SummaryInvocation(std::shared_ptr<SummaryStatistics> summary_stats)
         : m_stats(summary_stats),
@@ -219,9 +217,7 @@ class SummaryStatistics {
   };
 
 private:
-  void OnInvoked() noexcept {
-    m_count.fetch_add(1, std::memory_order_relaxed);
-  }
+  void OnInvoked() noexcept { m_count.fetch_add(1, std::memory_order_relaxed); }
   lldb_private::StatsDuration m_total_time;
   const std::string m_impl_type;
   const std::string m_name;
@@ -233,18 +229,19 @@ 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.
   SummaryStatisticsSP
   GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) {
     std::lock_guard<std::mutex> guard(m_map_mutex);
     auto iterator = m_summary_stats_map.find(provider.GetName());
-    if (iterator != m_summary_stats_map.end()) 
-        return iterator->second;
+    if (iterator != m_summary_stats_map.end())
+      return iterator->second;
     else {
-      auto it = m_summary_stats_map.try_emplace(provider.GetName(), 
-                                                std::make_shared<SummaryStatistics>(provider.GetName(), provider.GetSummaryKindName()));
+      auto it = m_summary_stats_map.try_emplace(
+          provider.GetName(),
+          std::make_shared<SummaryStatistics>(provider.GetName(),
+                                              provider.GetSummaryKindName()));
       return it.first->second;
     }
   }
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 4348197db3c318..50df01aac74004 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1224,7 +1224,8 @@ class Target : public std::enable_shared_from_this<Target>,
 
   void ClearAllLoadedSections();
 
-  lldb_private::SummaryStatisticsSP GetSummaryStatisticsSPForProviderName(lldb_private::TypeSummaryImpl &summary_provider);
+  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 df34e808c6e280..2cf366552f96d8 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -622,7 +622,7 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
       SummaryStatisticsSP stats_sp =
           target_sp->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);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index bf893cca05f4ff..079c3b6f4eb0a7 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3194,8 +3194,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);
+lldb_private::SummaryStatisticsSP Target::GetSummaryStatisticsSPForProviderName(
+    lldb_private::TypeSummaryImpl &summary_provider) {
+  return m_summary_statistics_cache.GetSummaryStatisticsForProviderName(
+      summary_provider);
 }
 
 SummaryStatisticsCache &Target::GetSummaryStatisticsCache() {

>From a0172fddb65caeeea3c9ae6c4d1521af3f5a21c8 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 19 Aug 2024 09:58:02 -0700
Subject: [PATCH 13/22] Actually rename variables in test after changing json
 key names

---
 lldb/test/API/commands/statistics/basic/TestStats.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 715219d9790104..900d8b415bbb52 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -940,7 +940,7 @@ def test_summary_statistics_providers(self):
         # 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)
+        self.assertIn("'count': 1", summary_provider_str)
         self.assertIn("'totalTime':", summary_provider_str)
         self.assertIn("'type': 'c++'", summary_provider_str)
 
@@ -952,7 +952,7 @@ def test_summary_statistics_providers(self):
         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("'count': 1", summary_provider_str)
         self.assertIn("'totalTime':", summary_provider_str)
         self.assertIn("'type': 'python'", summary_provider_str)
 
@@ -975,6 +975,6 @@ def test_summary_statistics_providers_vec(self):
         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("'count': 2", summary_provider_str)
         self.assertIn("'totalTime':", summary_provider_str)
         self.assertIn("'type': 'c++'", summary_provider_str)

>From 4a2f234f21c336af683d6ce98ea6fe864d0a9bcb Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 19 Aug 2024 10:04:25 -0700
Subject: [PATCH 14/22] Rebase and implement Michael's feedback on std::move in
 the summary stats constructor

---
 lldb/include/lldb/Target/Statistics.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index c802cace0eb422..a6810170deb239 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -181,7 +181,7 @@ struct StatisticsOptions {
 class SummaryStatistics {
 public:
   explicit SummaryStatistics(std::string name, std::string impl_type)
-      : m_total_time(), m_impl_type(impl_type), m_name(name), m_count(0) {}
+      : m_total_time(), m_impl_type(std::move(impl_type)), m_name(std::move(name)), m_count(0) {}
 
   std::string GetName() const { return m_name; };
   double GetTotalTime() const { return m_total_time.get().count(); }

>From 72c6f2feb7c7c1d068afaef17d0eda9a642580eb Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 19 Aug 2024 11:34:08 -0700
Subject: [PATCH 15/22] Reformat code and add note

---
 lldb/include/lldb/Target/Statistics.h | 18 +++++++++---------
 lldb/source/Core/ValueObject.cpp      |  3 +--
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index a6810170deb239..05a9ad7e13d749 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -178,6 +178,7 @@ struct StatisticsOptions {
 };
 
 /// A class that represents statistics about a TypeSummaryProviders invocations
+/// \note All members of this class need to be accessed in a thread safe manner
 class SummaryStatistics {
 public:
   explicit SummaryStatistics(std::string name, std::string impl_type)
@@ -234,16 +235,15 @@ class SummaryStatisticsCache {
   SummaryStatisticsSP
   GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) {
     std::lock_guard<std::mutex> guard(m_map_mutex);
-    auto iterator = m_summary_stats_map.find(provider.GetName());
-    if (iterator != m_summary_stats_map.end())
+    if (auto iterator = m_summary_stats_map.find(provider.GetName());
+         iterator != m_summary_stats_map.end())
       return iterator->second;
-    else {
-      auto it = m_summary_stats_map.try_emplace(
-          provider.GetName(),
-          std::make_shared<SummaryStatistics>(provider.GetName(),
-                                              provider.GetSummaryKindName()));
-      return it.first->second;
-    }
+
+    auto it = m_summary_stats_map.try_emplace(
+        provider.GetName(),
+        std::make_shared<SummaryStatistics>(provider.GetName(),
+                                            provider.GetSummaryKindName()));
+    return it.first->second;
   }
 
   llvm::json::Value ToJSON();
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 2cf366552f96d8..8a1c181cd35def 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -616,8 +616,7 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
                                                 // the synthetic children being
                                                 // up-to-date (e.g. ${svar%#})
 
-    TargetSP target_sp = GetExecutionContextRef().GetTargetSP();
-    if (target_sp) {
+    if (TargetSP target_sp = GetExecutionContextRef().GetTargetSP()) {
       // Get Shared pointer to the summary statistics container
       SummaryStatisticsSP stats_sp =
           target_sp->GetSummaryStatisticsCache()

>From 7c0894d45a0b1045ad364434b6031c5b8c164072 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 22 Aug 2024 14:58:50 -0700
Subject: [PATCH 16/22] Implement more feedback on naming, and finally add some
 multithreaded unit tests

---
 lldb/include/lldb/Target/Statistics.h         |  2 +-
 lldb/source/Core/ValueObject.cpp              |  2 +-
 lldb/source/Target/Target.cpp                 |  2 +-
 lldb/unittests/Target/CMakeLists.txt          |  1 +
 .../Target/SummaryStatisticsTest.cpp          | 71 +++++++++++++++++++
 5 files changed, 75 insertions(+), 3 deletions(-)
 create mode 100644 lldb/unittests/Target/SummaryStatisticsTest.cpp

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 05a9ad7e13d749..e05f025e87471d 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -233,7 +233,7 @@ class SummaryStatisticsCache {
   /// Get the SummaryStatistics object for a given provider name, or insert
   /// if statistics for that provider is not in the map.
   SummaryStatisticsSP
-  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) {
+  GetSummaryStatisticsForProvider(lldb_private::TypeSummaryImpl &provider) {
     std::lock_guard<std::mutex> guard(m_map_mutex);
     if (auto iterator = m_summary_stats_map.find(provider.GetName());
          iterator != m_summary_stats_map.end())
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8a1c181cd35def..f1bf850fe7e786 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -620,7 +620,7 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
       // Get Shared pointer to the summary statistics container
       SummaryStatisticsSP stats_sp =
           target_sp->GetSummaryStatisticsCache()
-              .GetSummaryStatisticsForProviderName(*summary_ptr);
+              .GetSummaryStatisticsForProvider(*summary_ptr);
 
       // Construct RAII types to time and collect data on summary creation.
       SummaryStatistics::SummaryInvocation invocation(stats_sp);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 079c3b6f4eb0a7..4fc4af8478d44a 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3196,7 +3196,7 @@ void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); }
 
 lldb_private::SummaryStatisticsSP Target::GetSummaryStatisticsSPForProviderName(
     lldb_private::TypeSummaryImpl &summary_provider) {
-  return m_summary_statistics_cache.GetSummaryStatisticsForProviderName(
+  return m_summary_statistics_cache.GetSummaryStatisticsForProvider(
       summary_provider);
 }
 
diff --git a/lldb/unittests/Target/CMakeLists.txt b/lldb/unittests/Target/CMakeLists.txt
index d10e93ca6336d6..a64876184de160 100644
--- a/lldb/unittests/Target/CMakeLists.txt
+++ b/lldb/unittests/Target/CMakeLists.txt
@@ -11,6 +11,7 @@ add_lldb_unittest(TargetTests
   RegisterFlagsTest.cpp
   RemoteAwarePlatformTest.cpp
   StackFrameRecognizerTest.cpp
+  SummaryStatisticsTest.cpp
   FindFileTest.cpp
 
   LINK_LIBS
diff --git a/lldb/unittests/Target/SummaryStatisticsTest.cpp b/lldb/unittests/Target/SummaryStatisticsTest.cpp
new file mode 100644
index 00000000000000..bd9a37a91fe421
--- /dev/null
+++ b/lldb/unittests/Target/SummaryStatisticsTest.cpp
@@ -0,0 +1,71 @@
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-private-enumerations.h"
+#include "lldb/lldb-private.h"
+#include "lldb/Utility/Stream.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include <thread>
+
+using namespace lldb_private;
+using Duration = std::chrono::duration<double>;
+
+class DummySummaryImpl : public lldb_private::TypeSummaryImpl {
+public:
+  DummySummaryImpl(Duration sleepTime):
+    TypeSummaryImpl(TypeSummaryImpl::Kind::eSummaryString, TypeSummaryImpl::Flags()),
+     m_sleepTime(sleepTime) {}
+
+  std::string GetName() override {
+    return "DummySummary";
+  }
+
+  std::string GetSummaryKindName() override {
+    return "dummy";
+  }
+
+  std::string GetDescription() override {
+    return "";
+  } 
+
+  bool FormatObject(ValueObject *valobj, std::string &dest,
+                    const TypeSummaryOptions &options) override {
+    return false;
+  }
+
+  void FakeFormat() {
+    std::this_thread::sleep_for(m_sleepTime);
+  }
+
+private:
+  Duration m_sleepTime;
+};
+
+TEST(MultithreadFormatting, Multithread) {
+  SummaryStatisticsCache statistics_cache;
+  DummySummaryImpl summary(Duration(1));
+  std::vector<std::thread> threads;
+  for (int i = 0; i < 10; ++i) {
+    threads.emplace_back(std::thread([&statistics_cache, &summary]() {
+      auto sp = statistics_cache.GetSummaryStatisticsForProvider(summary);
+      {
+        SummaryStatistics::SummaryInvocation invocation(sp);
+        summary.FakeFormat();
+      }
+    }));
+  }
+
+  for (auto &thread : threads) 
+    thread.join();
+
+  auto sp = statistics_cache.GetSummaryStatisticsForProvider(summary);
+  ASSERT_TRUE(sp->GetDurationReference().get().count() > 10);
+  ASSERT_TRUE(sp->GetSummaryCount() == 10);
+
+  std::string stats_as_json;
+  llvm::raw_string_ostream ss(stats_as_json);
+  ss << sp->ToJSON();
+  ASSERT_THAT(stats_as_json, ::testing::HasSubstr("\"name\":\"DummySummary\""));
+  ASSERT_THAT(stats_as_json, ::testing::HasSubstr("\"type\":\"dummy\""));
+}

>From 4ffeef963474a34a1204ca0a9f80dcb986f21881 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 22 Aug 2024 16:05:16 -0700
Subject: [PATCH 17/22] Format

---
 lldb/include/lldb/Target/Statistics.h         |  5 ++--
 .../Target/SummaryStatisticsTest.cpp          | 27 +++++++------------
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index e05f025e87471d..07855a43ba3e0a 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -182,7 +182,8 @@ struct StatisticsOptions {
 class SummaryStatistics {
 public:
   explicit SummaryStatistics(std::string name, std::string impl_type)
-      : m_total_time(), m_impl_type(std::move(impl_type)), m_name(std::move(name)), m_count(0) {}
+      : m_total_time(), m_impl_type(std::move(impl_type)),
+        m_name(std::move(name)), m_count(0) {}
 
   std::string GetName() const { return m_name; };
   double GetTotalTime() const { return m_total_time.get().count(); }
@@ -236,7 +237,7 @@ class SummaryStatisticsCache {
   GetSummaryStatisticsForProvider(lldb_private::TypeSummaryImpl &provider) {
     std::lock_guard<std::mutex> guard(m_map_mutex);
     if (auto iterator = m_summary_stats_map.find(provider.GetName());
-         iterator != m_summary_stats_map.end())
+        iterator != m_summary_stats_map.end())
       return iterator->second;
 
     auto it = m_summary_stats_map.try_emplace(
diff --git a/lldb/unittests/Target/SummaryStatisticsTest.cpp b/lldb/unittests/Target/SummaryStatisticsTest.cpp
index bd9a37a91fe421..b1cf2e07f2a86e 100644
--- a/lldb/unittests/Target/SummaryStatisticsTest.cpp
+++ b/lldb/unittests/Target/SummaryStatisticsTest.cpp
@@ -1,9 +1,9 @@
 #include "lldb/DataFormatters/TypeSummary.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Utility/Stream.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-private.h"
-#include "lldb/Utility/Stream.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include <thread>
@@ -13,30 +13,23 @@ using Duration = std::chrono::duration<double>;
 
 class DummySummaryImpl : public lldb_private::TypeSummaryImpl {
 public:
-  DummySummaryImpl(Duration sleepTime):
-    TypeSummaryImpl(TypeSummaryImpl::Kind::eSummaryString, TypeSummaryImpl::Flags()),
-     m_sleepTime(sleepTime) {}
+  DummySummaryImpl(Duration sleepTime)
+      : TypeSummaryImpl(TypeSummaryImpl::Kind::eSummaryString,
+                        TypeSummaryImpl::Flags()),
+        m_sleepTime(sleepTime) {}
 
-  std::string GetName() override {
-    return "DummySummary";
-  }
+  std::string GetName() override { return "DummySummary"; }
 
-  std::string GetSummaryKindName() override {
-    return "dummy";
-  }
+  std::string GetSummaryKindName() override { return "dummy"; }
 
-  std::string GetDescription() override {
-    return "";
-  } 
+  std::string GetDescription() override { return ""; }
 
   bool FormatObject(ValueObject *valobj, std::string &dest,
                     const TypeSummaryOptions &options) override {
     return false;
   }
 
-  void FakeFormat() {
-    std::this_thread::sleep_for(m_sleepTime);
-  }
+  void FakeFormat() { std::this_thread::sleep_for(m_sleepTime); }
 
 private:
   Duration m_sleepTime;
@@ -56,7 +49,7 @@ TEST(MultithreadFormatting, Multithread) {
     }));
   }
 
-  for (auto &thread : threads) 
+  for (auto &thread : threads)
     thread.join();
 
   auto sp = statistics_cache.GetSummaryStatisticsForProvider(summary);

>From dcaeda666ba362665d4641768e63dd3f4a72b874 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 22 Aug 2024 17:30:53 -0700
Subject: [PATCH 18/22] Remove comment

---
 lldb/source/Core/ValueObject.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index f1bf850fe7e786..fb41bf68f440d1 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -617,7 +617,6 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
                                                 // up-to-date (e.g. ${svar%#})
 
     if (TargetSP target_sp = GetExecutionContextRef().GetTargetSP()) {
-      // Get Shared pointer to the summary statistics container
       SummaryStatisticsSP stats_sp =
           target_sp->GetSummaryStatisticsCache()
               .GetSummaryStatisticsForProvider(*summary_ptr);

>From 479e6b52b21aba9d6a102d23522672976896fa55 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 22 Aug 2024 17:38:54 -0700
Subject: [PATCH 19/22] Add comment why SummaryStatSp isn't forward declared

---
 lldb/include/lldb/Target/Statistics.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 07855a43ba3e0a..b8d0b5ef6718ed 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -182,8 +182,7 @@ struct StatisticsOptions {
 class SummaryStatistics {
 public:
   explicit SummaryStatistics(std::string name, std::string impl_type)
-      : m_total_time(), m_impl_type(std::move(impl_type)),
-        m_name(std::move(name)), m_count(0) {}
+      : m_total_time(), m_impl_type(std::move(impl_type)), m_name(std::move(name)), m_count(0) {}
 
   std::string GetName() const { return m_name; };
   double GetTotalTime() const { return m_total_time.get().count(); }
@@ -226,6 +225,7 @@ class SummaryStatistics {
   std::atomic<uint64_t> m_count;
 };
 
+// Declaring here as there is no private forward
 typedef std::shared_ptr<SummaryStatistics> SummaryStatisticsSP;
 
 /// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
@@ -237,7 +237,7 @@ class SummaryStatisticsCache {
   GetSummaryStatisticsForProvider(lldb_private::TypeSummaryImpl &provider) {
     std::lock_guard<std::mutex> guard(m_map_mutex);
     if (auto iterator = m_summary_stats_map.find(provider.GetName());
-        iterator != m_summary_stats_map.end())
+         iterator != m_summary_stats_map.end())
       return iterator->second;
 
     auto it = m_summary_stats_map.try_emplace(

>From 9f9134ed5742e2e0c69962fd1280265d42d9c933 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 23 Aug 2024 12:21:04 -0700
Subject: [PATCH 20/22] Final tidy up feedback and run clang-format

---
 lldb/include/lldb/Target/Statistics.h           |  5 +++--
 lldb/source/DataFormatters/TypeSummary.cpp      |  4 +++-
 lldb/unittests/Target/SummaryStatisticsTest.cpp | 14 ++++----------
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index b8d0b5ef6718ed..2082c3af1dd060 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -182,7 +182,8 @@ struct StatisticsOptions {
 class SummaryStatistics {
 public:
   explicit SummaryStatistics(std::string name, std::string impl_type)
-      : m_total_time(), m_impl_type(std::move(impl_type)), m_name(std::move(name)), m_count(0) {}
+      : m_total_time(), m_impl_type(std::move(impl_type)),
+        m_name(std::move(name)), m_count(0) {}
 
   std::string GetName() const { return m_name; };
   double GetTotalTime() const { return m_total_time.get().count(); }
@@ -237,7 +238,7 @@ class SummaryStatisticsCache {
   GetSummaryStatisticsForProvider(lldb_private::TypeSummaryImpl &provider) {
     std::lock_guard<std::mutex> guard(m_map_mutex);
     if (auto iterator = m_summary_stats_map.find(provider.GetName());
-         iterator != m_summary_stats_map.end())
+        iterator != m_summary_stats_map.end())
       return iterator->second;
 
     auto it = m_summary_stats_map.try_emplace(
diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp
index 04d2c58ed4ce29..c7ec0edeb2e5dd 100644
--- a/lldb/source/DataFormatters/TypeSummary.cpp
+++ b/lldb/source/DataFormatters/TypeSummary.cpp
@@ -157,7 +157,7 @@ ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags,
                                          const char *python_script)
     : TypeSummaryImpl(Kind::eScript, flags), m_function_name(),
       m_python_script(), m_script_function_sp() {
-  // 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) {
     m_function_name.assign(function_name);
     m_script_formatter_name = function_name;
@@ -167,6 +167,8 @@ ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags,
     m_script_formatter_name = python_script;
   }
 
+  // Python scripts include the tabbing of the function def so we remove the
+  // leading spaces.
   m_script_formatter_name = m_script_formatter_name.erase(
       0, m_script_formatter_name.find_first_not_of(' '));
 }
diff --git a/lldb/unittests/Target/SummaryStatisticsTest.cpp b/lldb/unittests/Target/SummaryStatisticsTest.cpp
index b1cf2e07f2a86e..f4033752845408 100644
--- a/lldb/unittests/Target/SummaryStatisticsTest.cpp
+++ b/lldb/unittests/Target/SummaryStatisticsTest.cpp
@@ -13,10 +13,9 @@ using Duration = std::chrono::duration<double>;
 
 class DummySummaryImpl : public lldb_private::TypeSummaryImpl {
 public:
-  DummySummaryImpl(Duration sleepTime)
+  DummySummaryImpl()
       : TypeSummaryImpl(TypeSummaryImpl::Kind::eSummaryString,
-                        TypeSummaryImpl::Flags()),
-        m_sleepTime(sleepTime) {}
+                        TypeSummaryImpl::Flags()) {}
 
   std::string GetName() override { return "DummySummary"; }
 
@@ -28,23 +27,18 @@ class DummySummaryImpl : public lldb_private::TypeSummaryImpl {
                     const TypeSummaryOptions &options) override {
     return false;
   }
-
-  void FakeFormat() { std::this_thread::sleep_for(m_sleepTime); }
-
-private:
-  Duration m_sleepTime;
 };
 
 TEST(MultithreadFormatting, Multithread) {
   SummaryStatisticsCache statistics_cache;
-  DummySummaryImpl summary(Duration(1));
+  DummySummaryImpl summary;
   std::vector<std::thread> threads;
   for (int i = 0; i < 10; ++i) {
     threads.emplace_back(std::thread([&statistics_cache, &summary]() {
       auto sp = statistics_cache.GetSummaryStatisticsForProvider(summary);
       {
         SummaryStatistics::SummaryInvocation invocation(sp);
-        summary.FakeFormat();
+        std::this_thread::sleep_for(Duration(1));
       }
     }));
   }

>From aa71d5fa26c1f419f96f91e18eee5698a72f7563 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 4 Sep 2024 11:27:27 -0700
Subject: [PATCH 21/22] Rework tests a bit to support the CI introduced
 edgecase where sometimes they're summary strings.

---
 lldb/test/API/commands/statistics/basic/TestStats.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 900d8b415bbb52..0417a2aa8477a1 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -942,7 +942,8 @@ def test_summary_statistics_providers(self):
         self.assertIn("string", summary_provider_str)
         self.assertIn("'count': 1", summary_provider_str)
         self.assertIn("'totalTime':", summary_provider_str)
-        self.assertIn("'type': 'c++'", summary_provider_str)
+        # We may hit the std::string C++ provider, or a summary provider string
+        self.assertRegex(r"'type': '(c++|string)'", summary_provider_str)
 
         self.runCmd("continue")
         self.runCmd("command script import BoxFormatter.py")
@@ -974,7 +975,9 @@ def test_summary_statistics_providers_vec(self):
         self.assertIn("summaryProviderStatistics", stats)
         summary_providers = stats["summaryProviderStatistics"]
         summary_provider_str = str(summary_providers)
-        self.assertIn("std::vector", summary_provider_str)
         self.assertIn("'count': 2", summary_provider_str)
         self.assertIn("'totalTime':", summary_provider_str)
-        self.assertIn("'type': 'c++'", summary_provider_str)
+        self.assertRegex(r"'type': '(c++|string)'", summary_provider_str)
+        # We may hit the std::vector C++ provider, or a summary provider string
+        if "c++" in summary_provider_str:
+            self.assertIn("std::vector", summary_provider_str)

>From 37968584b3e7c52f5a5bf282593e884f6a46a3e1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 5 Sep 2024 13:14:18 -0700
Subject: [PATCH 22/22] Move to a unified function for the kind name

---
 .../include/lldb/DataFormatters/TypeSummary.h |  7 ++----
 lldb/include/lldb/Target/Statistics.h         |  9 ++++----
 lldb/source/DataFormatters/TypeSummary.cpp    | 22 +++++++++++--------
 .../commands/statistics/basic/TestStats.py    |  5 +++--
 4 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/lldb/include/lldb/DataFormatters/TypeSummary.h b/lldb/include/lldb/DataFormatters/TypeSummary.h
index 1e5c3f3e92daa3..5d8edfb34b5b57 100644
--- a/lldb/include/lldb/DataFormatters/TypeSummary.h
+++ b/lldb/include/lldb/DataFormatters/TypeSummary.h
@@ -264,7 +264,7 @@ class TypeSummaryImpl {
 
   /// Get the name of the kind of Summary Provider, either c++, summary string,
   /// script or python.
-  virtual std::string GetSummaryKindName() = 0;
+  virtual std::string GetSummaryKindName();
 
   uint32_t &GetRevision() { return m_my_revision; }
 
@@ -302,8 +302,7 @@ struct StringSummaryFormat : public TypeSummaryImpl {
   std::string GetDescription() override;
 
   std::string GetName() override;
-  std::string GetSummaryKindName() override;
-
+  
   static bool classof(const TypeSummaryImpl *S) {
     return S->GetKind() == Kind::eSummaryString;
   }
@@ -352,7 +351,6 @@ struct CXXFunctionSummaryFormat : public TypeSummaryImpl {
   }
 
   std::string GetName() override;
-  std::string GetSummaryKindName() override;
 
   typedef std::shared_ptr<CXXFunctionSummaryFormat> SharedPointer;
 
@@ -400,7 +398,6 @@ struct ScriptSummaryFormat : public TypeSummaryImpl {
   std::string GetDescription() 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 2082c3af1dd060..cd61938c9e9111 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -28,6 +28,9 @@ namespace lldb_private {
 
 using StatsClock = std::chrono::high_resolution_clock;
 using StatsTimepoint = std::chrono::time_point<StatsClock>;
+class SummaryStatistics;
+// Declaring here as there is no private forward
+typedef std::shared_ptr<SummaryStatistics> SummaryStatisticsSP;
 
 class StatsDuration {
 public:
@@ -201,7 +204,7 @@ class SummaryStatistics {
   /// Basic RAII class to increment the summary count when the call is complete.
   class SummaryInvocation {
   public:
-    SummaryInvocation(std::shared_ptr<SummaryStatistics> summary_stats)
+    SummaryInvocation(SummaryStatisticsSP summary_stats)
         : m_stats(summary_stats),
           m_elapsed_time(summary_stats->GetDurationReference()) {}
     ~SummaryInvocation() { m_stats->OnInvoked(); }
@@ -214,7 +217,7 @@ class SummaryStatistics {
     /// @}
 
   private:
-    std::shared_ptr<SummaryStatistics> m_stats;
+    SummaryStatisticsSP m_stats;
     ElapsedTime m_elapsed_time;
   };
 
@@ -226,8 +229,6 @@ class SummaryStatistics {
   std::atomic<uint64_t> m_count;
 };
 
-// Declaring here as there is no private forward
-typedef std::shared_ptr<SummaryStatistics> SummaryStatisticsSP;
 
 /// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
 class SummaryStatisticsCache {
diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp
index c7ec0edeb2e5dd..6a5746512a8394 100644
--- a/lldb/source/DataFormatters/TypeSummary.cpp
+++ b/lldb/source/DataFormatters/TypeSummary.cpp
@@ -48,6 +48,19 @@ TypeSummaryOptions::SetCapping(lldb::TypeSummaryCapping cap) {
 TypeSummaryImpl::TypeSummaryImpl(Kind kind, const TypeSummaryImpl::Flags &flags)
     : m_flags(flags), m_kind(kind) {}
 
+std::string TypeSummaryImpl::GetSummaryKindName() {
+  switch (m_kind) {
+    case Kind::eSummaryString:
+      return "string";
+    case Kind::eCallback:
+      return "callback";
+    case Kind::eScript:
+      return "python";
+    case Kind::eInternal:
+      return "c++";
+  }
+}
+
 StringSummaryFormat::StringSummaryFormat(const TypeSummaryImpl::Flags &flags,
                                          const char *format_cstr)
     : TypeSummaryImpl(Kind::eSummaryString, flags), m_format_str() {
@@ -117,7 +130,6 @@ std::string StringSummaryFormat::GetDescription() {
 }
 
 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)
@@ -150,7 +162,6 @@ std::string CXXFunctionSummaryFormat::GetDescription() {
 
 std::string CXXFunctionSummaryFormat::GetName() { return m_description; }
 
-std::string CXXFunctionSummaryFormat::GetSummaryKindName() { return "c++"; }
 
 ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags,
                                          const char *function_name,
@@ -220,10 +231,3 @@ std::string ScriptSummaryFormat::GetDescription() {
 }
 
 std::string ScriptSummaryFormat::GetName() { return m_script_formatter_name; }
-
-std::string ScriptSummaryFormat::GetSummaryKindName() {
-  if (!m_python_script.empty())
-    return "python";
-
-  return "script";
-}
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 0417a2aa8477a1..c48d95f64f2f28 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -943,7 +943,8 @@ def test_summary_statistics_providers(self):
         self.assertIn("'count': 1", summary_provider_str)
         self.assertIn("'totalTime':", summary_provider_str)
         # We may hit the std::string C++ provider, or a summary provider string
-        self.assertRegex(r"'type': '(c++|string)'", summary_provider_str)
+        self.assertIn("'type':", summary_provider_str)
+        self.assertTrue("c++" in summary_provider_str or "string" in summary_provider_str)
 
         self.runCmd("continue")
         self.runCmd("command script import BoxFormatter.py")
@@ -977,7 +978,7 @@ def test_summary_statistics_providers_vec(self):
         summary_provider_str = str(summary_providers)
         self.assertIn("'count': 2", summary_provider_str)
         self.assertIn("'totalTime':", summary_provider_str)
-        self.assertRegex(r"'type': '(c++|string)'", summary_provider_str)
+        self.assertIn("'type':", summary_provider_str)
         # We may hit the std::vector C++ provider, or a summary provider string
         if "c++" in summary_provider_str:
             self.assertIn("std::vector", summary_provider_str)