[Lldb-commits] [lldb] Fix statistics dump to report per-target (PR #113723)

via lldb-commits lldb-commits at lists.llvm.org
Mon Oct 28 16:04:45 PDT 2024


https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/113723

>From abf234c1009b23b000a2b39684fb888084cf5e8c Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Thu, 24 Oct 2024 17:14:55 -0700
Subject: [PATCH 1/2] Report statistics per target

---
 lldb/include/lldb/API/SBDebugger.h            |  2 +
 lldb/include/lldb/API/SBTarget.h              |  3 ++
 lldb/include/lldb/Core/Module.h               |  2 +
 lldb/include/lldb/Symbol/SymbolFile.h         |  3 ++
 lldb/include/lldb/Symbol/SymbolFileOnDemand.h |  2 +
 lldb/include/lldb/Target/Statistics.h         | 12 ++++++
 lldb/source/API/SBDebugger.cpp                |  6 +++
 lldb/source/API/SBTarget.cpp                  |  7 ++++
 lldb/source/Core/Module.cpp                   |  9 +++++
 .../Plugins/SymbolFile/DWARF/DWARFIndex.h     |  2 +
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      |  6 +++
 .../SymbolFile/DWARF/SymbolFileDWARF.h        |  2 +
 lldb/source/Symbol/SymbolFileOnDemand.cpp     |  6 +++
 lldb/source/Target/Statistics.cpp             | 24 ++++++++++-
 .../commands/statistics/basic/TestStats.py    | 40 ++++++++++++++++++-
 .../API/commands/statistics/basic/second.cpp  |  5 +++
 16 files changed, 127 insertions(+), 4 deletions(-)
 create mode 100644 lldb/test/API/commands/statistics/basic/second.cpp

diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 6afa1c932ab050..d80d609b3e7a28 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -426,6 +426,8 @@ class LLDB_API SBDebugger {
 
   SBTypeSynthetic GetSyntheticForType(SBTypeNameSpecifier);
 
+  void ResetStatistics();
+
 #ifndef SWIG
   /// Run the command interpreter.
   ///
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index 35c2ed9c20a238..2e4392990bc4a8 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -101,6 +101,9 @@ class LLDB_API SBTarget {
   ///     A SBStructuredData with the statistics collected.
   lldb::SBStructuredData GetStatistics(SBStatisticsOptions options);
 
+  /// Reset the statistics collected for this target.
+  void ResetStatistics();
+
   /// Return the platform object associated with the target.
   ///
   /// After return, the platform object should be checked for
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 5589c1c9a350dc..9170aca3ed7283 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -880,6 +880,8 @@ class Module : public std::enable_shared_from_this<Module>,
   /// ElapsedTime RAII object.
   StatsDuration &GetSymtabIndexTime() { return m_symtab_index_time; }
 
+  void ResetStatistics();
+
   /// \class LookupInfo Module.h "lldb/Core/Module.h"
   /// A class that encapsulates name lookup information.
   ///
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index 8419495da73a22..837b922ae77f75 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -422,6 +422,9 @@ class SymbolFile : public PluginInterface {
   /// hasn't been indexed yet, or a valid duration if it has.
   virtual StatsDuration::Duration GetDebugInfoIndexTime() { return {}; }
 
+  /// Reset the statistics for the symbol file.
+  virtual void ResetStatistics() {}
+
   /// Get the additional modules that this symbol file uses to parse debug info.
   ///
   /// Some debug info is stored in stand alone object files that are represented
diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index 8073d1816860e3..7a366bfabec865 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -182,6 +182,8 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
   lldb_private::StatsDuration::Duration GetDebugInfoParseTime() override;
   lldb_private::StatsDuration::Duration GetDebugInfoIndexTime() override;
 
+  void ResetStatistics() override;
+
   uint32_t GetAbilities() override;
 
   Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); }
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index f3414ae314f339..91a3ffb5bfa3d3 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -41,6 +41,8 @@ class StatsDuration {
   }
   operator Duration() const { return get(); }
 
+  void reset() { value.store(0, std::memory_order_relaxed); }
+
   StatsDuration &operator+=(Duration dur) {
     value.fetch_add(std::chrono::duration_cast<InternalDuration>(dur).count(),
                     std::memory_order_relaxed);
@@ -311,6 +313,16 @@ class DebuggerStats {
   ReportStatistics(Debugger &debugger, Target *target,
                    const lldb_private::StatisticsOptions &options);
 
+  /// Reset metrics associated with one or all targets in a debugger.
+  ///
+  /// \param debugger
+  ///   The debugger to reset the target list from if \a target is NULL.
+  ///
+  /// \param target
+  ///   The target to reset statistics for, or if null, reset statistics
+  ///   for all targets
+  static void ResetStatistics(Debugger &debugger, Target *target);
+
 protected:
   // Collecting stats can be set to true to collect stats that are expensive
   // to collect. By default all stats that are cheap to collect are enabled.
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 47931f1c16f9a3..4efec747aacff1 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1667,6 +1667,12 @@ SBTypeSynthetic SBDebugger::GetSyntheticForType(SBTypeNameSpecifier type_name) {
       DataVisualization::GetSyntheticForType(type_name.GetSP()));
 }
 
+void SBDebugger::ResetStatistics() {
+  LLDB_INSTRUMENT_VA(this);
+  if (m_opaque_sp)
+    DebuggerStats::ResetStatistics(*m_opaque_sp, nullptr);
+}
+
 static llvm::ArrayRef<const char *> GetCategoryArray(const char **categories) {
   if (categories == nullptr)
     return {};
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index d5017ad6bff166..b4cf8091e0f4bc 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -220,6 +220,13 @@ SBStructuredData SBTarget::GetStatistics(SBStatisticsOptions options) {
   return data;
 }
 
+void SBTarget::ResetStatistics() {
+  LLDB_INSTRUMENT_VA(this);
+  TargetSP target_sp(GetSP());
+  if (target_sp)
+    DebuggerStats::ResetStatistics(target_sp->GetDebugger(), target_sp.get());
+}
+
 void SBTarget::SetCollectingStats(bool v) {
   LLDB_INSTRUMENT_VA(this, v);
 
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 88cc957e91fac4..9972bbff3d8923 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1600,6 +1600,15 @@ bool Module::MergeArchitecture(const ArchSpec &arch_spec) {
   return SetArchitecture(merged_arch);
 }
 
+void Module::ResetStatistics() {
+  m_symtab_parse_time.reset();
+  m_symtab_index_time.reset();
+  SymbolFile *sym_file = GetSymbolFile();
+  if (sym_file) {
+    sym_file->ResetStatistics();
+  }
+}
+
 llvm::VersionTuple Module::GetVersion() {
   if (ObjectFile *obj_file = GetObjectFile())
     return obj_file->GetVersion();
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index fea3a4fd697389..8bdefc55d4f554 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -83,6 +83,8 @@ class DWARFIndex {
 
   StatsDuration::Duration GetIndexTime() { return m_index_time; }
 
+  void ResetStatistics() { m_index_time.reset(); }
+
 protected:
   Module &m_module;
   StatsDuration m_index_time;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 9287d4baf19e9c..860be681e8bd76 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4464,6 +4464,12 @@ StatsDuration::Duration SymbolFileDWARF::GetDebugInfoIndexTime() {
   return {};
 }
 
+void SymbolFileDWARF::ResetStatistics() {
+  m_parse_time.reset();
+  if (m_index)
+    return m_index->ResetStatistics();
+}
+
 Status SymbolFileDWARF::CalculateFrameVariableError(StackFrame &frame) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   CompileUnit *cu = frame.GetSymbolContext(eSymbolContextCompUnit).comp_unit;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 4967b37d753a09..ed540246001f02 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -318,6 +318,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
 
   StatsDuration &GetDebugInfoParseTimeRef() { return m_parse_time; }
 
+  void ResetStatistics() override;
+
   virtual lldb::offset_t
   GetVendorDWARFOpcodeSize(const DataExtractor &data,
                            const lldb::offset_t data_offset,
diff --git a/lldb/source/Symbol/SymbolFileOnDemand.cpp b/lldb/source/Symbol/SymbolFileOnDemand.cpp
index 0cfe9fc1514b5a..94979b2fb1c228 100644
--- a/lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ b/lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -555,6 +555,12 @@ StatsDuration::Duration SymbolFileOnDemand::GetDebugInfoIndexTime() {
   return m_sym_file_impl->GetDebugInfoIndexTime();
 }
 
+void SymbolFileOnDemand::ResetStatistics() {
+  LLDB_LOG(GetLog(), "[{0}] {1} is not skipped", GetSymbolFileName(),
+           __FUNCTION__);
+  return m_sym_file_impl->ResetStatistics();
+}
+
 void SymbolFileOnDemand::SetLoadDebugInfoEnabled() {
   if (m_debug_info_enabled)
     return;
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index ae2f65ea4c4bdc..bddb70902a30de 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -236,6 +236,22 @@ void TargetStats::IncreaseSourceRealpathCompatibleCount(uint32_t count) {
 
 bool DebuggerStats::g_collecting_stats = false;
 
+void DebuggerStats::ResetStatistics(Debugger &debugger, Target *target) {
+  std::lock_guard<std::recursive_mutex> guard(
+      Module::GetAllocationModuleCollectionMutex());
+  const uint64_t num_modules = target != nullptr
+                                   ? target->GetImages().GetSize()
+                                   : Module::GetNumberAllocatedModules();
+  for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
+    Module *module = target != nullptr
+                         ? target->GetImages().GetModuleAtIndex(image_idx).get()
+                         : Module::GetAllocatedModuleAtIndex(image_idx);
+    if (module == nullptr)
+      continue;
+    module->ResetStatistics();
+  }
+}
+
 llvm::json::Value DebuggerStats::ReportStatistics(
     Debugger &debugger, Target *target,
     const lldb_private::StatisticsOptions &options) {
@@ -261,14 +277,18 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   std::vector<ModuleStats> modules;
   std::lock_guard<std::recursive_mutex> guard(
       Module::GetAllocationModuleCollectionMutex());
-  const uint64_t num_modules = Module::GetNumberAllocatedModules();
+  const uint64_t num_modules = target != nullptr
+                                   ? target->GetImages().GetSize()
+                                   : Module::GetNumberAllocatedModules();
   uint32_t num_debug_info_enabled_modules = 0;
   uint32_t num_modules_has_debug_info = 0;
   uint32_t num_modules_with_variable_errors = 0;
   uint32_t num_modules_with_incomplete_types = 0;
   uint32_t num_stripped_modules = 0;
   for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
-    Module *module = Module::GetAllocatedModuleAtIndex(image_idx);
+    Module *module = target != nullptr
+                         ? target->GetImages().GetModuleAtIndex(image_idx).get()
+                         : Module::GetAllocatedModuleAtIndex(image_idx);
     ModuleStats module_stat;
     module_stat.symtab_parse_time = module->GetSymtabParseTime().get().count();
     module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index a0a9eeb6493207..54881c13bcb689 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -1,7 +1,8 @@
-import lldb
 import json
 import os
 import re
+
+import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -540,7 +541,7 @@ def test_no_dsym_binary_has_symfile_identifiers_in_stats(self):
         # in the stats.
         self.runCmd("b main.cpp:7")
 
-        debug_stats = self.get_stats()
+        debug_stats = self.get_stats("--all-targets")
 
         exe_stats = self.find_module_in_metrics(exe, debug_stats)
         # If we don't have a dSYM file, there should not be a key/value pair in
@@ -986,3 +987,38 @@ def test_summary_statistics_providers_vec(self):
         # 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)
+
+    @skipIfWindows
+    def test_multiple_targets(self):
+        """
+        Test statistics dump only reports the stats from current target and
+        "statistics dump --all-targets" includes all target stats.
+        """
+        da = {"CXX_SOURCES": "main.cpp", "EXE": self.getBuildArtifact("a.out")}
+        self.build(dictionary=da)
+        self.addTearDownCleanup(dictionary=da)
+
+        db = {"CXX_SOURCES": "second.cpp", "EXE": self.getBuildArtifact("second.out")}
+        self.build(dictionary=db)
+        self.addTearDownCleanup(dictionary=db)
+
+        main_exe = self.getBuildArtifact("a.out")
+        second_exe = self.getBuildArtifact("second.out")
+
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.cpp"), None, "a.out"
+        )
+        debugger_stats1 = self.get_stats()
+        self.assertIsNotNone(self.find_module_in_metrics(main_exe, debugger_stats1))
+        self.assertIsNone(self.find_module_in_metrics(second_exe, debugger_stats1))
+
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("second.cpp"), None, "second.out"
+        )
+        debugger_stats2 = self.get_stats()
+        self.assertIsNone(self.find_module_in_metrics(main_exe, debugger_stats2))
+        self.assertIsNotNone(self.find_module_in_metrics(second_exe, debugger_stats2))
+
+        all_targets_stats = self.get_stats("--all-targets")
+        self.assertIsNotNone(self.find_module_in_metrics(main_exe, all_targets_stats))
+        self.assertIsNotNone(self.find_module_in_metrics(second_exe, all_targets_stats))
diff --git a/lldb/test/API/commands/statistics/basic/second.cpp b/lldb/test/API/commands/statistics/basic/second.cpp
new file mode 100644
index 00000000000000..3af4e320c2fb53
--- /dev/null
+++ b/lldb/test/API/commands/statistics/basic/second.cpp
@@ -0,0 +1,5 @@
+// Test that the lldb command `statistics` works.
+
+int main(void) {
+  return 0; // break here
+}

>From 52b0207bcceffe646bf61919ca8b3cffbffa5993 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Mon, 28 Oct 2024 16:04:21 -0700
Subject: [PATCH 2/2] Address review feedback

---
 lldb/include/lldb/API/SBDebugger.h | 3 +++
 lldb/include/lldb/API/SBTarget.h   | 2 ++
 lldb/source/Core/Module.cpp        | 3 +--
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index d80d609b3e7a28..f1d6579c862e3e 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -426,6 +426,9 @@ class LLDB_API SBDebugger {
 
   SBTypeSynthetic GetSyntheticForType(SBTypeNameSpecifier);
 
+  /// Clear collected statistics for targets belonging to this debugger. This
+  /// includes clearing symbol table and debug info parsing/index time for all
+  /// modules.
   void ResetStatistics();
 
 #ifndef SWIG
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index 2e4392990bc4a8..a0546fd7a4e3de 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -102,6 +102,8 @@ class LLDB_API SBTarget {
   lldb::SBStructuredData GetStatistics(SBStatisticsOptions options);
 
   /// Reset the statistics collected for this target.
+  /// This includes clearing symbol table and debug info parsing/index time for
+  /// all modules.
   void ResetStatistics();
 
   /// Return the platform object associated with the target.
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 9972bbff3d8923..e0daf129e8af51 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1604,9 +1604,8 @@ void Module::ResetStatistics() {
   m_symtab_parse_time.reset();
   m_symtab_index_time.reset();
   SymbolFile *sym_file = GetSymbolFile();
-  if (sym_file) {
+  if (sym_file)
     sym_file->ResetStatistics();
-  }
 }
 
 llvm::VersionTuple Module::GetVersion() {



More information about the lldb-commits mailing list