[lldb] Add count for number of DWO files loaded in statistics #144424
+242
−19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-lldb Author: None (qxy11) ChangesSummaryA new
TestingManual Testing Unit test Full diff: https://github.com/llvm/llvm-project/pull/144424.diff 10 Files Affected:
diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index 75c7f230ddf3d..42fbc2508889a 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -425,6 +425,11 @@ class SymbolFile : public PluginInterface {
/// Reset the statistics for the symbol file.
virtual void ResetStatistics() {}
+ /// Get the number of loaded DWO files for this symbol file.
+ /// This is used for statistics gathering and will return 0 for most
+ /// symbol file implementations except DWARF symbol files.
+ virtual uint32_t GetLoadedDwoFileCount() const { return 0; }
+
/// Get the additional modules that this symbol file uses to parse debug info.
///
/// Some debug info is stored in stand alone object files that are represented
diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index de05732469448..572abf590345c 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -247,13 +247,24 @@ def getLLDBObjRoot(self):
def _getDebugInfoArgs(self, debug_info):
if debug_info is None:
return []
- if debug_info == "dwarf":
- return ["MAKE_DSYM=NO"]
- if debug_info == "dwo":
- return ["MAKE_DSYM=NO", "MAKE_DWO=YES"]
- if debug_info == "gmodules":
- return ["MAKE_DSYM=NO", "MAKE_GMODULES=YES"]
- return None
+
+ debug_options = debug_info if isinstance(debug_info, list) else [debug_info]
+ option_flags = {
+ "dwarf": {"MAKE_DSYM": "NO"},
+ "dwo": {"MAKE_DSYM": "NO", "MAKE_DWO": "YES"},
+ "gmodules": {"MAKE_DSYM": "NO", "MAKE_GMODULES": "YES"},
+ "debug_names": {"MAKE_DEBUG_NAMES": "YES"},
+ }
+
+ # Collect all flags, with later options overriding earlier ones
+ flags = {}
+
+ for option in debug_options:
+ if not option or option not in option_flags:
+ return None # Invalid options
+ flags.update(option_flags[option])
+
+ return [f"{key}={value}" for key, value in flags.items()]
def getBuildCommand(
self,
diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 06959f226066a..58833e1b0cc78 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -276,6 +276,10 @@ ifeq "$(MAKE_DWO)" "YES"
CFLAGS += -gsplit-dwarf
endif
+ifeq "$(MAKE_DEBUG_NAMES)" "YES"
+ CFLAGS += -gpubnames
+endif
+
ifeq "$(USE_PRIVATE_MODULE_CACHE)" "YES"
THE_CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/private-module-cache
else
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 71f204c03a42a..acd9d2230c201 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4358,6 +4358,7 @@ StatsDuration::Duration SymbolFileDWARF::GetDebugInfoIndexTime() {
void SymbolFileDWARF::ResetStatistics() {
m_parse_time.reset();
+ m_loaded_dwo_file_count = 0;
if (m_index)
return m_index->ResetStatistics();
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index d2d30d7decb16..d695961bbfc1d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -315,6 +315,11 @@ class SymbolFileDWARF : public SymbolFileCommon {
void ResetStatistics() override;
+ /// Get the number of loaded DWO files for this symbol file
+ uint32_t GetLoadedDwoFileCount() const override {
+ return m_loaded_dwo_file_count;
+ }
+
virtual lldb::offset_t
GetVendorDWARFOpcodeSize(const DataExtractor &data,
const lldb::offset_t data_offset,
@@ -497,6 +502,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
void InitializeFirstCodeAddress();
+ void IncrementLoadedDwoFileCount() { m_loaded_dwo_file_count++; }
+
void
GetCompileOptions(std::unordered_map<lldb::CompUnitSP, Args> &args) override;
@@ -550,6 +557,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
/// valid value that can be used in DIERef objects which will contain
/// an index that identifies the .DWO or .o file.
std::optional<uint64_t> m_file_index;
+ /// Count of loaded DWO files for this symbol file
+ uint32_t m_loaded_dwo_file_count = 0;
};
} // namespace dwarf
} // namespace lldb_private::plugin
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index c1829abe72933..1f0e847da2905 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -31,7 +31,7 @@ SymbolFileDWARFDwo::SymbolFileDWARFDwo(SymbolFileDWARF &base_symbol_file,
/*update_module_section_list*/ false)),
m_base_symbol_file(base_symbol_file) {
SetFileIndex(id);
-
+ m_base_symbol_file.IncrementLoadedDwoFileCount();
// Parsing of the dwarf unit index is not thread-safe, so we need to prime it
// to enable subsequent concurrent lookups.
m_context.GetAsLLVM().getCUIndex();
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 6ec8f8963baf9..b30d7f755f2fc 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -322,6 +322,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
uint32_t num_modules_with_incomplete_types = 0;
uint32_t num_stripped_modules = 0;
uint32_t symtab_symbol_count = 0;
+ uint32_t total_loaded_dwo_file_count = 0;
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
Module *module = target != nullptr
? target->GetImages().GetModuleAtIndex(image_idx).get()
@@ -353,6 +354,9 @@ llvm::json::Value DebuggerStats::ReportStatistics(
for (const auto &symbol_module : symbol_modules.Modules())
module_stat.symfile_modules.push_back((intptr_t)symbol_module.get());
}
+ // Add DWO file count from this symbol file (will be 0 for non-DWARF
+ // symbol files)
+ total_loaded_dwo_file_count += sym_file->GetLoadedDwoFileCount();
module_stat.debug_info_index_loaded_from_cache =
sym_file->GetDebugInfoIndexWasLoadedFromCache();
if (module_stat.debug_info_index_loaded_from_cache)
@@ -427,6 +431,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
{"totalDebugInfoEnabled", num_debug_info_enabled_modules},
{"totalSymbolTableStripped", num_stripped_modules},
{"totalSymbolTableSymbolCount", symtab_symbol_count},
+ {"totalLoadedDwoFileCount", total_loaded_dwo_file_count},
};
if (include_targets) {
diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py
index 83132b40d85db..de4dafaf0a77d 100644
--- a/lldb/test/API/commands/statistics/basic/TestStats.py
+++ b/lldb/test/API/commands/statistics/basic/TestStats.py
@@ -177,6 +177,7 @@ def test_default_no_run(self):
"totalDebugInfoIndexLoadedFromCache",
"totalDebugInfoIndexSavedToCache",
"totalDebugInfoParseTime",
+ "totalLoadedDwoFileCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
if self.getPlatform() != "windows":
@@ -287,6 +288,7 @@ def test_default_with_run(self):
"totalDebugInfoIndexLoadedFromCache",
"totalDebugInfoIndexSavedToCache",
"totalDebugInfoParseTime",
+ "totalLoadedDwoFileCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
stats = debug_stats["targets"][0]
@@ -325,6 +327,7 @@ def test_memory(self):
"totalDebugInfoIndexLoadedFromCache",
"totalDebugInfoIndexSavedToCache",
"totalDebugInfoByteSize",
+ "totalLoadedDwoFileCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
@@ -377,6 +380,7 @@ def test_modules(self):
"totalDebugInfoIndexLoadedFromCache",
"totalDebugInfoIndexSavedToCache",
"totalDebugInfoByteSize",
+ "totalLoadedDwoFileCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
stats = debug_stats["targets"][0]
@@ -485,6 +489,7 @@ def test_breakpoints(self):
"totalDebugInfoIndexLoadedFromCache",
"totalDebugInfoIndexSavedToCache",
"totalDebugInfoByteSize",
+ "totalLoadedDwoFileCount",
]
self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
target_stats = debug_stats["targets"][0]
@@ -512,6 +517,32 @@ def test_breakpoints(self):
self.verify_keys(
breakpoint, 'target_stats["breakpoints"]', bp_keys_exist, None
)
+
+ def test_loaded_dwo_file_count(self):
+ """
+ Test "statistics dump" and the loaded dwo file count.
+ Builds a binary w/ separate .dwo files and debug_names, and then
+ verifies the loaded dwo file count is the expected count after running
+ various commands
+ """
+ da = {"CXX_SOURCES": "third.cpp baz.cpp", "EXE": self.getBuildArtifact("a.out")}
+ self.build(dictionary=da, debug_info=["dwo", "debug_names"])
+ self.addTearDownCleanup(dictionary=da)
+ exe = self.getBuildArtifact("a.out")
+ target = self.createTestTarget(file_path=exe)
+ debug_stats = self.get_stats()
+
+ self.assertIn("totalLoadedDwoFileCount", debug_stats)
+ self.assertEqual(debug_stats["totalLoadedDwoFileCount"], 0)
+
+ self.runCmd("b main")
+ debug_stats_after_main = self.get_stats()
+ self.assertEqual(debug_stats_after_main["totalLoadedDwoFileCount"], 1)
+
+ self.runCmd("type lookup Baz")
+ debug_stats_after_type_lookup = self.get_stats()
+ self.assertEqual(debug_stats_after_type_lookup["totalLoadedDwoFileCount"], 2)
+
@skipUnlessDarwin
@no_debug_info_test
diff --git a/lldb/test/API/commands/statistics/basic/baz.cpp b/lldb/test/API/commands/statistics/basic/baz.cpp
new file mode 100644
index 0000000000000..536758b17d839
--- /dev/null
+++ b/lldb/test/API/commands/statistics/basic/baz.cpp
@@ -0,0 +1,12 @@
+// Helper that the lldb command `statistics dump` works in split-dwarf mode.
+
+struct Baz {
+ int x;
+ bool y;
+};
+
+void baz() {
+ Baz b;
+ b.x = 1;
+ b.y = true;
+}
diff --git a/lldb/test/API/commands/statistics/basic/third.cpp b/lldb/test/API/commands/statistics/basic/third.cpp
new file mode 100644
index 0000000000000..3943b9c2faafe
--- /dev/null
+++ b/lldb/test/API/commands/statistics/basic/third.cpp
@@ -0,0 +1,7 @@
+// Test that the lldb command `statistics dump` works.
+
+void baz();
+int main(void) {
+ baz();
+ return 0;
+}
|
dmpots
reviewed
Jun 16, 2025
| /// Get the number of loaded DWO files for this symbol file. | ||
| /// This is used for statistics gathering and will return 0 for most | ||
| /// symbol file implementations except DWARF symbol files. | ||
| virtual uint32_t GetLoadedDwoFileCount() const { return 0; } |
There was a problem hiding this comment.
There is a GetSeparateDebugInfo member function that returns info about separate debug info (which dwo is one type). Looking here it looks like we could use it to find all the dwos that were loaded.
I wonder if it would be better to use this existing function when generating the stats instead of adding a new overload here.
There was a problem hiding this comment.
Thanks for the reference! In GetSeparateDebugInfo here, the call to GetDwoSymbolFile ends up loading all DWO files (even if they weren't already loaded). I think it'd work if we set the load_all_debug_info to false in the call and do GetDwoSymbolFile(false), in which case we'd have to add an extra load_all_debug_info argument to GetSeparateDebugInfo as well so that other existing use cases for GetSeparateDebugInfo don't get effected. I can try that instead of adding the new overload.
|
Besides Also, we should test this against dwp file situation, and non-split-dwarf situation, what are the numbers we should report? |
Address comments to use `GetSeparateDebugInfo` instead of adding a new Symbol File function. Also add a totalLoadedDwoFileCount to statistics dump and add some unit tests for expected behavior in the DWP case and non-split-dwarf cases.
dmpots
reviewed
Jun 17, 2025
lldb/source/Target/Statistics.cpp
Outdated
| @@ -353,6 +355,30 @@ llvm::json::Value DebuggerStats::ReportStatistics( | |||
| for (const auto &symbol_module : symbol_modules.Modules()) | |||
| module_stat.symfile_modules.push_back((intptr_t)symbol_module.get()); | |||
| } | |||
| // Count DWO files from this symbol file using GetSeparateDebugInfo | |||
There was a problem hiding this comment.
Could we put this logic into a separate function like and call it here?
Something like
updateDwoCounts(sym_file, total_dwo_file_count, totoal_loaded_dwo_file_count)
…es_eager_loads_dwo_files Adds an extra unit test and refactors some logic into a helper function. Also added additional comments in code and tests to clarify purpose of debug_names usage and how it prevents the eager loading of DWO files
dmpots
approved these changes
Jun 17, 2025
There was a problem hiding this comment.
LGTM! You should get approvals from @jeffreytan81 and @clayborg as well before merging.
| self.assertEqual(debug_stats["totalDwoFileCount"], 0) | ||
| self.assertEqual(debug_stats["totalLoadedDwoFileCount"], 0) | ||
|
|
||
| def test_debug_names_eager_loads_dwo_files(self): |
There was a problem hiding this comment.
nit: The name test_debug_names_eager_loads_dwo_files makes it sound like we are checking that debug names causes eager loading, but we are actually checking the opposite.
clayborg
requested changes
Jun 17, 2025
lldb/source/Target/Statistics.cpp
Outdated
| StructuredData::Dictionary separate_debug_info; | ||
| if (sym_file->GetSeparateDebugInfo(separate_debug_info, | ||
| /*errors_only=*/false, | ||
| /*load_all_debug_info*/ false)) { |
There was a problem hiding this comment.
/*load_all_debug_info=*/false
lldb/source/Target/Statistics.cpp
Outdated
Comment on lines
389
to
390
| UpdateDwoFileCounts(sym_file, total_dwo_file_count, | ||
| total_loaded_dwo_file_count); |
There was a problem hiding this comment.
We should probably break this down and add statistics for each module using keys named "loadedDwoFileCount" and "dwoFileCount" in each module's statistics. And then we still add totals at the end as you have. So maybe fetch into module_stat first and then update the totals:
UpdateDwoFileCounts(sym_file, module_stat.total_dwo_file_count,
module_stat.loaded_dwo_file_count);
total_dwo_file_count += module_stat.total_dwo_file_count;
total_loaded_dwo_file_count += module_stat.loaded_dwo_file_count;
We will need to add two things to ModuleStats:
uint32_t dwo_file_count = 0;
uint32_t loaded_dwo_file_count = 0;
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
A new
totalLoadedDwoFileCountandtotalDwoFileCountcounters to available statisctics when calling "statistics dump".GetSeparateDebugInfois modified to also take in aload_all_debug_infoargument. This arg is set default to True, as was the previous default setting, and is only functional forSymbolFileDwarf, where previously all calls to get debug info were default toGetDwoSymbolFile(true), and we can now pass in whether to force load debug info if it wasn't already loaded.Statistics, useGetSeparateDebugInfoto sum up the total number of loaded/parsed DWO files along with the total number of DWO files. This is done by checking whether the DWO file was already successfullyloadedin the collected DWO data, anding adding to thetotalLoadedDwoFileCount, and adding tototalDwoFileCountfor all CU units.Expected Behavior
totalLoadedDwoFileCountwould be the number of loaded DWO files andtotalDwoFileCountwould be the total count of DWO files.totalLoadedDwoFileCountwould be the number of parsed compile units, whiletotalDwoFileCountwould be the total number of CUs in the DWP file. This should be similar to the counts we get from loading separate DWO files rather than only counting whether a single DWP file was loaded.totalDwoFileCountandtotalLoadedDwoFileCountto be 0 since no separate debug info is loaded.Testing
Manual Testing
On an internal script that has many DWO files,
statistics dumpwas called before and after atype lookupcommand. ThetotalLoadedDwoFileCountincreased as expected after thetype lookup.Unit test
Added three unit tests that build with new "third.cpp" and "baz.cpp" files. For tests with w/ flags
-gsplit-dwarf -gpubnames, this generates 2 DWO files. Then, the test incrementally adds breakpoints, and does a type lookup, and the count should increase for each of these as new DWO files get loaded to support these.