[lldb] Add symbol/table count into statistics

[royitaqi]

New stats

The following stats are added and are available in both "statistics dump" command and in python API.

1. In summary:
   1. Add totalSymbolsLoaded. The total number of symbols loaded in all modules.
   2. Add totalSymbolTablesLoaded . The total number symbol tables loaded in all modules.
2. In each module's stats:
   1. Add symbolsLoaded. The number of symbols loaded in the current module.

Example

Example statistics dump output:

(lldb) statistics dump
{
  ...,
  "modules": [
    {
      "path": "/Users/<username>/demo/simple/a.out",
      "symbolsLoaded": 6,
      ...
    },
    ...
  ],
  ...
  "totalSymbolTablesLoaded": 42,
  "totalSymbolsLoaded": 32198
}

Tests

Manual test: Built and ran lldb on a helloworld program. Ran statistics dump. Verified the above stats.

Unit test: Ran the following tests:

$ bin/lldb-dotest -p TestStats.py ~/llvm-sand/external/llvm-project/lldb/test/API/commands/statistics/basic/
...
Ran 18 tests in 192.676s

OK (skipped=3)

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (royitaqi)

Changes

These stats are added or changed:

1. In summary:
   1. Add totalSymbolsLoaded. The total number of symbols loaded in all modules.
   2. Add totalSymbolTablesLoaded . The total number symbol tables loaded in all modules.
2. In each module's stats:
   1. Add symbolsLoaded. The number of symbols loaded in the current module.

--

Still running tests. Will update here.

---

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

3 Files Affected:

