[lldb] Add/change options in statistics dump to control what sections are dumped (#95075)

royitaqi · web-flow · commit 70f41a8c3054 · 2024-06-18T17:21:20.000-07:00
# Added/changed options

The following options are **added** to the `statistics dump` command:
* `--targets=bool`: Boolean. Dumps the `targets` section.
* `--modules=bool`: Boolean. Dumps the `modules` section.
When both options are given, the field `moduleIdentifiers` will be
dumped for each target in the `targets` section.

The following options are **changed**:
* `--transcript=bool`: Changed to a boolean. Dumps the `transcript`
section.

# Behavior of `statistics dump` with various options

The behavior is **backward compatible**:
- When no options are provided, `statistics dump` dumps all sections.
- When `--summary` is provided, only dumps the summary info.

**New** behavior:
- `--targets=bool`, `--modules=bool`, `--transcript=bool` overrides the
above "default".

For **example**:
- `statistics dump --modules=false` dumps summary + targets +
transcript. No modules.
- `statistics dump --summary --targets=true --transcript=true` dumps
summary + targets (in summary mode) + transcript.


# Added options into public API

In `SBStatisticsOptions`, add:
* `Set/GetIncludeTargets`
* `Set/GetIncludeModules`
* `Set/GetIncludeTranscript`

**Alternative considered**: Thought about adding
`Set/GetIncludeSections(string sections_spec)`, which receives a
comma-separated list of section names to be included ("targets",
"modules", "transcript"). The **benefit** of this approach is that the
API is more future-proof when it comes to possible adding/changing of
section names. **However**, I feel the section names are likely to
remain unchanged for a while - it's not like we plan to make big changes
to the output of `statistics dump` any time soon. The **downsides** of
this approach are: 1\ the readability of the API is worse (requires
reading doc to understand what string can be accepted), 2\ string input
are more prone to human error (e.g. typo "target" instead of expected
"targets").


# Tests

```
bin/llvm-lit -sv ../external/llvm-project/lldb/test/API/commands/statistics/basic/TestStats.py
```

```
./tools/lldb/unittests/Interpreter/InterpreterTests
```

New test cases have been added to verify:
* Different sections are dumped/not dumped when different
`StatisticsOptions` are given through command line (CLI or
`HandleCommand`; see `test_sections_existence_through_command`) or API
(see `test_sections_existence_through_api`).
* The order in which the options are given in command line does not
matter (see `test_order_of_options_do_not_matter`).

---------

Co-authored-by: Roy Shi &lt;royshi@meta.com&gt;
diff --git a/lldb/include/lldb/API/SBStatisticsOptions.h b/lldb/include/lldb/API/SBStatisticsOptions.h
@@ -22,9 +22,46 @@ class LLDB_API SBStatisticsOptions {
 
   const SBStatisticsOptions &operator=(const lldb::SBStatisticsOptions &rhs);
 
+  /// If true, dump only high-level summary statistics. Exclude details like
+  /// targets, modules, breakpoints, etc. This turns off `IncludeTargets`,
+  /// `IncludeModules` and `IncludeTranscript` by default.
+  ///
+  /// Defaults to false.
   void SetSummaryOnly(bool b);
   bool GetSummaryOnly();
 
+  /// If true, dump statistics for the targets, including breakpoints,
+  /// expression evaluations, frame variables, etc.
+  ///
+  /// Defaults to true, unless the `SummaryOnly` mode is enabled, in which case
+  /// this is turned off unless specified.
+  ///
+  /// If both `IncludeTargets` and `IncludeModules` are true, a list of module
+  /// identifiers will be added to the "targets" section.
+  void SetIncludeTargets(bool b);
+  bool GetIncludeTargets() const;
+
+  /// If true, dump statistics for the modules, including time and size of
+  /// various aspects of the module and debug information, type system, path,
+  /// etc.
+  ///
+  /// Defaults to true, unless the `SummaryOnly` mode is enabled, in which case
+  /// this is turned off unless specified.
+  ///
+  /// If both `IncludeTargets` and `IncludeModules` are true, a list of module
+  /// identifiers will be added to the "targets" section.
+  void SetIncludeModules(bool b);
+  bool GetIncludeModules() const;
+
+  /// If true and the setting `interpreter.save-transcript` is enabled, include
+  /// a JSON array with all commands the user and/or scripts executed during a
+  /// debug session.
+  ///
+  /// Defaults to true, unless the `SummaryOnly` mode is enabled, in which case
+  /// this is turned off unless specified.
+  void SetIncludeTranscript(bool b);
+  bool GetIncludeTranscript() const;
+
   /// If set to true, the debugger will load all debug info that is available
   /// and report statistics on the total amount. If this is set to false, then
   /// only report statistics on the currently loaded debug information.
diff --git a/lldb/include/lldb/Interpreter/OptionArgParser.h b/lldb/include/lldb/Interpreter/OptionArgParser.h
@@ -29,6 +29,9 @@ struct OptionArgParser {
 
   static bool ToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr);
 
+  static llvm::Expected<bool> ToBoolean(llvm::StringRef option_name,
+                                        llvm::StringRef option_arg);
+
   static char ToChar(llvm::StringRef s, char fail_value, bool *success_ptr);
 
   static int64_t ToOptionEnum(llvm::StringRef s,
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
@@ -131,9 +131,48 @@ struct ConstStringStats {
 };
 
 struct StatisticsOptions {
-  bool summary_only = false;
-  bool load_all_debug_info = false;
-  bool include_transcript = false;
+public:
+  void SetSummaryOnly(bool value) { m_summary_only = value; }
+  bool GetSummaryOnly() const { return m_summary_only.value_or(false); }
+
+  void SetLoadAllDebugInfo(bool value) { m_load_all_debug_info = value; }
+  bool GetLoadAllDebugInfo() const {
+    return m_load_all_debug_info.value_or(false);
+  }
+
+  void SetIncludeTargets(bool value) { m_include_targets = value; }
+  bool GetIncludeTargets() const {
+    if (m_include_targets.has_value())
+      return m_include_targets.value();
+    // `m_include_targets` has no value set, so return a value based on
+    // `m_summary_only`.
+    return !GetSummaryOnly();
+  }
+
+  void SetIncludeModules(bool value) { m_include_modules = value; }
+  bool GetIncludeModules() const {
+    if (m_include_modules.has_value())
+      return m_include_modules.value();
+    // `m_include_modules` has no value set, so return a value based on
+    // `m_summary_only`.
+    return !GetSummaryOnly();
+  }
+
+  void SetIncludeTranscript(bool value) { m_include_transcript = value; }
+  bool GetIncludeTranscript() const {
+    if (m_include_transcript.has_value())
+      return m_include_transcript.value();
+    // `m_include_transcript` has no value set, so return a value based on
+    // `m_summary_only`.
+    return !GetSummaryOnly();
+  }
+
+private:
+  std::optional<bool> m_summary_only;
+  std::optional<bool> m_load_all_debug_info;
+  std::optional<bool> m_include_targets;
+  std::optional<bool> m_include_modules;
+  std::optional<bool> m_include_transcript;
 };
 
 /// A class that represents statistics for a since lldb_private::Target.
diff --git a/lldb/source/API/SBStatisticsOptions.cpp b/lldb/source/API/SBStatisticsOptions.cpp
@@ -18,7 +18,6 @@ using namespace lldb_private;
 SBStatisticsOptions::SBStatisticsOptions()
     : m_opaque_up(new StatisticsOptions()) {
   LLDB_INSTRUMENT_VA(this);
-  m_opaque_up->summary_only = false;
 }
 
 SBStatisticsOptions::SBStatisticsOptions(const SBStatisticsOptions &rhs) {
@@ -39,17 +38,43 @@ SBStatisticsOptions::operator=(const SBStatisticsOptions &rhs) {
 }
 
 void SBStatisticsOptions::SetSummaryOnly(bool b) {
-  m_opaque_up->summary_only = b;
+  m_opaque_up->SetSummaryOnly(b);
 }
 
-bool SBStatisticsOptions::GetSummaryOnly() { return m_opaque_up->summary_only; }
+bool SBStatisticsOptions::GetSummaryOnly() {
+  return m_opaque_up->GetSummaryOnly();
+}
+
+void SBStatisticsOptions::SetIncludeTargets(bool b) {
+  m_opaque_up->SetIncludeTargets(b);
+}
+
+bool SBStatisticsOptions::GetIncludeTargets() const {
+  return m_opaque_up->GetIncludeTargets();
+}
+
+void SBStatisticsOptions::SetIncludeModules(bool b) {
+  m_opaque_up->SetIncludeModules(b);
+}
+
+bool SBStatisticsOptions::GetIncludeModules() const {
+  return m_opaque_up->GetIncludeModules();
+}
+
+void SBStatisticsOptions::SetIncludeTranscript(bool b) {
+  m_opaque_up->SetIncludeTranscript(b);
+}
+
+bool SBStatisticsOptions::GetIncludeTranscript() const {
+  return m_opaque_up->GetIncludeTranscript();
+}
 
 void SBStatisticsOptions::SetReportAllAvailableDebugInfo(bool b) {
-  m_opaque_up->load_all_debug_info = b;
+  m_opaque_up->SetLoadAllDebugInfo(b);
 }
 
 bool SBStatisticsOptions::GetReportAllAvailableDebugInfo() {
-  return m_opaque_up->load_all_debug_info;
+  return m_opaque_up->GetLoadAllDebugInfo();
 }
 
 const lldb_private::StatisticsOptions &SBStatisticsOptions::ref() const {
diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandOptionArgumentTable.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Target/Target.h"
 
 using namespace lldb;
@@ -76,13 +77,31 @@ class CommandObjectStatsDump : public CommandObjectParsed {
         m_all_targets = true;
         break;
       case 's':
-        m_stats_options.summary_only = true;
+        m_stats_options.SetSummaryOnly(true);
         break;
       case 'f':
-        m_stats_options.load_all_debug_info = true;
+        m_stats_options.SetLoadAllDebugInfo(true);
+        break;
+      case 'r':
+        if (llvm::Expected<bool> bool_or_error =
+                OptionArgParser::ToBoolean("--targets", option_arg))
+          m_stats_options.SetIncludeTargets(*bool_or_error);
+        else
+          error = bool_or_error.takeError();
+        break;
+      case 'm':
+        if (llvm::Expected<bool> bool_or_error =
+                OptionArgParser::ToBoolean("--modules", option_arg))
+          m_stats_options.SetIncludeModules(*bool_or_error);
+        else
+          error = bool_or_error.takeError();
         break;
       case 't':
-        m_stats_options.include_transcript = true;
+        if (llvm::Expected<bool> bool_or_error =
+                OptionArgParser::ToBoolean("--transcript", option_arg))
+          m_stats_options.SetIncludeTranscript(*bool_or_error);
+        else
+          error = bool_or_error.takeError();
         break;
       default:
         llvm_unreachable("Unimplemented option");
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
@@ -1425,8 +1425,28 @@ let Command = "statistics dump" in {
     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_targets: Option<"targets", "r">, Group<1>,
+    Arg<"Boolean">,
+    Desc<"Dump statistics for the targets, including breakpoints, expression "
+    "evaluations, frame variables, etc. "
+    "Defaults to true, unless the '--summary' mode is enabled, in which case "
+    "this is turned off unless specified. "
+    "If both the '--targets' and the '--modules' options are 'true', a list "
+    "of module identifiers will be added to the 'targets' section.">;
+  def statistics_dump_modules: Option<"modules", "m">, Group<1>,
+    Arg<"Boolean">,
+    Desc<"Dump statistics for the modules, including time and size of various "
+    "aspects of the module and debug information, type system, path, etc. "
+    "Defaults to true, unless the '--summary' mode is enabled, in which case "
+    "this is turned off unless specified. "
+    "If both the '--targets' and the '--modules' options are 'true', a list "
+    "of module identifiers will be added to the 'targets' section.">;
   def statistics_dump_transcript: Option<"transcript", "t">, Group<1>,
+    Arg<"Boolean">,
     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.">;
+    "option is 'true', include a JSON array with all commands the user and/or "
+    "scripts executed during a debug session. "
+    "Defaults to true, unless the '--summary' mode is enabled, in which case "
+    "this is turned off unless specified.">;
+
 }
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -35,6 +35,20 @@ bool OptionArgParser::ToBoolean(llvm::StringRef ref, bool fail_value,
   return fail_value;
 }
 
+llvm::Expected<bool> OptionArgParser::ToBoolean(llvm::StringRef option_name,
+                                                llvm::StringRef option_arg) {
+  bool parse_success;
+  const bool option_value =
+      ToBoolean(option_arg, false /* doesn't matter */, &parse_success);
+  if (parse_success)
+    return option_value;
+  else
+    return llvm::createStringError(
+        "Invalid boolean value for option '%s': '%s'",
+        option_name.str().c_str(),
+        option_arg.empty() ? "<null>" : option_arg.str().c_str());
+}
+
 char OptionArgParser::ToChar(llvm::StringRef s, char fail_value,
                              bool *success_ptr) {
   if (success_ptr)
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
@@ -107,7 +107,8 @@ TargetStats::ToJSON(Target &target,
                     const lldb_private::StatisticsOptions &options) {
   json::Object target_metrics_json;
   ProcessSP process_sp = target.GetProcessSP();
-  const bool summary_only = options.summary_only;
+  const bool summary_only = options.GetSummaryOnly();
+  const bool include_modules = options.GetIncludeModules();
   if (!summary_only) {
     CollectStats(target);
 
@@ -117,8 +118,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_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 =
@@ -224,9 +226,11 @@ llvm::json::Value DebuggerStats::ReportStatistics(
     Debugger &debugger, Target *target,
     const lldb_private::StatisticsOptions &options) {
 
-  const bool summary_only = options.summary_only;
-  const bool load_all_debug_info = options.load_all_debug_info;
-  const bool include_transcript = options.include_transcript;
+  const bool summary_only = options.GetSummaryOnly();
+  const bool load_all_debug_info = options.GetLoadAllDebugInfo();
+  const bool include_targets = options.GetIncludeTargets();
+  const bool include_modules = options.GetIncludeModules();
+  const bool include_transcript = options.GetIncludeTranscript();
 
   json::Array json_targets;
   json::Array json_modules;
@@ -314,7 +318,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
     if (module_stat.debug_info_had_incomplete_types)
       ++num_modules_with_incomplete_types;
 
-    if (!summary_only) {
+    if (include_modules) {
       module_stat.identifier = (intptr_t)module;
       module_stat.path = module->GetFileSpec().GetPath();
       if (ConstString object_name = module->GetObjectName()) {
@@ -347,13 +351,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_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,10 +368,13 @@ 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_modules) {
+    global_stats.try_emplace("modules", std::move(json_modules));
+  }
+
   if (include_transcript) {
     // When transcript is available, add it to the to-be-returned statistics.
     //
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
diff --git a/lldb/unittests/Interpreter/TestOptionArgParser.cpp b/lldb/unittests/Interpreter/TestOptionArgParser.cpp
