In statistics dump --summary, add back the targets section (#97004)

# Change #95075 accidentally removed the `targets` section from `statistics dump --summary`. Adding it back, by setting the default value to `true` in `StatisticsOptions::GetIncludeTargets()`. Updated the description for the options. Updated tests. # Verification Manually verified the fix by running `statist dump --summary` and comparing three versions of LLDB (in commit order): 1. Before #95075 2. After #95075 3. After this fix The expected result is that 1 and 3 give the same sections, while 2 is missing the `targets` section when in summary mode. The output (see Appendix) matches the expectation. # Appendix: Manual Test Output ## `statistics dump --summary` of 1 ``` (lldb) statistics dump --summary { "memory": { "strings": { "bytesTotal": 724992, "bytesUnused": 714547, "bytesUsed": 10445 } }, "targets": [ { "sourceMapDeduceCount": 0, "totalSharedLibraryEventHitCount": 0 } ], "totalDebugInfoByteSize": 597, "totalDebugInfoEnabled": 1, "totalDebugInfoIndexLoadedFromCache": 0, "totalDebugInfoIndexSavedToCache": 0, "totalDebugInfoIndexTime": 0.00070699999999999995, "totalDebugInfoParseTime": 2.5999999999999998e-05, "totalModuleCount": 1, "totalModuleCountHasDebugInfo": 1, "totalModuleCountWithIncompleteTypes": 0, "totalModuleCountWithVariableErrors": 0, "totalSymbolTableIndexTime": 0.000223, "totalSymbolTableParseTime": 0.00025799999999999998, "totalSymbolTableStripped": 0, "totalSymbolTablesLoadedFromCache": 0, "totalSymbolTablesSavedToCache": 0 } (lldb) ``` ## `statistics dump --summary` of 3 Should be the same as above. ``` (lldb) statistics dump --summary { "memory": { "strings": { "bytesTotal": 516096, "bytesUnused": 510353, "bytesUsed": 5743 } }, "targets": [ { "sourceMapDeduceCount": 0, "totalSharedLibraryEventHitCount": 0 } ], "totalDebugInfoByteSize": 597, "totalDebugInfoEnabled": 1, "totalDebugInfoIndexLoadedFromCache": 0, "totalDebugInfoIndexSavedToCache": 0, "totalDebugInfoIndexTime": 0.0022139999999999998, "totalDebugInfoParseTime": 0.00031700000000000001, "totalModuleCount": 1, "totalModuleCountHasDebugInfo": 1, "totalModuleCountWithIncompleteTypes": 0, "totalModuleCountWithVariableErrors": 0, "totalSymbolTableIndexTime": 0.0014499999999999999, "totalSymbolTableParseTime": 0.001848, "totalSymbolTableStripped": 0, "totalSymbolTablesLoadedFromCache": 0, "totalSymbolTablesSavedToCache": 0 } (lldb) ``` ## `statistics dump --summary` of 2 Should be missing the `targets` section. ``` (lldb) statistics dump --summary { "memory": { "strings": { "bytesTotal": 716800, "bytesUnused": 705887, "bytesUsed": 10913 } }, "totalDebugInfoByteSize": 597, "totalDebugInfoEnabled": 1, "totalDebugInfoIndexLoadedFromCache": 0, "totalDebugInfoIndexSavedToCache": 0, "totalDebugInfoIndexTime": 0.001374, "totalDebugInfoParseTime": 0.000174, "totalModuleCount": 1, "totalModuleCountHasDebugInfo": 1, "totalModuleCountWithIncompleteTypes": 0, "totalModuleCountWithVariableErrors": 0, "totalSymbolTableIndexTime": 0.00068300000000000001, "totalSymbolTableParseTime": 0.0010139999999999999, "totalSymbolTableStripped": 0, "totalSymbolTablesLoadedFromCache": 0, "totalSymbolTablesSavedToCache": 0 } (lldb) ``` Co-authored-by: royshi <royshi@meta.com>
llvm · Jun 28, 2024 · f65a52a · f65a52a
1 parent e17b17d
commit f65a52a
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 11 deletions.
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
@@ -144,9 +144,8 @@ struct StatisticsOptions {
   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();
+    // Default to true in both default mode and summary mode.
+    return true;
   }
 
   void SetIncludeModules(bool value) { m_include_modules = value; }

diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
@@ -1429,17 +1429,16 @@ let Command = "statistics dump" in {
     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 "
+    "Defaults to true in both default mode and summary mode. "
+    "In default mode, if both '--targets' and '--modules' 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 "
+    "In default mode, if both '--targets' and '--modules' 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">,

diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -702,6 +702,8 @@ def get_test_cases_for_sections_existence(self):
                     "targets.moduleIdentifiers": True,
                     "targets.breakpoints": True,
                     "targets.expressionEvaluation": True,
+                    "targets.frameVariable": True,
+                    "targets.totalSharedLibraryEventHitCount": True,
                     "modules": True,
                     "transcript": True,
                 },
@@ -713,10 +715,12 @@ def get_test_cases_for_sections_existence(self):
                 },
                 "expect": {
                     "commands": False,
-                    "targets": False,
+                    "targets": True,
                     "targets.moduleIdentifiers": False,
                     "targets.breakpoints": False,
                     "targets.expressionEvaluation": False,
+                    "targets.frameVariable": False,
+                    "targets.totalSharedLibraryEventHitCount": True,
                     "modules": False,
                     "transcript": False,
                 },
