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

jeffreytan81 · jeffreytan81 · web-flow · commit 24feaab8380c · 2024-11-17T20:36:54.000-08:00
"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 &lt;jeffreytan@fb.com&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,5 @@
+// Test that the lldb command `statistics` works.
+
+int main(void) {
+  return 0; // break here
+}
