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 <roy...@meta.com> Date: Mon, 10 Jun 2024 17:04:11 -0700 Subject: [PATCH 1/5] 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 <roy...@meta.com> Date: Mon, 10 Jun 2024 17:04:11 -0700 Subject: [PATCH 2/5] 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 <roy...@meta.com> Date: Mon, 10 Jun 2024 21:54:40 -0700 Subject: [PATCH 3/5] 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 <roy...@meta.com> Date: Mon, 10 Jun 2024 21:55:44 -0700 Subject: [PATCH 4/5] 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}'", + ) >From a8fc16d5f31fd45258b5b7f950cd080a9d83680a Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Mon, 10 Jun 2024 22:10:00 -0700 Subject: [PATCH 5/5] Add options into SBStatisticsOptions --- lldb/include/lldb/API/SBStatisticsOptions.h | 9 ++++++++ lldb/source/API/SBStatisticsOptions.cpp | 24 +++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/lldb/include/lldb/API/SBStatisticsOptions.h b/lldb/include/lldb/API/SBStatisticsOptions.h index a0055135e36c2..35ebe3db1c53b 100644 --- a/lldb/include/lldb/API/SBStatisticsOptions.h +++ b/lldb/include/lldb/API/SBStatisticsOptions.h @@ -25,6 +25,15 @@ class LLDB_API SBStatisticsOptions { void SetSummaryOnly(bool b); bool GetSummaryOnly(); + void SetIncludeTargets(bool b); + bool GetIncludeTargets() const; + + void SetIncludeModules(bool b); + bool GetIncludeModules() const; + + 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/source/API/SBStatisticsOptions.cpp b/lldb/source/API/SBStatisticsOptions.cpp index 7e826c4c93ebc..7cf8664955ae7 100644 --- a/lldb/source/API/SBStatisticsOptions.cpp +++ b/lldb/source/API/SBStatisticsOptions.cpp @@ -44,6 +44,30 @@ void SBStatisticsOptions::SetSummaryOnly(bool b) { bool SBStatisticsOptions::GetSummaryOnly() { return m_opaque_up->summary_only; } +void SBStatisticsOptions::SetIncludeTargets(bool b) { + m_opaque_up->include_targets = b; +} + +bool SBStatisticsOptions::GetIncludeTargets() const { + return m_opaque_up->include_targets; +} + +void SBStatisticsOptions::SetIncludeModules(bool b) { + m_opaque_up->include_modules = b; +} + +bool SBStatisticsOptions::GetIncludeModules() const { + return m_opaque_up->include_modules; +} + +void SBStatisticsOptions::SetIncludeTranscript(bool b) { + m_opaque_up->include_transcript = b; +} + +bool SBStatisticsOptions::GetIncludeTranscript() const { + return m_opaque_up->include_transcript; +} + void SBStatisticsOptions::SetReportAllAvailableDebugInfo(bool b) { m_opaque_up->load_all_debug_info = b; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits