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

via lldb-commits lldb-commits at lists.llvm.org
Sun Nov 17 20:36:57 PST 2024


Author: jeffreytan81
Date: 2024-11-17T20:36:54-08:00
New Revision: 24feaab8380c69d5fa3eb8c21ef2d660913fd4a9

URL: https://github.com/llvm/llvm-project/commit/24feaab8380c69d5fa3eb8c21ef2d660913fd4a9
DIFF: https://github.com/llvm/llvm-project/commit/24feaab8380c69d5fa3eb8c21ef2d660913fd4a9.diff

LOG: Fix statistics dump to report per-target (#113723)

"statistics dump" currently report the statistics of all targets in
debugger instead of current target. This is wrong because there is a
"statistics dump --all-targets" option that supposed to include
everything.

This PR fixes the issue by only report statistics for current target
instead of all. It also includes the change to reset statistics debug
info/symbol table parsing/indexing time during debugger destroy. This is
required so that we report current statistics if we plan to reuse
lldb/lldb-dap across debug sessions

---------

Co-authored-by: jeffreytan81 <jeffreytan at fb.com>

Added: 
    lldb/test/API/commands/statistics/basic/second.cpp

Modified: 
    lldb/include/lldb/API/SBDebugger.h
    lldb/include/lldb/API/SBTarget.h
    lldb/include/lldb/Breakpoint/Breakpoint.h
    lldb/include/lldb/Core/Module.h
    lldb/include/lldb/Symbol/SymbolFile.h
    lldb/include/lldb/Symbol/SymbolFileOnDemand.h
    lldb/include/lldb/Target/Statistics.h
    lldb/include/lldb/Target/Target.h
    lldb/source/API/SBDebugger.cpp
    lldb/source/API/SBTarget.cpp
    lldb/source/Breakpoint/Breakpoint.cpp
    lldb/source/Core/Module.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
    lldb/source/Symbol/SymbolFileOnDemand.cpp
    lldb/source/Target/Statistics.cpp
    lldb/source/Target/Target.cpp
    lldb/test/API/commands/statistics/basic/TestStats.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 6afa1c932ab050..1f42ec3cdc7d51 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -426,6 +426,11 @@ 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, breakpoint resolve time and target statistics.
+  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..9b97359d49cf9e 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -101,6 +101,11 @@ class LLDB_API SBTarget {
   ///     A SBStructuredData with the statistics collected.
   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, breakpoint resolve time and target statistics.
+  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/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 0b6bf593be37a9..f623a2e0c295b9 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -588,6 +588,8 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>,
   /// Get statistics associated with this breakpoint in JSON format.
   llvm::json::Value GetStatistics();
 
+  void ResetStatistics();
+
   /// Get the time it took to resolve all locations in this breakpoint.
   StatsDuration::Duration GetResolveTime() const { return m_resolve_time; }
 

diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 23257e429ad0d6..90e0f4b6a3aac7 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -881,6 +881,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..ee365357fcf31f 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);
@@ -201,6 +203,8 @@ class SummaryStatistics {
 
   llvm::json::Value ToJSON() const;
 
+  void Reset() { m_total_time.reset(); }
+
   /// Basic RAII class to increment the summary count when the call is complete.
   class SummaryInvocation {
   public:
@@ -250,6 +254,8 @@ class SummaryStatisticsCache {
 
   llvm::json::Value ToJSON();
 
+  void Reset();
+
 private:
   llvm::StringMap<SummaryStatisticsSP> m_summary_stats_map;
   std::mutex m_map_mutex;
@@ -271,6 +277,7 @@ class TargetStats {
   StatsDuration &GetCreateTime() { return m_create_time; }
   StatsSuccessFail &GetExpressionStats() { return m_expr_eval; }
   StatsSuccessFail &GetFrameVariableStats() { return m_frame_var; }
+  void Reset(Target &target);
 
 protected:
   StatsDuration m_create_time;
@@ -311,6 +318,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/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index b7a5dacada4fe8..0d1943450d622b 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1635,6 +1635,8 @@ class Target : public std::enable_shared_from_this<Target>,
   llvm::json::Value
   ReportStatistics(const lldb_private::StatisticsOptions &options);
 
+  void ResetStatistics();
+
   TargetStats &GetStatistics() { return m_stats; }
 
 protected:

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 3c4c3d8d85292e..2a33161bc21edc 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/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index 54ebafc3f65b5c..337c1a4ac401fa 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -1138,3 +1138,5 @@ json::Value Breakpoint::GetStatistics() {
   }
   return json::Value(std::move(bp));
 }
+
+void Breakpoint::ResetStatistics() { m_resolve_time.reset(); }

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 03eb81459b29bc..9601c834d9b8fe 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1609,6 +1609,14 @@ 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 ac1f75e91c2195..15d85033434e70 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -91,6 +91,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 666595a6d0635f..72276464723d08 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4467,6 +4467,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..8173d20457e21b 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -201,6 +201,24 @@ TargetStats::ToJSON(Target &target,
   return target_metrics_json;
 }
 
+void TargetStats::Reset(Target &target) {
+  m_launch_or_attach_time.reset();
+  m_first_private_stop_time.reset();
+  m_first_public_stop_time.reset();
+  // Report both the normal breakpoint list and the internal breakpoint list.
+  for (int i = 0; i < 2; ++i) {
+    BreakpointList &breakpoints = target.GetBreakpointList(i == 1);
+    std::unique_lock<std::recursive_mutex> lock;
+    breakpoints.GetListMutex(lock);
+    size_t num_breakpoints = breakpoints.GetSize();
+    for (size_t i = 0; i < num_breakpoints; i++) {
+      Breakpoint *bp = breakpoints.GetBreakpointAtIndex(i).get();
+      bp->ResetStatistics();
+    }
+  }
+  target.GetSummaryStatisticsCache().Reset();
+}
+
 void TargetStats::SetLaunchOrAttachTime() {
   m_launch_or_attach_time = StatsClock::now();
   m_first_private_stop_time = std::nullopt;
@@ -236,6 +254,28 @@ 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();
+  }
+  if (target)
+    target->ResetStatistics();
+  else {
+    for (const auto &target : debugger.GetTargetList().Targets())
+      target->ResetStatistics();
+  }
+}
+
 llvm::json::Value DebuggerStats::ReportStatistics(
     Debugger &debugger, Target *target,
     const lldb_private::StatisticsOptions &options) {
@@ -261,14 +301,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();
@@ -440,3 +484,8 @@ json::Value SummaryStatisticsCache::ToJSON() {
 
   return json_summary_stats;
 }
+
+void SummaryStatisticsCache::Reset() {
+  for (const auto &summary_stat : m_summary_stats_map)
+    summary_stat.second->Reset();
+}

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 300dca163c41a7..d70274a4b7c7c4 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -5149,3 +5149,5 @@ llvm::json::Value
 Target::ReportStatistics(const lldb_private::StatisticsOptions &options) {
   return m_stats.ToJSON(*this, options);
 }
+
+void Target::ResetStatistics() { m_stats.Reset(*this); }

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
+}


        


More information about the lldb-commits mailing list