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

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Fri Aug 9 17:36:23 PDT 2024


https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/102708

This PR adds a statistics provider cache, which allows an individual target to keep a rolling tally of it's total time and number of invocations for a given summary provider. This information is then available in statistics dump to help slow summary providers, and gleam more into insight into LLDB's time use.



>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/3] Initial attempt at new classes Summary statistics in
 SummaryStatistics.h/cpp.

---
 lldb/include/lldb/Target/SummaryStatistics.h | 37 ++++++++++++++++++++
 lldb/include/lldb/Target/Target.h            |  5 +++
 lldb/source/Core/ValueObject.cpp             |  5 +++
 lldb/source/Target/CMakeLists.txt            |  1 +
 lldb/source/Target/SummaryStatistics.cpp     | 26 ++++++++++++++
 lldb/source/Target/Target.cpp                |  9 +++++
 6 files changed, 83 insertions(+)
 create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 create mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/SummaryStatistics.h b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00000000000000..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.h -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+  SummaryStatistics(lldb_private::ConstString name) : 
+    m_total_time(), m_name(name), m_summary_count(0) {}
+
+  lldb_private::StatsDuration &GetDurationReference();
+
+  lldb_private::ConstString GetName() const;
+
+  uint64_t GetSummaryCount() const;
+
+private:
+   lldb_private::StatsDuration m_total_time;
+   lldb_private::ConstString m_name;
+   uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional<bool>
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public std::enable_shared_from_this<Target>,
 
   void ClearAllLoadedSections();
 
+  lldb_private::StatsDuration& GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
   /// Set the \a Trace object containing processor trace information of this
   /// target.
   ///
@@ -1554,6 +1558,7 @@ class Target : public std::enable_shared_from_this<Target>,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
                        /// libraries and anything dynamically loaded).
+  std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> m_summary_stats_map;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
       m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
                                                 // the synthetic children being
                                                 // up-to-date (e.g. ${svar%#})
+    StatsDuration &summary_duration = GetExecutionContextRef()
+                                        .GetProcessSP()
+                                        ->GetTarget()
+                                        .GetSummaryProviderDuration(GetTypeName());
+    ElapsedTime elapsed(summary_duration);
     summary_ptr->FormatObject(this, destination, actual_options);
   }
   m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
+  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp b/lldb/source/Target/SummaryStatistics.cpp
new file mode 100644
index 00000000000000..62f9776d796341
--- /dev/null
+++ b/lldb/source/Target/SummaryStatistics.cpp
@@ -0,0 +1,26 @@
+//===-- SummaryStatistics.cpp -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Target/SummaryStatistics.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+
+StatsDuration& SummaryStatistics::GetDurationReference() {
+  m_summary_count++;
+  return m_total_time;
+}
+
+ConstString SummaryStatistics::GetName() const {
+  return m_name;
+}
+
+uint64_t SummaryStatistics::GetSummaryCount() const {
+  return m_summary_count;
+}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 129683c43f0c1a..0f213579ae6f53 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3193,6 +3193,15 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP &section_sp,
 
 void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); }
 