@@ -733,11 +737,25 @@ def get_test_cases_for_sections_existence(self):
                     "targets.moduleIdentifiers": False,
                     "targets.breakpoints": False,
                     "targets.expressionEvaluation": False,
+                    "targets.frameVariable": False,
                     "targets.totalSharedLibraryEventHitCount": True,
                     "modules": False,
                     "transcript": False,
                 },
             },
+            {  # Summary mode without targets
+                "command_options": " --summary --targets=false",
+                "api_options": {
+                    "SetSummaryOnly": True,
+                    "SetIncludeTargets": False,
+                },
+                "expect": {
+                    "commands": False,
+                    "targets": False,
+                    "modules": False,
+                    "transcript": False,
+                },
+            },
             {  # Summary mode with modules
                 "command_options": " --summary --modules=true",
                 "api_options": {
@@ -746,15 +764,17 @@ def get_test_cases_for_sections_existence(self):
                 },
                 "expect": {
                     "commands": False,
-                    "targets": False,
+                    "targets": True,
                     "targets.moduleIdentifiers": False,
                     "targets.breakpoints": False,
                     "targets.expressionEvaluation": False,
+                    "targets.frameVariable": False,
+                    "targets.totalSharedLibraryEventHitCount": True,
                     "modules": True,
                     "transcript": False,
                 },
             },
-            {  # Everything mode but without modules and transcript
+            {  # Default mode without modules and transcript
                 "command_options": " --modules=false --transcript=false",
                 "api_options": {
                     "SetIncludeModules": False,
@@ -766,11 +786,13 @@ def get_test_cases_for_sections_existence(self):
                     "targets.moduleIdentifiers": False,
                     "targets.breakpoints": True,
                     "targets.expressionEvaluation": True,
+                    "targets.frameVariable": True,
+                    "targets.totalSharedLibraryEventHitCount": True,
                     "modules": False,
                     "transcript": False,
                 },
             },
-            {  # Everything mode but without modules
+            {  # Default mode without modules
                 "command_options": " --modules=false",
                 "api_options": {
                     "SetIncludeModules": False,
@@ -781,6 +803,8 @@ def get_test_cases_for_sections_existence(self):
                     "targets.moduleIdentifiers": False,
                     "targets.breakpoints": True,
                     "targets.expressionEvaluation": True,
+                    "targets.frameVariable": True,
+                    "targets.totalSharedLibraryEventHitCount": True,
                     "modules": False,
                     "transcript": True,
                 },