Skip to content

[lldb] Include memory stats in statistics summary #94671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

bulbazord
Copy link
Member

The summary already includes other size information, e.g. total debug info size in bytes. The only other way I can get this information is by dumping all statistics which can be quite large. Adding it to the summary seems fair.

The summary already includes other size information, e.g. total debug
info size in bytes. The only other way I can get this information is by
dumping all statistics which can be quite large. Adding it to the
summary seems fair.
@bulbazord bulbazord requested review from clayborg and kusmour June 6, 2024 19:39
@bulbazord bulbazord requested a review from JDevlieghere as a code owner June 6, 2024 19:39
@llvmbot llvmbot added the lldb label Jun 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 6, 2024

@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)

Changes

The summary already includes other size information, e.g. total debug info size in bytes. The only other way I can get this information is by dumping all statistics which can be quite large. Adding it to the summary seems fair.


Full diff: https://github.com/llvm/llvm-project/pull/94671.diff

1 Files Affected:

  • (modified) lldb/source/Target/Statistics.cpp (+5-5)
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index be0848573f812..2a5300012511a 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -355,14 +355,14 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   }
   global_stats.try_emplace("targets", std::move(json_targets));
 
+  ConstStringStats const_string_stats;
+  json::Object json_memory{
+      {"strings", const_string_stats.ToJSON()},
+  };
+  global_stats.try_emplace("memory", std::move(json_memory));
   if (!summary_only) {
-    ConstStringStats const_string_stats;
-    json::Object json_memory{
-        {"strings", const_string_stats.ToJSON()},
-    };
     json::Value cmd_stats = debugger.GetCommandInterpreter().GetStatistics();
     global_stats.try_emplace("modules", std::move(json_modules));
-    global_stats.try_emplace("memory", std::move(json_memory));
     global_stats.try_emplace("commands", std::move(cmd_stats));
   }
 

Copy link
Contributor

@kusmour kusmour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The memory section is reasonably small to be in the summary

  "memory": {
    "strings": {
      "bytesTotal": 3145728,
      "bytesUnused": 1426827,
      "bytesUsed": 1718901
    }
  },

LGTM!

@bulbazord bulbazord merged commit 9293fc7 into llvm:main Jun 6, 2024
7 checks passed
@bulbazord bulbazord deleted the memory-stats branch June 6, 2024 20:18
@bulbazord
Copy link
Member Author

Ah, I see there was a test for this that I missed. I've gotten an email about it, taking a look now.

Failed Tests (1):
  lldb-api :: functionalities/stats_api/TestStatisticsAPI.py

@bulbazord
Copy link
Member Author

Fix: #94683

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants