[PATCH] Create a progress meter when reading symbols

[PATCH] Create a progress meter when reading symbols

[Tom Tromey]

This patch changes symbol reading to create a progress meter, and
changes the DWARF reader to update it.  This makes gdb give some
feedback when reading symbols.

Regression tested on x86-64 Fedora 32.

gdb/ChangeLog
2021-01-10  Tom Tromey  <tom@tromey.com>

	* symfile.c (symbol_file_add_with_addrs): Create a progress
	meter.
	(symbol_file_add_with_addrs): Create a progress meter.
	(reread_symbols): Create a progress meter.
	* psymtab.c (require_partial_symbols): Create a progress meter.
	* dwarf2/read.c (create_cus_from_index_list): Add
	total_progress_steps parameter.
	(create_cus_from_index): Likewise.
	(create_signatured_type_table_from_index): Add base_steps and
	total_progress_steps parameters.
	(dwarf2_read_gdb_index): Update.
	(dw2_expand_all_symtabs): Notify progress.
	(dwarf2_build_psymtabs_hard): Likewise.
---
 gdb/ChangeLog     | 16 ++++++++++++++
 gdb/dwarf2/read.c | 31 ++++++++++++++++++++-------
 gdb/psymtab.c     | 15 ++++++++++++--
 gdb/symfile.c     | 53 +++++++++++++++++++++++++++++++++--------------
 4 files changed, 90 insertions(+), 25 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 9032186ef89..46fe71dc479 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2653,7 +2653,8 @@ static void
 create_cus_from_index_list (dwarf2_per_bfd *per_bfd,
 			    const gdb_byte *cu_list, offset_type n_elements,
 			    struct dwarf2_section_info *section,
-			    int is_dwz)
+			    int is_dwz,
+			    double total_progress_steps)
 {
   for (offset_type i = 0; i < n_elements; i += 2)
     {
@@ -2668,6 +2669,8 @@ create_cus_from_index_list (dwarf2_per_bfd *per_bfd,
 	= create_cu_from_index_list (per_bfd, section, is_dwz, sect_off,
 				     length);
       per_bfd->all_comp_units.push_back (per_cu);
+      current_uiout->progress (per_bfd->all_comp_units.size ()
+			       / total_progress_steps);
     }
 }
 
@@ -2677,20 +2680,21 @@ create_cus_from_index_list (dwarf2_per_bfd *per_bfd,
 static void
 create_cus_from_index (dwarf2_per_bfd *per_bfd,
 		       const gdb_byte *cu_list, offset_type cu_list_elements,
-		       const gdb_byte *dwz_list, offset_type dwz_elements)
+		       const gdb_byte *dwz_list, offset_type dwz_elements,
+		       double total_progress_steps)
 {
   gdb_assert (per_bfd->all_comp_units.empty ());
   per_bfd->all_comp_units.reserve ((cu_list_elements + dwz_elements) / 2);
 
   create_cus_from_index_list (per_bfd, cu_list, cu_list_elements,
-			      &per_bfd->info, 0);
+			      &per_bfd->info, 0, total_progress_steps);
 
   if (dwz_elements == 0)
     return;
 
   dwz_file *dwz = dwarf2_get_dwz_file (per_bfd);
   create_cus_from_index_list (per_bfd, dwz_list, dwz_elements,
-			      &dwz->info, 1);
+			      &dwz->info, 1, total_progress_steps);
 }
 
 /* Create the signatured type hash table from the index.  */
@@ -2698,7 +2702,8 @@ create_cus_from_index (dwarf2_per_bfd *per_bfd,
 static void
 create_signatured_type_table_from_index
   (dwarf2_per_bfd *per_bfd, struct dwarf2_section_info *section,
-   const gdb_byte *bytes, offset_type elements)
+   const gdb_byte *bytes, offset_type elements,
+   double base_steps, double total_progress_steps)
 {
   gdb_assert (per_bfd->all_type_units.empty ());
   per_bfd->all_type_units.reserve (elements / 3);
@@ -2735,6 +2740,7 @@ create_signatured_type_table_from_index
       *slot = sig_type;
 
       per_bfd->all_type_units.push_back (sig_type);
+      current_uiout->progress ((base_steps + i) / total_progress_steps);
     }
 
   per_bfd->signatured_types = std::move (sig_types_hash);
