[Lldb-commits] [lldb] Add options to "statistics dump" to control what sections are dumped (PR #95075)

via lldb-commits lldb-commits at lists.llvm.org
Mon Jun 10 21:55:56 PDT 2024


https://github.com/royitaqi updated https://github.com/llvm/llvm-project/pull/95075

>From f57f98f22425d3c869621b43b65da705d50b5934 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 17:04:11 -0700
Subject: [PATCH 1/4] Add --targets and --modules option to statistics dump

---
 lldb/include/lldb/Target/Statistics.h       |  2 ++
 lldb/source/Commands/CommandObjectStats.cpp |  6 ++++++
 lldb/source/Commands/Options.td             | 16 +++++++++++++---
 lldb/source/Target/Statistics.cpp           | 21 ++++++++++++++-------
 4 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index c04d529290fff..b24520b72a95d 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -133,6 +133,8 @@ struct ConstStringStats {
 struct StatisticsOptions {
   bool summary_only = false;
   bool load_all_debug_info = false;
+  bool include_targets = false;
+  bool include_modules = false;
   bool include_transcript = false;
 };
 
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
index 1935b0fdfadfb..1347b1ba25b1d 100644
--- a/lldb/source/Commands/CommandObjectStats.cpp
+++ b/lldb/source/Commands/CommandObjectStats.cpp
@@ -81,6 +81,12 @@ class CommandObjectStatsDump : public CommandObjectParsed {
       case 'f':
         m_stats_options.load_all_debug_info = true;
         break;
+      case 'r':
+        m_stats_options.include_targets = true;
+        break;
+      case 'm':
+        m_stats_options.include_modules = true;
+        break;
       case 't':
         m_stats_options.include_transcript = true;
         break;
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index cee5a81d3796b..45f46334d5c73 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -1416,16 +1416,26 @@ let Command = "trace schema" in {
 }
 
 let Command = "statistics dump" in {
-  def statistics_dump_all: Option<"all-targets", "a">, Group<1>,
+  def statistics_dump_all: Option<"all-targets", "a">, GroupRange<1, 2>,
     Desc<"Include statistics for all targets.">;
   def statistics_dump_summary: Option<"summary", "s">, Group<1>,
     Desc<"Dump only high-level summary statistics. "
          "Exclude targets, modules, breakpoints etc... details.">;
-  def statistics_dump_force: Option<"load-all-debug-info", "f">, Group<1>,
+  def statistics_dump_force: Option<"load-all-debug-info", "f">, GroupRange<1, 2>,
     Desc<"Dump the total possible debug info statistics. "
     "Force loading all the debug information if not yet loaded, and collect "
     "statistics with those.">;
-  def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
+  def statistics_dump_targets: Option<"targets", "r">, Group<2>,
+    Desc<"Dump statistics for the targets, including breakpoints, expression "
+    "evaluations, frame variables, etc. "
+    "If both the '--targets' and the '--modules' options are specified, a "
+    "list of module identifiers will be added to the 'targets' section.">;
+  def statistics_dump_modules: Option<"modules", "m">, Group<2>,
+    Desc<"Dump statistics for the modules, including time and size of various "
+    "aspects of the module and debug information, type system, path, etc. "
+    "If both the '--targets' and the '--modules' options are specified, a "
+    "list of module identifiers will be added to the 'targets' section.">;
+  def statistics_dump_transcript: Option<"transcript", "t">, Group<2>,
     Desc<"If the setting interpreter.save-transcript is enabled and this "
     "option is specified, include a JSON array with all commands the user and/"
     "or scripts executed during a debug session.">;
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 2a5300012511a..13c430d73990e 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -226,6 +226,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 
   const bool summary_only = options.summary_only;
   const bool load_all_debug_info = options.load_all_debug_info;
+  const bool include_targets = options.include_targets;
+  const bool include_modules = options.include_modules;
   const bool include_transcript = options.include_transcript;
 
   json::Array json_targets;
@@ -347,13 +349,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
       {"totalSymbolTableStripped", num_stripped_modules},
   };
 
-  if (target) {
-    json_targets.emplace_back(target->ReportStatistics(options));
-  } else {
-    for (const auto &target : debugger.GetTargetList().Targets())
+  if (!summary_only || include_targets) {
+    if (target) {
       json_targets.emplace_back(target->ReportStatistics(options));
+    } else {
+      for (const auto &target : debugger.GetTargetList().Targets())
+        json_targets.emplace_back(target->ReportStatistics(options));
+    }
+    global_stats.try_emplace("targets", std::move(json_targets));
   }
-  global_stats.try_emplace("targets", std::move(json_targets));
 
   ConstStringStats const_string_stats;
   json::Object json_memory{
@@ -362,11 +366,14 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   global_stats.try_emplace("memory", std::move(json_memory));
   if (!summary_only) {
     json::Value cmd_stats = debugger.GetCommandInterpreter().GetStatistics();
-    global_stats.try_emplace("modules", std::move(json_modules));
     global_stats.try_emplace("commands", std::move(cmd_stats));
   }
 
-  if (include_transcript) {
+  if (!summary_only || include_modules) {
+    global_stats.try_emplace("modules", std::move(json_modules));
+  }
+
+  if (!summary_only || include_transcript) {
     // When transcript is available, add it to the to-be-returned statistics.
     //
     // NOTE:

>From abfa4fd52f059f9500b223d43bfe50829ad376f2 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 17:04:11 -0700
Subject: [PATCH 2/4] Add --targets and --modules option to statistics dump

---
 lldb/include/lldb/Target/Statistics.h         |   2 +
 lldb/source/Commands/CommandObjectStats.cpp   |   6 ++
 lldb/source/Commands/Options.td               |  16 ++-
 lldb/source/Target/Statistics.cpp             |  33 ++++--
 .../commands/statistics/basic/TestStats.py    | 102 ++++++++++++++++--
 5 files changed, 140 insertions(+), 19 deletions(-)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index c04d529290fff..b24520b72a95d 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -133,6 +133,8 @@ struct ConstStringStats {
 struct StatisticsOptions {
   bool summary_only = false;
   bool load_all_debug_info = false;
+  bool include_targets = false;
+  bool include_modules = false;
   bool include_transcript = false;
 };
 
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
index 1935b0fdfadfb..1347b1ba25b1d 100644
--- a/lldb/source/Commands/CommandObjectStats.cpp
+++ b/lldb/source/Commands/CommandObjectStats.cpp
@@ -81,6 +81,12 @@ class CommandObjectStatsDump : public CommandObjectParsed {
       case 'f':
         m_stats_options.load_all_debug_info = true;
         break;
+      case 'r':
+        m_stats_options.include_targets = true;
+        break;
+      case 'm':
+        m_stats_options.include_modules = true;
+        break;
       case 't':
         m_stats_options.include_transcript = true;
         break;
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index cee5a81d3796b..45f46334d5c73 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -1416,16 +1416,26 @@ let Command = "trace schema" in {
 }
 
 let Command = "statistics dump" in {
-  def statistics_dump_all: Option<"all-targets", "a">, Group<1>,
+  def statistics_dump_all: Option<"all-targets", "a">, GroupRange<1, 2>,
     Desc<"Include statistics for all targets.">;
   def statistics_dump_summary: Option<"summary", "s">, Group<1>,
     Desc<"Dump only high-level summary statistics. "
          "Exclude targets, modules, breakpoints etc... details.">;
-  def statistics_dump_force: Option<"load-all-debug-info", "f">, Group<1>,
+  def statistics_dump_force: Option<"load-all-debug-info", "f">, GroupRange<1, 2>,
     Desc<"Dump the total possible debug info statistics. "
     "Force loading all the debug information if not yet loaded, and collect "
     "statistics with those.">;
-  def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
+  def statistics_dump_targets: Option<"targets", "r">, Group<2>,
+    Desc<"Dump statistics for the targets, including breakpoints, expression "
+    "evaluations, frame variables, etc. "
+    "If both the '--targets' and the '--modules' options are specified, a "
+    "list of module identifiers will be added to the 'targets' section.">;
+  def statistics_dump_modules: Option<"modules", "m">, Group<2>,
+    Desc<"Dump statistics for the modules, including time and size of various "
+    "aspects of the module and debug information, type system, path, etc. "
+    "If both the '--targets' and the '--modules' options are specified, a "
+    "list of module identifiers will be added to the 'targets' section.">;
+  def statistics_dump_transcript: Option<"transcript", "t">, Group<2>,
     Desc<"If the setting interpreter.save-transcript is enabled and this "
     "option is specified, include a JSON array with all commands the user and/"
     "or scripts executed during a debug session.">;
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 2a5300012511a..15c7dfe07ac87 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -108,6 +108,10 @@ TargetStats::ToJSON(Target &target,
   json::Object target_metrics_json;
   ProcessSP process_sp = target.GetProcessSP();
   const bool summary_only = options.summary_only;
+  const bool include_targets = options.include_targets;
+  const bool include_modules = options.include_modules;
+  const bool include_transcript = options.include_transcript;
+  const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
   if (!summary_only) {
     CollectStats(target);
 
@@ -117,8 +121,9 @@ TargetStats::ToJSON(Target &target,
 
     target_metrics_json.try_emplace(m_expr_eval.name, m_expr_eval.ToJSON());
     target_metrics_json.try_emplace(m_frame_var.name, m_frame_var.ToJSON());
-    target_metrics_json.try_emplace("moduleIdentifiers",
-                                    std::move(json_module_uuid_array));
+    if (include_default_sections || include_modules)
+      target_metrics_json.try_emplace("moduleIdentifiers",
+                                      std::move(json_module_uuid_array));
 
     if (m_launch_or_attach_time && m_first_private_stop_time) {
       double elapsed_time =
@@ -226,7 +231,10 @@ llvm::json::Value DebuggerStats::ReportStatistics(
 
   const bool summary_only = options.summary_only;
   const bool load_all_debug_info = options.load_all_debug_info;
+  const bool include_targets = options.include_targets;
+  const bool include_modules = options.include_modules;
   const bool include_transcript = options.include_transcript;
+  const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
 
   json::Array json_targets;
   json::Array json_modules;
@@ -314,7 +322,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
     if (module_stat.debug_info_had_incomplete_types)
       ++num_modules_with_incomplete_types;
 
-    if (!summary_only) {
+    if (include_default_sections || include_modules) {
       module_stat.identifier = (intptr_t)module;
       module_stat.path = module->GetFileSpec().GetPath();
       if (ConstString object_name = module->GetObjectName()) {
@@ -347,13 +355,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
       {"totalSymbolTableStripped", num_stripped_modules},
   };
 
-  if (target) {
-    json_targets.emplace_back(target->ReportStatistics(options));
-  } else {
-    for (const auto &target : debugger.GetTargetList().Targets())
+  if (include_default_sections || include_targets) {
+    if (target) {
       json_targets.emplace_back(target->ReportStatistics(options));
+    } else {
+      for (const auto &target : debugger.GetTargetList().Targets())
+        json_targets.emplace_back(target->ReportStatistics(options));
+    }
+    global_stats.try_emplace("targets", std::move(json_targets));
   }
-  global_stats.try_emplace("targets", std::move(json_targets));
 
   ConstStringStats const_string_stats;
   json::Object json_memory{
@@ -362,11 +372,14 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   global_stats.try_emplace("memory", std::move(json_memory));
   if (!summary_only) {
     json::Value cmd_stats = debugger.GetCommandInterpreter().GetStatistics();
-    global_stats.try_emplace("modules", std::move(json_modules));
     global_stats.try_emplace("commands", std::move(cmd_stats));
   }
 
-  if (include_transcript) {
+  if (include_default_sections || include_modules) {
+    global_stats.try_emplace("modules", std::move(json_modules));
+  }
+
+  if (include_default_sections || include_transcript) {
     // When transcript is available, add it to the to-be-returned statistics.
     //
     // NOTE:
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index ebe6166eb45e5..2132d2df03b4b 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -657,16 +657,106 @@ def test_transcript_happy_path(self):
         # The second "statistics dump" in the transcript should have no output
         self.assertNotIn("output", transcript[2])
 
-    def test_transcript_should_not_exist_when_not_asked_for(self):
+    def test_sections_existence(self):
         """
-        Test "statistics dump" and the transcript information.
+        Test "statistics dump" and the existence of sections when different
+        options are given.
         """
         self.build()
         exe = self.getBuildArtifact("a.out")
         target = self.createTestTarget(file_path=exe)
         self.runCmd("settings set interpreter.save-transcript true")
-        self.runCmd("version")
 
-        # Verify the output of a first "statistics dump"
-        debug_stats = self.get_stats()  # Not with "--transcript"
-        self.assertNotIn("transcript", debug_stats)
+        test_cases = [
+            {   # statistics dump
+                "options": "",
+                "expect": {
+                    "commands": True,
+                    "targets": True,
+                    "targets.moduleIdentifiers": True,
+                    "targets.breakpoints": True,
+                    "modules": True,
+                    "transcript": True,
+                },
+            },
+            {   # statistics dump --summary
+                "options": " --summary",
+                "expect": {
+                    "commands": False,
+                    "targets": False,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": False,
+                    "modules": False,
+                    "transcript": False,
+                },
+            },
+            {   # statistics dump --targets
+                "options": " --targets",
+                "expect": {
+                    "commands": True,
+                    "targets": True,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": True,
+                    "modules": False,
+                    "transcript": False,
+                },
+            },
+            {   # statistics dump --modules
+                "options": " --modules",
+                "expect": {
+                    "commands": True,
+                    "targets": False,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": False,
+                    "modules": True,
+                    "transcript": False,
+                },
+            },
+            {   # statistics dump --targets --modules
+                "options": " --targets --modules",
+                "expect": {
+                    "commands": True,
+                    "targets": True,
+                    "targets.moduleIdentifiers": True,
+                    "targets.breakpoints": True,
+                    "modules": True,
+                    "transcript": False,
+                },
+            },
+            {   # statistics dump --transcript
+                "options": " --transcript",
+                "expect": {
+                    "commands": True,
+                    "targets": False,
+                    "targets.moduleIdentifiers": False,
+                    "targets.breakpoints": False,
+                    "modules": False,
+                    "transcript": True,
+                },
+            },
+        ]
+
+        for test_case in test_cases:
+            options = test_case["options"]
+            debug_stats = self.get_stats(options)
+            # The following fields should always exist
+            self.assertIn("totalDebugInfoEnabled", debug_stats, "Global stats should always exist")
+            self.assertIn("memory", debug_stats, "'memory' should always exist")
+            # The following fields should exist/not exist depending on the test case
+            for field_name in test_case["expect"]:
+                idx = field_name.find(".")
+                if idx == -1:
+                    # `field` is a top-level field
+                    exists = field_name in debug_stats
+                    should_exist = test_case["expect"][field_name]
+                    should_exist_string = "" if should_exist else "not "
+                    self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")
+                else:
+                    # `field` is a string of "<top-level field>.<second-level field>"
+                    top_level_field_name = field_name[0:idx]
+                    second_level_field_name = field_name[idx+1:]
+                    for top_level_field in debug_stats[top_level_field_name] if top_level_field_name in debug_stats else []:
+                        exists = second_level_field_name in top_level_field
+                        should_exist = test_case["expect"][field_name]
+                        should_exist_string = "" if should_exist else "not "
+                        self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")

>From 358497da7fb85394aceca4a14ea55e7649e5d4db Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 21:54:40 -0700
Subject: [PATCH 3/4] Fix C++ format

---
 lldb/source/Target/Statistics.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 15c7dfe07ac87..bbb556925eb1f 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -111,7 +111,8 @@ TargetStats::ToJSON(Target &target,
   const bool include_targets = options.include_targets;
   const bool include_modules = options.include_modules;
   const bool include_transcript = options.include_transcript;
-  const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
+  const bool include_default_sections = !summary_only && !include_targets &&
+                                        !include_modules && !include_transcript;
   if (!summary_only) {
     CollectStats(target);
 
@@ -234,7 +235,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   const bool include_targets = options.include_targets;
   const bool include_modules = options.include_modules;
   const bool include_transcript = options.include_transcript;
-  const bool include_default_sections = !summary_only && !include_targets && !include_modules && !include_transcript;
+  const bool include_default_sections = !summary_only && !include_targets &&
+                                        !include_modules && !include_transcript;
 
   json::Array json_targets;
   json::Array json_modules;

>From 32e51e0e4f1debfc3338af8a8f226f02ec7187f7 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Mon, 10 Jun 2024 21:55:44 -0700
Subject: [PATCH 4/4] Fix python format

---
 .../commands/statistics/basic/TestStats.py    | 36 +++++++++++++------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 2132d2df03b4b..db254e36c4b6c 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -668,7 +668,7 @@ def test_sections_existence(self):
         self.runCmd("settings set interpreter.save-transcript true")
 
         test_cases = [
-            {   # statistics dump
+            {  # statistics dump
                 "options": "",
                 "expect": {
                     "commands": True,
@@ -679,7 +679,7 @@ def test_sections_existence(self):
                     "transcript": True,
                 },
             },
-            {   # statistics dump --summary
+            {  # statistics dump --summary
                 "options": " --summary",
                 "expect": {
                     "commands": False,
@@ -690,7 +690,7 @@ def test_sections_existence(self):
                     "transcript": False,
                 },
             },
-            {   # statistics dump --targets
+            {  # statistics dump --targets
                 "options": " --targets",
                 "expect": {
                     "commands": True,
@@ -701,7 +701,7 @@ def test_sections_existence(self):
                     "transcript": False,
                 },
             },
-            {   # statistics dump --modules
+            {  # statistics dump --modules
                 "options": " --modules",
                 "expect": {
                     "commands": True,
@@ -712,7 +712,7 @@ def test_sections_existence(self):
                     "transcript": False,
                 },
             },
-            {   # statistics dump --targets --modules
+            {  # statistics dump --targets --modules
                 "options": " --targets --modules",
                 "expect": {
                     "commands": True,
@@ -723,7 +723,7 @@ def test_sections_existence(self):
                     "transcript": False,
                 },
             },
-            {   # statistics dump --transcript
+            {  # statistics dump --transcript
                 "options": " --transcript",
                 "expect": {
                     "commands": True,
@@ -740,7 +740,9 @@ def test_sections_existence(self):
             options = test_case["options"]
             debug_stats = self.get_stats(options)
             # The following fields should always exist
-            self.assertIn("totalDebugInfoEnabled", debug_stats, "Global stats should always exist")
+            self.assertIn(
+                "totalDebugInfoEnabled", debug_stats, "Global stats should always exist"
+            )
             self.assertIn("memory", debug_stats, "'memory' should always exist")
             # The following fields should exist/not exist depending on the test case
             for field_name in test_case["expect"]:
@@ -750,13 +752,25 @@ def test_sections_existence(self):
                     exists = field_name in debug_stats
                     should_exist = test_case["expect"][field_name]
                     should_exist_string = "" if should_exist else "not "
-                    self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")
+                    self.assertEqual(
+                        exists,
+                        should_exist,
+                        f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
+                    )
                 else:
                     # `field` is a string of "<top-level field>.<second-level field>"
                     top_level_field_name = field_name[0:idx]
-                    second_level_field_name = field_name[idx+1:]
-                    for top_level_field in debug_stats[top_level_field_name] if top_level_field_name in debug_stats else []:
+                    second_level_field_name = field_name[idx + 1 :]
+                    for top_level_field in (
+                        debug_stats[top_level_field_name]
+                        if top_level_field_name in debug_stats
+                        else []
+                    ):
                         exists = second_level_field_name in top_level_field
                         should_exist = test_case["expect"][field_name]
                         should_exist_string = "" if should_exist else "not "
-                        self.assertEqual(exists, should_exist, f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'")
+                        self.assertEqual(
+                            exists,
+                            should_exist,
+                            f"'{field_name}' should {should_exist_string}exist for 'statistics dump{options}'",
+                        )



More information about the lldb-commits mailing list