]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb: fix slowdown during skeletonless type units processing
authorSimon Marchi <simon.marchi@polymtl.ca>
Wed, 8 Oct 2025 19:01:34 +0000 (15:01 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Mon, 20 Oct 2025 18:19:25 +0000 (14:19 -0400)
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 <tom@tromey.com>
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
(cherry picked from commit 2c3d37c44b70a6e45a43045fcf07e8e739573c49)

gdb/dwarf2/read.c

index 57b10aa6b5fb3f67bcf5954f220e78a9a674b4ee..97b4f64e5b6963b438ba536fee7783d0921a7d0f 100644 (file)
@@ -2418,7 +2418,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,
@@ -2432,17 +2435,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
@@ -2530,8 +2523,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);
@@ -2573,6 +2569,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;
@@ -3481,6 +3478,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.  */
@@ -3495,15 +3494,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