From 2c3d37c44b70a6e45a43045fcf07e8e739573c49 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 8 Oct 2025 15:01:34 -0400 Subject: [PATCH] gdb: fix slowdown during skeletonless type units processing My commit 6474c699a525 ("gdb/dwarf: sort dwarf2_per_bfd::all_units by (section, offset)") introduced a pretty bad performance regression in the "skeletonless type units" step. I have a pretty big executable (Blender) compiled with -gsplit-dwarf (to generate .dwo files) and -fdebug-types-section (to generate type units). Before the offending commit: Time for "DWARF skeletonless type units": wall 29.126, user 28.507, sys 0.497, user+sys 29.004, 99.6 % CPU ... and after: Time for "DWARF skeletonless type units": wall 120.768, user 119.543, sys 0.651, user+sys 120.194, 99.5 % CPU The reason for the slowdown is that add_type_unit now inserts type units at the right place in the all_units vector to keep it sorted. These repeated insertions in the middle of the vector require shifting a lot of elements and end up taking a lot of time. This patch fixes it by doing just one sort at the end of process_skeletonless_type_units. The responsibility of keeping the all_units sorted is delegated to the callers of add_type_unit. The other two callers call finalize_all_units right after calling add_type_unit. One drawback that is probably not a real one: in process_skeletonless_type_unit, we call process_type_unit. If something in there needs to look up another type unit by (section, offset), it wouldn't find it. I don't think that's a real issue though, as type units are typically self contained. If a type unit needs to refer to a type defined in another type unit, it would do so by signature, with DW_FORM_ref_sig8. And during the indexing phase, I don't think we even look at the DW_AT_type of things anyway. With this patch applied, I am back to: Time for "DWARF skeletonless type units": wall 29.277, user 28.632, sys 0.521, user+sys 29.153, 99.6 % CPU I would like to cherry pick this patch to GDB 17, to avoid shipping GDB 17 with the performance regression. Change-Id: I2a5b89ebca9e1a4e6248032e144520c9a579f47a Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33526 Approved-By: Tom Tromey Reviewed-By: Andrew Burgess --- gdb/dwarf2/read.c | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 955893c5f0c..b35d33b5762 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -2488,7 +2488,10 @@ read_abbrev_offset (dwarf2_per_objfile *per_objfile, return (sect_offset) read_offset (abfd, info_ptr, offset_size); } -/* Add an entry for signature SIG to per_bfd->signatured_types. */ +/* Add an entry for signature SIG to per_bfd->signatured_types. + + This functions leaves PER_BFD::ALL_UNITS unsorted. The caller must call + finalize_all_units after adding one or more type units. */ static signatured_type_set::iterator add_type_unit (dwarf2_per_bfd *per_bfd, dwarf2_section_info *section, @@ -2502,17 +2505,7 @@ add_type_unit (dwarf2_per_bfd *per_bfd, dwarf2_section_info *section, false /* is_dwz */, sig); signatured_type *sig_type = sig_type_holder.get (); - /* Preserve the ordering of per_bfd->all_units. */ - auto insert_it - = std::lower_bound (per_bfd->all_units.begin (), per_bfd->all_units.end (), - sig_type, - [] (const dwarf2_per_cu_up &lhs, - const signatured_type *rhs) { - return all_units_less_than (*lhs, { rhs->section, - rhs->sect_off }); - }); - - per_bfd->all_units.emplace (insert_it, sig_type_holder.release ()); + per_bfd->all_units.emplace_back (sig_type_holder.release ()); auto emplace_ret = per_bfd->signatured_types.emplace (sig_type); /* Assert that an insertion took place - that there wasn't a type unit with @@ -2600,8 +2593,11 @@ lookup_dwo_signatured_type (struct dwarf2_cu *cu, ULONGEST sig) /* If the global table doesn't have an entry for this TU, add one. */ if (sig_type_it == per_bfd->signatured_types.end ()) - sig_type_it = add_type_unit (per_bfd, dwo_entry->section, - dwo_entry->sect_off, dwo_entry->length, sig); + { + sig_type_it = add_type_unit (per_bfd, dwo_entry->section, + dwo_entry->sect_off, dwo_entry->length, sig); + finalize_all_units (per_bfd); + } if ((*sig_type_it)->dwo_unit == nullptr) fill_in_sig_entry_from_dwo_entry (per_objfile, *sig_type_it, dwo_entry); @@ -2643,6 +2639,7 @@ lookup_dwp_signatured_type (struct dwarf2_cu *cu, ULONGEST sig) sig_type_it = add_type_unit (per_bfd, dwo_entry->section, dwo_entry->sect_off, dwo_entry->length, sig); + finalize_all_units (per_bfd); fill_in_sig_entry_from_dwo_entry (per_objfile, *sig_type_it, dwo_entry); return *sig_type_it; @@ -3598,6 +3595,8 @@ cooked_index_worker_debug_info::process_skeletonless_type_unit this TU. */ sig_type_it = add_type_unit (per_bfd, dwo_unit->section, dwo_unit->sect_off, dwo_unit->length, dwo_unit->signature); + /* finalize_all_units is called just once by process_skeletonless_type_units + after going through all skeletonless type units. */ fill_in_sig_entry_from_dwo_entry (per_objfile, *sig_type_it, dwo_unit); /* This does the job that build_type_psymtabs would have done. */ @@ -3612,15 +3611,20 @@ cooked_index_worker_debug_info::process_skeletonless_type_units (dwarf2_per_objfile *per_objfile, cooked_index_worker_result *storage) { scoped_time_it time_it ("DWARF skeletonless type units", m_per_command_time); + dwarf2_per_bfd *per_bfd = per_objfile->per_bfd; /* Skeletonless TUs in DWP files without .gdb_index is not supported yet. */ - if (per_objfile->per_bfd->dwp_file == nullptr) - for (const dwo_file_up &file : per_objfile->per_bfd->dwo_files) - for (const dwo_unit_up &unit : file->tus) - storage->catch_error ([&] () - { - process_skeletonless_type_unit (unit.get (), per_objfile, storage); - }); + if (per_bfd->dwp_file == nullptr) + { + for (const dwo_file_up &file : per_bfd->dwo_files) + for (const dwo_unit_up &unit : file->tus) + storage->catch_error ([&] () + { + process_skeletonless_type_unit (unit.get (), per_objfile, storage); + }); + + finalize_all_units (per_bfd); + } } void -- 2.47.3