@@ -3257,8 +3263,11 @@ dwarf2_read_gdb_index
 	}
     }
 
+  double cu_steps = (double) cu_list_elements + dwz_list_elements;
+  double total_steps = cu_steps + types_list_elements;
+
   create_cus_from_index (per_bfd, cu_list, cu_list_elements, dwz_list,
-			 dwz_list_elements);
+			 dwz_list_elements, total_steps);
 
   if (types_list_elements)
     {
@@ -3270,7 +3279,8 @@ dwarf2_read_gdb_index
       dwarf2_section_info *section = &per_bfd->types[0];
 
       create_signatured_type_table_from_index (per_bfd, section, types_list,
-					       types_list_elements);
+					       types_list_elements,
+					       cu_steps, total_steps);
     }
 
   create_addrmap_from_index (per_objfile, map.get ());
@@ -3852,6 +3862,8 @@ dw2_expand_all_symtabs (struct objfile *objfile)
 	 dw2_instantiate_symtab to skip partial CUs -- any important
 	 partial CU will be read via DW_TAG_imported_unit anyway.  */
       dw2_instantiate_symtab (per_cu, per_objfile, true);
+
+      current_uiout->progress ((double) i / total_units);
     }
 }
 
@@ -8165,13 +8177,16 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
     = make_scoped_restore (&objfile->partial_symtabs->psymtabs_addrmap,
 			   addrmap_create_mutable (&temp_obstack));
 
-  for (dwarf2_per_cu_data *per_cu : per_objfile->per_bfd->all_comp_units)
+  int n_cus = per_objfile->per_bfd->all_comp_units.size ();
+  for (int i = 0; i < n_cus; ++i)
     {
+      dwarf2_per_cu_data *per_cu = per_objfile->per_bfd->all_comp_units[i];
       if (per_cu->v.psymtab != NULL)
 	/* In case a forward DW_TAG_imported_unit has read the CU already.  */
 	continue;
       process_psymtab_comp_unit (per_cu, per_objfile, false,
 				 language_minimal);
+      current_uiout->progress ((double) i / n_cus);
     }
 
   /* This has to wait until we read the CUs, we need the list of DWOs.  */
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 58fd067905d..cac1356cc60 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -36,6 +36,7 @@
 #include "gdbcmd.h"
 #include <algorithm>
 #include <set>