+lldb_private::StatsDuration& Target::GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name) {
+  if (m_summary_stats_map.count(summary_provider_name) == 0) {
+    SummaryStatistics summary_stats(summary_provider_name);
+    m_summary_stats_map.emplace(summary_provider_name, SummaryStatistics(summary_provider_name));
+  }
+
+  return m_summary_stats_map[summary_provider_name].GetDurationReference();
+}
+
 void Target::SaveScriptedLaunchInfo(lldb_private::ProcessInfo &process_info) {
   if (process_info.IsScriptedProcess()) {
     // Only copy scripted process launch options.

>From a3d63fe12b1e7f16c95a9e8f8f4fbd96c9e93a5a Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 9 Aug 2024 16:14:24 -0700
Subject: [PATCH 2/3] Implement inovcation timing for summary providers

---
 lldb/include/lldb/Target/Statistics.h         | 80 +++++++++++++++++++
 lldb/include/lldb/Target/SummaryStatistics.h  | 37 ---------
 lldb/include/lldb/Target/Target.h             |  6 +-
 lldb/source/Core/ValueObject.cpp              | 12 ++-
 lldb/source/Target/CMakeLists.txt             |  1 -
 lldb/source/Target/Statistics.cpp             | 18 +++++
 lldb/source/Target/SummaryStatistics.cpp      | 26 ------
 lldb/source/Target/Target.cpp                 | 11 ++-
 .../API/commands/statistics/basic/Makefile    |  2 +-
 .../commands/statistics/basic/TestStats.py    |  1 +
 .../test/API/commands/statistics/basic/main.c |  2 -
 11 files changed, 116 insertions(+), 80 deletions(-)
 delete mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 delete mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 35bd7f8a66e055..3444c4673d9f15 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/JSON.h"
 #include <atomic>
 #include <chrono>
+#include <mutex>
 #include <optional>
 #include <ratio>
 #include <string>
@@ -25,6 +26,7 @@ namespace lldb_private {
 
 using StatsClock = std::chrono::high_resolution_clock;
 using StatsTimepoint = std::chrono::time_point<StatsClock>;
+using Duration = std::chrono::duration<double>;
 
 class StatsDuration {
 public:
@@ -174,6 +176,83 @@ struct StatisticsOptions {
   std::optional<bool> m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  SummaryStatistics() = default;
+  SummaryStatistics(lldb_private::ConstString name) : 
+    m_total_time(), m_name(name), m_summary_count(0) {}
+
+  SummaryStatistics(const SummaryStatistics &&rhs)
+      : m_total_time(), m_name(rhs.m_name), m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
+
+  lldb_private::ConstString GetName() { return m_name; };
+  double GetAverageTime() const {
+    return m_total_time.get().count() / m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  double GetTotalTime() const {
+    return m_total_time.get().count();
+  }
+
+  uint64_t GetSummaryCount() const {
+    return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration& GetDurationReference() {
+    return m_total_time;
+  }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the 
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+    SummaryInvocation(SummaryStatistics &summary) : m_summary(summary) {}
+    ~SummaryInvocation() {
+      m_summary.OnInvoked();
+    }
+  private:
+    SummaryStatistics &m_summary;
+  };
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked() {
+    m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+  lldb_private::StatsDuration m_total_time;
+  lldb_private::ConstString m_name;
+  std::atomic<uint64_t> m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  SummaryStatisticsCache() = default;
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  lldb_private::SummaryStatistics &GetSummaryStatisticsForProviderName(lldb_private::ConstString summary_provider_name) {
+    m_map_mutex.lock();
+    if (m_summary_stats_map.count(summary_provider_name) == 0) {
+      m_summary_stats_map.emplace(summary_provider_name, SummaryStatistics(summary_provider_name));
+    }
+
+    SummaryStatistics &summary_stats = m_summary_stats_map.at(summary_provider_name);
+    m_map_mutex.unlock();
+    return summary_stats;
+  }
+
+  llvm::json::Value ToJSON();
+
+private:
+  std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> m_summary_stats_map;
+  std::mutex m_map_mutex;
+};
+
 /// A class that represents statistics for a since lldb_private::Target.
 class TargetStats {
 public:
@@ -198,6 +277,7 @@ class TargetStats {
   StatsSuccessFail m_frame_var{"frameVariable"};
   std::vector<intptr_t> m_module_identifiers;
   uint32_t m_source_map_deduce_count = 0;
+  SummaryStatisticsCache m_summary_stats_cache;
   void CollectStats(Target &target);
 };
 
diff --git a/lldb/include/lldb/Target/SummaryStatistics.h b/lldb/include/lldb/Target/SummaryStatistics.h
deleted file mode 100644
index 0198249ba0b170..00000000000000
--- a/lldb/include/lldb/Target/SummaryStatistics.h
+++ /dev/null
@@ -1,37 +0,0 @@
-//===-- SummaryStatistics.h -----------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
-#define LLDB_TARGET_SUMMARYSTATISTICS_H
-
-
-#include "lldb/Target/Statistics.h"
-#include "llvm/ADT/StringRef.h"
-
-namespace lldb_private {
-
-class SummaryStatistics {
-public:
-  SummaryStatistics(lldb_private::ConstString name) : 
-    m_total_time(), m_name(name), m_summary_count(0) {}
-
-  lldb_private::StatsDuration &GetDurationReference();
-
-  lldb_private::ConstString GetName() const;
-
-  uint64_t GetSummaryCount() const;
-
-private:
-   lldb_private::StatsDuration m_total_time;
-   lldb_private::ConstString m_name;
-   uint64_t m_summary_count;
-};
-
-} // namespace lldb_private
-
-#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index ee6a009b0af95d..ae1ea43c01003b 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,7 +30,6 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
-#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -1223,7 +1222,8 @@ class Target : public std::enable_shared_from_this<Target>,
 
   void ClearAllLoadedSections();
 
-  lldb_private::StatsDuration& GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+  lldb_private::SummaryStatistics& GetSummaryStatisticsForProvider(lldb_private::ConstString summary_provider_name);
+  lldb_private::SummaryStatisticsCache& GetSummaryStatisticsCache();
 
   /// Set the \a Trace object containing processor trace information of this
   /// target.
@@ -1558,7 +1558,7 @@ class Target : public std::enable_shared_from_this<Target>,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
                        /// libraries and anything dynamically loaded).
-  std::map<lldb_private::ConstString, lldb_private::SummaryStatistics> m_summary_stats_map;
+  SummaryStatisticsCache m_summary_statistics_cache;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index bed4ab8d69cbda..8e6ff41c08539f 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,12 +615,16 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
       m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
                                                 // the synthetic children being
                                                 // up-to-date (e.g. ${svar%#})
-    StatsDuration &summary_duration = GetExecutionContextRef()
+    SummaryStatistics &summary_stats = GetExecutionContextRef()
                                         .GetProcessSP()
                                         ->GetTarget()
-                                        .GetSummaryProviderDuration(GetTypeName());
-    ElapsedTime elapsed(summary_duration);
-    summary_ptr->FormatObject(this, destination, actual_options);
+                                        .GetSummaryStatisticsForProvider(GetTypeName());
+    /// Construct RAII types to time and collect data on summary creation.
+    SummaryStatistics::SummaryInvocation summary_inv(summary_stats);
+    {
+      ElapsedTime elapsed(summary_stats.GetDurationReference());
+      summary_ptr->FormatObject(this, destination, actual_options);
+    }
   }
   m_flags.m_is_getting_summary = false;
   return !destination.empty();
diff --git a/lldb/source/Target/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt
index e51da37cd84db3..a42c44b761dc56 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,7 +46,6 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
-  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 583d1524881fc3..626214cb7a3187 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -354,6 +354,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   if (include_targets) {
     if (target) {
       json_targets.emplace_back(target->ReportStatistics(options));
+      json_targets.emplace_back(target->GetSummaryStatisticsCache().ToJSON());
     } else {
       for (const auto &target : debugger.GetTargetList().Targets())
         json_targets.emplace_back(target->ReportStatistics(options));
@@ -408,3 +409,20 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 
   return std::move(global_stats);
 }
+
+llvm::json::Value SummaryStatistics::ToJSON() const {
+  return json::Object {
+    {"invocationCount", GetSummaryCount()},
+    {"totalTime", GetTotalTime()},
+    {"averageTime", GetAverageTime()}
+  };
+}
+
+json::Value SummaryStatisticsCache::ToJSON() {
+  m_map_mutex.lock();
+  json::Array json_summary_stats;
+  for (const auto &summary_stat : m_summary_stats_map)
+    json_summary_stats.emplace_back(summary_stat.second.ToJSON());
+  m_map_mutex.unlock();
+  return json::Object{{"summaryProviderStatistics", std::move(json_summary_stats)}};
+}
diff --git a/lldb/source/Target/SummaryStatistics.cpp b/lldb/source/Target/SummaryStatistics.cpp
deleted file mode 100644
index 62f9776d796341..00000000000000
--- a/lldb/source/Target/SummaryStatistics.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-//===-- SummaryStatistics.cpp -----------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Target/SummaryStatistics.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-
-StatsDuration& SummaryStatistics::GetDurationReference() {
-  m_summary_count++;
-  return m_total_time;
-}
-
-ConstString SummaryStatistics::GetName() const {
-  return m_name;
-}
-
-uint64_t SummaryStatistics::GetSummaryCount() const {
-  return m_summary_count;
-}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 0f213579ae6f53..834d48763bdff8 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3193,13 +3193,12 @@ bool Target::SetSectionUnloaded(const lldb::SectionSP &section_sp,
 
 void Target::ClearAllLoadedSections() { m_section_load_history.Clear(); }
 
-lldb_private::StatsDuration& Target::GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name) {
-  if (m_summary_stats_map.count(summary_provider_name) == 0) {
-    SummaryStatistics summary_stats(summary_provider_name);
-    m_summary_stats_map.emplace(summary_provider_name, SummaryStatistics(summary_provider_name));
-  }
+lldb_private::SummaryStatistics& Target::GetSummaryStatisticsForProvider(lldb_private::ConstString summary_provider_name) {
+  return m_summary_statistics_cache.GetSummaryStatisticsForProviderName(summary_provider_name);
+}
 
-  return m_summary_stats_map[summary_provider_name].GetDurationReference();
+lldb_private::SummaryStatisticsCache& Target::GetSummaryStatisticsCache() {
+  return m_summary_statistics_cache;
 }
 
 void Target::SaveScriptedLaunchInfo(lldb_private::ProcessInfo &process_info) {
diff --git a/lldb/test/API/commands/statistics/basic/Makefile b/lldb/test/API/commands/statistics/basic/Makefile
index c9319d6e6888a4..3d0b98f13f3d7b 100644
--- a/lldb/test/API/commands/statistics/basic/Makefile
+++ b/lldb/test/API/commands/statistics/basic/Makefile
@@ -1,2 +1,2 @@
-C_SOURCES := main.c
+CXX_SOURCES := main.cpp
 include Makefile.rules
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index a8ac60090a760d..0a435efbf0a1a0 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -250,6 +250,7 @@ def test_default_with_run(self):
             "launchOrAttachTime",
             "moduleIdentifiers",
             "targetCreateTime",
+            "summaryProviderStatistics"
         ]
         self.verify_keys(stats, '"stats"', keys_exist, None)
         self.assertGreater(stats["firstStopTime"], 0.0)
diff --git a/lldb/test/API/commands/statistics/basic/main.c b/lldb/test/API/commands/statistics/basic/main.c
index 2c5b4f5f098ee4..85c5654d38ff74 100644
--- a/lldb/test/API/commands/statistics/basic/main.c
+++ b/lldb/test/API/commands/statistics/basic/main.c
@@ -1,7 +1,5 @@
 // Test that the lldb command `statistics` works.
-
 int main(void) {
   int patatino = 27;
-
   return 0; // break here
 }

>From ad4b20d2074203fce9e4c47fd6045699b1528d86 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 9 Aug 2024 16:54:37 -0700
Subject: [PATCH 3/3] Sprinkle new entry into tests where there is nothing in
 the array, and add a dedicated test for strings. Migrate main.c to main.cpp
 to facilitate this.

---
 lldb/include/lldb/Target/Statistics.h         |  2 +-
 lldb/source/Target/Statistics.cpp             | 18 ++++++++-----
 .../commands/statistics/basic/TestStats.py    | 26 +++++++++++++++++--
 .../statistics/basic/{main.c => main.cpp}     |  8 ++++++
 4 files changed, 44 insertions(+), 10 deletions(-)
 rename lldb/test/API/commands/statistics/basic/{main.c => main.cpp} (52%)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 3444c4673d9f15..06ca5c7923f747 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -186,7 +186,7 @@ class SummaryStatistics {
   SummaryStatistics(const SummaryStatistics &&rhs)
       : m_total_time(), m_name(rhs.m_name), m_summary_count(rhs.m_summary_count.load(std::memory_order_relaxed)) {}
 
-  lldb_private::ConstString GetName() { return m_name; };
+  lldb_private::ConstString GetName() const { return m_name; };
   double GetAverageTime() const {
     return m_total_time.get().count() / m_summary_count.load(std::memory_order_relaxed);
   }
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 626214cb7a3187..bcb8fdbca42a3c 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -192,6 +192,8 @@ TargetStats::ToJSON(Target &target,
   }
   target_metrics_json.try_emplace("sourceMapDeduceCount",
                                   m_source_map_deduce_count);
+  target_metrics_json.try_emplace("summaryProviderStatistics", 
+                                  target.GetSummaryStatisticsCache().ToJSON());
   return target_metrics_json;
 }
 
@@ -354,7 +356,6 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   if (include_targets) {
     if (target) {
       json_targets.emplace_back(target->ReportStatistics(options));
-      json_targets.emplace_back(target->GetSummaryStatisticsCache().ToJSON());
     } else {
       for (const auto &target : debugger.GetTargetList().Targets())
         json_targets.emplace_back(target->ReportStatistics(options));
@@ -411,18 +412,21 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 }
 
 llvm::json::Value SummaryStatistics::ToJSON() const {
-  return json::Object {
-    {"invocationCount", GetSummaryCount()},
-    {"totalTime", GetTotalTime()},
-    {"averageTime", GetAverageTime()}
-  };
+  json::Object body {{
+      {"invocationCount", GetSummaryCount()},
+      {"totalTime", GetTotalTime()},
+      {"averageTime", GetAverageTime()}
+    }};
+  return json::Object{{GetName().AsCString(), std::move(body)}};
 }
 
+
 json::Value SummaryStatisticsCache::ToJSON() {
   m_map_mutex.lock();
   json::Array json_summary_stats;
   for (const auto &summary_stat : m_summary_stats_map)
     json_summary_stats.emplace_back(summary_stat.second.ToJSON());
+    
   m_map_mutex.unlock();
-  return json::Object{{"summaryProviderStatistics", std::move(json_summary_stats)}};
+  return json_summary_stats;
 }
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 0a435efbf0a1a0..85e76e526849ab 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -81,7 +81,7 @@ def get_command_stats(self, debug_stats):
     def test_expressions_frame_var_counts(self):
         self.build()
         lldbutil.run_to_source_breakpoint(
-            self, "// break here", lldb.SBFileSpec("main.c")
+            self, "// break here", lldb.SBFileSpec("main.cpp")
         )
 
         self.expect("expr patatino", substrs=["27"])
@@ -224,7 +224,7 @@ def test_default_with_run(self):
         self.build()
         target = self.createTestTarget()
         lldbutil.run_to_source_breakpoint(
-            self, "// break here", lldb.SBFileSpec("main.c")
+            self, "// break here", lldb.SBFileSpec("main.cpp")
         )
         debug_stats = self.get_stats()
         debug_stat_keys = [
@@ -448,6 +448,7 @@ def test_breakpoints(self):
             "targetCreateTime",
             "moduleIdentifiers",
             "totalBreakpointResolveTime",
+            "summaryProviderStatistics"
         ]
         self.verify_keys(target_stats, '"stats"', keys_exist, None)
         self.assertGreater(target_stats["totalBreakpointResolveTime"], 0.0)
@@ -919,3 +920,24 @@ def test_order_of_options_do_not_matter(self):
                 debug_stats_1,
                 f"The order of options '{options[0]}' and '{options[1]}' should not matter",
             )
+    
+    def test_summary_statistics_providers(self):
+        """
+        Test summary timing statistics is included in statistics dump when 
+        a type with a summary provider exists, and is evaluated.
+        """
+
+        self.build()
+        target = self.createTestTarget()
+        lldbutil.run_to_source_breakpoint(
+            self, "// stop here", lldb.SBFileSpec("main.cpp")
+        )
+        self.expect("frame var", substrs=["hello world"])
+        stats = self.get_target_stats(self.get_stats())
+        self.assertIn("summaryProviderStatistics", stats)
+        summary_providers = stats["summaryProviderStatistics"]
+        # We don't want to take a dependency on the type name, so we just look
+        # for string and that it was called once.
+        summary_provider_str = str(summary_providers)
+        self.assertIn("string", summary_provider_str)
+        self.assertIn("'invocationCount': 1", summary_provider_str)
diff --git a/lldb/test/API/commands/statistics/basic/main.c b/lldb/test/API/commands/statistics/basic/main.cpp
similarity index 52%
rename from lldb/test/API/commands/statistics/basic/main.c
rename to lldb/test/API/commands/statistics/basic/main.cpp
index 85c5654d38ff74..3e3f34421eca30 100644
--- a/lldb/test/API/commands/statistics/basic/main.c
+++ b/lldb/test/API/commands/statistics/basic/main.cpp
@@ -1,5 +1,13 @@
 // Test that the lldb command `statistics` works.
+#include <string>
+
+void foo() {
+  std::string str = "hello world";
+  str += "\n"; // stop here
+}
+
 int main(void) {
   int patatino = 27;
+  foo();
   return 0; // break here
 }



More information about the lldb-commits mailing list