• (modified) lldb/include/lldb/Target/Statistics.h (+1)
• (modified) lldb/source/Target/Statistics.cpp (+13-5)
• (modified) lldb/test/API/commands/statistics/basic/TestStats.py (+18-5)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index ee365357fcf31..b87a12a8ab9cd 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -120,6 +120,7 @@ struct ModuleStats {
   llvm::StringMap<llvm::json::Value> type_system_stats;
   double symtab_parse_time = 0.0;
   double symtab_index_time = 0.0;
+  uint32_t num_symbols_loaded = 0;
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint64_t debug_info_size = 0;
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index b5d2e7bda1edf..f70fb529544a5 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -71,6 +71,7 @@ json::Value ModuleStats::ToJSON() const {
   module.try_emplace("debugInfoHadIncompleteTypes",
                      debug_info_had_incomplete_types);
   module.try_emplace("symbolTableStripped", symtab_stripped);
+  module.try_emplace("symbolsLoaded", num_symbols_loaded);
   if (!symfile_path.empty())
     module.try_emplace("symbolFilePath", symfile_path);
 
@@ -293,7 +294,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint32_t symtabs_loaded = 0;
-  uint32_t symtabs_saved = 0;
+  uint32_t symtabs_loaded_from_cache = 0;
+  uint32_t symtabs_saved_to_cache = 0;
   uint32_t debug_index_loaded = 0;
   uint32_t debug_index_saved = 0;
   uint64_t debug_info_size = 0;
@@ -309,6 +311,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   uint32_t num_modules_with_variable_errors = 0;
   uint32_t num_modules_with_incomplete_types = 0;
   uint32_t num_stripped_modules = 0;
+  uint32_t num_symbols_loaded = 0;
   for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
     Module *module = target != nullptr
                          ? target->GetImages().GetModuleAtIndex(image_idx).get()
@@ -318,12 +321,15 @@ llvm::json::Value DebuggerStats::ReportStatistics(
     module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
     Symtab *symtab = module->GetSymtab(/*can_create=*/false);
     if (symtab) {
+      module_stat.num_symbols_loaded = symtab->GetNumSymbols();
+      num_symbols_loaded += module_stat.num_symbols_loaded;
+      symtabs_loaded++;
       module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache();
       if (module_stat.symtab_loaded_from_cache)
-        ++symtabs_loaded;
+        ++symtabs_loaded_from_cache;
       module_stat.symtab_saved_to_cache = symtab->GetWasSavedToCache();
       if (module_stat.symtab_saved_to_cache)
-        ++symtabs_saved;
+        ++symtabs_saved_to_cache;
     }
     SymbolFile *sym_file = module->GetSymbolFile(/*can_create=*/false);
     if (sym_file) {
@@ -393,8 +399,9 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   json::Object global_stats{
       {"totalSymbolTableParseTime", symtab_parse_time},
       {"totalSymbolTableIndexTime", symtab_index_time},
-      {"totalSymbolTablesLoadedFromCache", symtabs_loaded},
-      {"totalSymbolTablesSavedToCache", symtabs_saved},
+      {"totalSymbolTablesLoaded", symtabs_loaded},
+      {"totalSymbolTablesLoadedFromCache", symtabs_loaded_from_cache},
+      {"totalSymbolTablesSavedToCache", symtabs_saved_to_cache},
       {"totalDebugInfoParseTime", debug_parse_time},
       {"totalDebugInfoIndexTime", debug_index_time},
       {"totalDebugInfoIndexLoadedFromCache", debug_index_loaded},
@@ -407,6 +414,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
        num_modules_with_incomplete_types},
       {"totalDebugInfoEnabled", num_debug_info_enabled_modules},
       {"totalSymbolTableStripped", num_stripped_modules},
+      {"totalSymbolsLoaded", num_symbols_loaded},
   };
 
   if (include_targets) {
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 54881c13bcb68..5dcad88af7b84 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -159,6 +159,8 @@ def test_default_no_run(self):
         """
         self.build()
         target = self.createTestTarget()
+
+        # Verify top-level keys.
         debug_stats = self.get_stats()
         debug_stat_keys = [
             "memory",
@@ -168,6 +170,7 @@ def test_default_no_run(self):
             "totalSymbolTableIndexTime",
             "totalSymbolTablesLoadedFromCache",
             "totalSymbolTablesSavedToCache",
+            "totalSymbolsLoaded",
             "totalDebugInfoByteSize",
             "totalDebugInfoIndexTime",
             "totalDebugInfoIndexLoadedFromCache",
@@ -175,16 +178,26 @@ def test_default_no_run(self):
             "totalDebugInfoParseTime",
         ]
         self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
-        stats = debug_stats["targets"][0]
-        keys_exist = [
+        
+        # Verify target stats keys.
+        target_stats = debug_stats["targets"][0]
+        target_stat_keys_exist = [
             "expressionEvaluation",
             "frameVariable",
             "moduleIdentifiers",
             "targetCreateTime",
         ]
-        keys_missing = ["firstStopTime", "launchOrAttachTime"]
-        self.verify_keys(stats, '"stats"', keys_exist, keys_missing)
-        self.assertGreater(stats["targetCreateTime"], 0.0)
+        target_stat_keys_missing = ["firstStopTime", "launchOrAttachTime"]
+        self.verify_keys(target_stats, '"target_stats"', target_stat_keys, target_stat_keys_missing)
+        self.assertGreater(target_stats["targetCreateTime"], 0.0)
+
+        # Verify module stats keys.
+        for module_stats in debug_stats["modules"]:
+            module_stat_keys_exist = [
+                "symbolsLoaded",
+            ]
+            self.verify_keys(module_stats, '"module_stats"', module_stat_keys_exist, None)
+
 
     def test_default_with_run(self):
         """Test "statistics dump" when running the target to a breakpoint.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[dmpots]

LGTM

[royitaqi]

Will wait until tomorrow morning/noon before merging, so that:

1. Other folks will have a chance to review.
2. Avoid failing tests after work hour so that folks can sleep.

[royitaqi]

Merging this PR, because it will make it easier for me to add a test case in #136236

[omjavaid]

this PR caused lldb windows buildbot to fail

https://lab.llvm.org/buildbot/#/builders/141/builds/8084

[omjavaid]

the reported number of symbols is zero on Windows

"symbolsLoaded": 0,
...
},
...
],
...
"totalSymbolTablesLoaded": 0,
"totalSymbolsLoaded": 0
}

[DavidSpickett]

Might be another case of us building with clang, producing DWARF, then linking with link.exe which discards the DWARF. I've skipped a few tests for this already.

(though I wonder if we should just be using lld anyway)

[omjavaid]

I have tested the using clang and used lld-link as the default linker the symbols are still zero. For now I have added a bypass commit for this to make buildbot happy.

[royitaqi]

@dmpots: See the comments above and the revert of #136236. Both are about the symbol count being inconsistent under different configurations (os, linker, debug info plug-in etc; so far it's not clear what is causing the difference).

I propose that for both PR we should remove the verification about the symbol count. Reasons:

1. This PR([lldb] Add symbol/table count into statistics #136226)'s goal is to add symbol count to stats dump. The goal isn't to assure that the count is zero or not.
2. In [lldb] Avoid force loading symbols in statistics collection #136236, we should drop the test case in TestStats.py, because there is no guarantee that a different configuration won't cause the symbols to be loaded during target creation.

WDYT?

[clayborg]

Can we renamed "symbolsLoaded" to "symbolTableSymbolCount" and "totalSymbolsLoaded" to "totalSymbolTableSymbolCount"? This would be more consistent with the other symbol table statistics