+#include "cli/cli-style.h"
 
 static struct partial_symbol *lookup_partial_symbol (struct objfile *,
 						     struct partial_symtab *,
@@ -86,10 +87,20 @@ require_partial_symbols (struct objfile *objfile, bool verbose)
 
       if (objfile->sf->sym_read_psymbols)
 	{
+	  gdb::optional<ui_out::progress_meter> meter;
+
 	  if (verbose)
-	    printf_filtered (_("Reading symbols from %s...\n"),
-			     objfile_name (objfile));
+	    {
+	      string_file styled (current_uiout->can_emit_style_escape ());
+	      fprintf_filtered (&styled,
+				_("Reading symbols from %ps..."),
+				styled_string (file_name_style.style (),
+					       objfile_name (objfile)));
+	      meter.emplace (current_uiout, styled.string (), verbose);
+	    }
+
 	  (*objfile->sf->sym_read_psymbols) (objfile);
+	  meter.reset ();
 
 	  if (verbose && !objfile_has_symbols (objfile))
 	    printf_filtered (_("(No debugging symbols found in %s)\n"),
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 82693f91367..2de73c4b16c 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1073,15 +1073,25 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name,
   /* We either created a new mapped symbol table, mapped an existing
      symbol table file which has not had initial symbol reading
      performed, or need to read an unmapped symbol table.  */
-  if (should_print)
-    {
-      if (deprecated_pre_add_symbol_hook)
-	deprecated_pre_add_symbol_hook (name);
-      else
-	printf_filtered (_("Reading symbols from %ps...\n"),
-			 styled_string (file_name_style.style (), name));
-    }
-  syms_from_objfile (objfile, addrs, add_flags);
+  {
+    std::string message;
+    if (should_print)
+      {
+	if (deprecated_pre_add_symbol_hook)
+	  deprecated_pre_add_symbol_hook (name);
+	else
+	  {
+	    string_file styled (current_uiout->can_emit_style_escape ());
+	    fprintf_filtered (&styled,
+			      _("Reading symbols from %ps..."),
+			      styled_string (file_name_style.style (), name));
+	    message = std::move (styled.string ());
+	  }
+      }
+
+    ui_out::progress_meter meter (current_uiout, message, should_print);
+    syms_from_objfile (objfile, addrs, add_flags);
+  }
 
   /* We now have at least a partial symbol table.  Check to see if the
      user requested that all symbols be read on initial access via either
@@ -1090,10 +1100,16 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name,
 
   if ((flags & OBJF_READNOW))
     {
+      std::string message;
       if (should_print)
-	printf_filtered (_("Expanding full symbols from %ps...\n"),
-			 styled_string (file_name_style.style (), name));
+	{
+	  string_file styled (current_uiout->can_emit_style_escape ());
+	  fprintf_filtered (&styled, _("Expanding full symbols from %ps...\n"),
+			    styled_string (file_name_style.style (), name));
+	  message = std::move (styled.string ());
+	}
 
+      ui_out::progress_meter meter (current_uiout, message, should_print);
       if (objfile->sf)
 	objfile->sf->qf->expand_all_symtabs (objfile);
     }
@@ -2442,9 +2458,6 @@ reread_symbols (void)
       new_modtime = new_statbuf.st_mtime;
       if (new_modtime != objfile->mtime)
 	{
-	  printf_filtered (_("`%s' has changed; re-reading symbols.\n"),
-			   objfile_name (objfile));
-
 	  /* There are various functions like symbol_file_add,
 	     symfile_bfd_open, syms_from_objfile, etc., which might
 	     appear to do what we want.  But they have various other
@@ -2584,7 +2597,17 @@ reread_symbols (void)
 	  /* Recompute section offsets and section indices.  */
 	  objfile->sf->sym_offsets (objfile, {});
 
-	  read_symbols (objfile, 0);
+	  {
+	    string_file styled (current_uiout->can_emit_style_escape ());
+	    fprintf_filtered (&styled,
+			      _("`%ps' has changed; re-reading symbols.\n"),
+			      styled_string (file_name_style.style (),
+					     objfile_name (objfile)));
+	    ui_out::progress_meter meter (current_uiout, styled.string (),
+					  true);
+
+	    read_symbols (objfile, 0);
+	  }
 
 	  if (!objfile_has_symbols (objfile))
 	    {
-- 
2.26.2

Re: [PATCH] Create a progress meter when reading symbols

[Simon Marchi]

On 2021-01-10 12:52 p.m., Tom Tromey wrote:
> This patch changes symbol reading to create a progress meter, and
> changes the DWARF reader to update it.  This makes gdb give some
> feedback when reading symbols.
> 
> Regression tested on x86-64 Fedora 32.

This is awesome, I love it!

Simon

Re: [PATCH] Create a progress meter when reading symbols

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> On 2021-01-10 12:52 p.m., Tom Tromey wrote:
>> This patch changes symbol reading to create a progress meter, and
>> changes the DWARF reader to update it.  This makes gdb give some
>> feedback when reading symbols.
>> 
>> Regression tested on x86-64 Fedora 32.

Simon> This is awesome, I love it!

Thanks.

I re-ran the regression tests, and there is a failure, so I will hold
onto it until I get a chance to debug that.

Tom