[RFC] Add ability to skip demangling partial symbols

Message ID 20200220004554.24976-1-tom@tromey.com
State New
Headers show
Series
  • [RFC] Add ability to skip demangling partial symbols
Related show

Commit Message

Tom Tromey Feb. 20, 2020, 12:45 a.m.
Since the "physname" patches (many years ago now), the DWARF reader
essentially computes the mangled itself while reading psymbols, and
passes this to add_psymbol_to_list.

This function then calls compute_and_set_names -- which tries to
demangle the name again.  However, this is just extra work that, if it
ever succeeds, can only do so by accident.

It would be good to fix the physname patches to do the right thing.
That is, store the mangled form, and arrange to set both forms when
creating the partial symbol (without re-demangling).

However, given that it has been many years and this hasn't happened,
it seemed to me that it would be ok to at least exploit the efficiency
gain here, by avoiding demangling.  That is what this patch
implements.

In my build this speeds up "file ./gdb" from:

Command execution time: 6.090913 (cpu), 5.925363 (wall)

to:

Command execution time: 5.306672 (cpu), 5.138870 (wall)

gdb/ChangeLog
2020-02-19  Tom Tromey  <tom@tromey.com>

	* psymtab.c (add_psymbol_to_bcache, add_psymbol_to_list): Add
	"no_demangle" parameter.
	* psympriv.h (add_psymbol_to_list): Add "no_demangle" parameter.
	* dwarf2/read.c (add_partial_symbol, load_partial_dies): Update
	calls to add_psymbol_to_list.
---
 gdb/ChangeLog     |  8 ++++++++
 gdb/dwarf2/read.c | 24 ++++++++++++------------
 gdb/psympriv.h    |  9 +++++++--
 gdb/psymtab.c     | 26 ++++++++++++++++++++++----
 4 files changed, 49 insertions(+), 18 deletions(-)

-- 
2.17.2

Comments

Tom Tromey March 14, 2020, 7:11 p.m. | #1
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:


Tom> Since the "physname" patches (many years ago now), the DWARF reader
Tom> essentially computes the mangled itself while reading psymbols, and
Tom> passes this to add_psymbol_to_list.

Tom> This function then calls compute_and_set_names -- which tries to
Tom> demangle the name again.  However, this is just extra work that, if it
Tom> ever succeeds, can only do so by accident.

Tom> It would be good to fix the physname patches to do the right thing.
Tom> That is, store the mangled form, and arrange to set both forms when
Tom> creating the partial symbol (without re-demangling).

Tom> However, given that it has been many years and this hasn't happened,
Tom> it seemed to me that it would be ok to at least exploit the efficiency
Tom> gain here, by avoiding demangling.  That is what this patch
Tom> implements.

Any comments on this?  I'm inclined to check it in but I wonder what
others think.

Tom
Tom Tromey March 24, 2020, 3:56 p.m. | #2
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:


Tom> However, given that it has been many years and this hasn't happened,
Tom> it seemed to me that it would be ok to at least exploit the efficiency
Tom> gain here, by avoiding demangling.  That is what this patch
Tom> implements.

While looking into another bug, I needed this patch as well; but this
time I found a somewhat cleaner way to implement it.  So, I will most
likely be dropping this one.

Tom

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4d767a59af7..34be28ebc2f 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -8164,7 +8164,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			       SECT_OFF_TEXT (objfile),
 			       psymbol_placement::GLOBAL,
 			       addr,
-			       cu->language, objfile);
+			       cu->language, objfile, true);
 	}
       else
 	{
@@ -8173,7 +8173,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			       VAR_DOMAIN, LOC_BLOCK,
 			       SECT_OFF_TEXT (objfile),
 			       psymbol_placement::STATIC,
-			       addr, cu->language, objfile);
+			       addr, cu->language, objfile, true);
 	}
 
       if (pdi->main_subprogram && actual_name != NULL)
@@ -8185,7 +8185,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			   -1, (pdi->is_external
 				? psymbol_placement::GLOBAL
 				: psymbol_placement::STATIC),
-			   0, cu->language, objfile);
+			   0, cu->language, objfile, true);
       break;
     case DW_TAG_variable:
       if (pdi->d.locdesc)
@@ -8221,7 +8221,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 				 VAR_DOMAIN, LOC_STATIC,
 				 SECT_OFF_TEXT (objfile),
 				 psymbol_placement::GLOBAL,
-				 addr, cu->language, objfile);
+				 addr, cu->language, objfile, true);
 	}
       else
 	{
@@ -8238,7 +8238,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			       SECT_OFF_TEXT (objfile),
 			       psymbol_placement::STATIC,
 			       has_loc ? addr : 0,
-			       cu->language, objfile);
+			       cu->language, objfile, true);
 	}
       break;
     case DW_TAG_typedef:
@@ -8248,7 +8248,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			   built_actual_name != NULL,
 			   VAR_DOMAIN, LOC_TYPEDEF, -1,
 			   psymbol_placement::STATIC,
-			   0, cu->language, objfile);
+			   0, cu->language, objfile, true);
       break;
     case DW_TAG_imported_declaration:
     case DW_TAG_namespace:
@@ -8256,7 +8256,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			   built_actual_name != NULL,
 			   VAR_DOMAIN, LOC_TYPEDEF, -1,
 			   psymbol_placement::GLOBAL,
-			   0, cu->language, objfile);
+			   0, cu->language, objfile, true);
       break;
     case DW_TAG_module:
       /* With Fortran 77 there might be a "BLOCK DATA" module
@@ -8267,7 +8267,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			     built_actual_name != NULL,
 			     MODULE_DOMAIN, LOC_TYPEDEF, -1,
 			     psymbol_placement::GLOBAL,
-			     0, cu->language, objfile);
+			     0, cu->language, objfile, true);
       break;
     case DW_TAG_class_type:
     case DW_TAG_interface_type:
@@ -8290,7 +8290,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			   cu->language == language_cplus
 			   ? psymbol_placement::GLOBAL
 			   : psymbol_placement::STATIC,
-			   0, cu->language, objfile);
+			   0, cu->language, objfile, true);
 
       break;
     case DW_TAG_enumerator:
@@ -8300,7 +8300,7 @@  add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 			   cu->language == language_cplus
 			   ? psymbol_placement::GLOBAL
 			   : psymbol_placement::STATIC,
-			   0, cu->language, objfile);
+			   0, cu->language, objfile, true);
       break;
     default:
       break;
@@ -17758,7 +17758,7 @@  load_partial_dies (const struct die_reader_specs *reader,
 	    add_psymbol_to_list (pdi.name, false,
 				 VAR_DOMAIN, LOC_TYPEDEF, -1,
 				 psymbol_placement::STATIC,
-				 0, cu->language, objfile);
+				 0, cu->language, objfile, true);
 	  info_ptr = locate_pdi_sibling (reader, &pdi, info_ptr);
 	  continue;
 	}
@@ -17794,7 +17794,7 @@  load_partial_dies (const struct die_reader_specs *reader,
 				 cu->language == language_cplus
 				 ? psymbol_placement::GLOBAL
 				 : psymbol_placement::STATIC,
-				 0, cu->language, objfile);
+				 0, cu->language, objfile, true);
 
 	  info_ptr = locate_pdi_sibling (reader, &pdi, info_ptr);
 	  continue;
diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index 1b1f57cf764..7fa2010743b 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -404,7 +404,11 @@  enum class psymbol_placement
    an address, zero is passed.
 
    LANGUAGE is the language from which the symbol originates.  This will
-   influence, amongst other things, how the symbol name is demangled. */
+   influence, amongst other things, how the symbol name is demangled.
+
+   NO_DEMANGLE can be used to indicate that NAME has already been
+   demangled.  Some symbol readers compute the demangled name, and
+   this avoids the extra expense of an attempted demangling.  */
 
 extern void add_psymbol_to_list (gdb::string_view name,
 				 bool copy_name, domain_enum domain,
@@ -413,7 +417,8 @@  extern void add_psymbol_to_list (gdb::string_view name,
 				 psymbol_placement where,
 				 CORE_ADDR coreaddr,
 				 enum language language,
-				 struct objfile *objfile);
+				 struct objfile *objfile,
+				 bool no_demangle = false);
 
 /* Initialize storage for partial symbols.  If partial symbol storage
    has already been initialized, this does nothing.  TOTAL_SYMBOLS is
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 56576b3bcec..07cae9ce8e8 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1570,7 +1570,7 @@  add_psymbol_to_bcache (gdb::string_view name, bool copy_name,
 		       short section,
 		       CORE_ADDR coreaddr,
 		       enum language language, struct objfile *objfile,
-		       int *added)
+		       bool no_demangle, int *added)
 {
   struct partial_symbol psymbol;
   memset (&psymbol, 0, sizeof (psymbol));
@@ -1580,7 +1580,23 @@  add_psymbol_to_bcache (gdb::string_view name, bool copy_name,
   psymbol.domain = domain;
   psymbol.aclass = theclass;
   psymbol.ginfo.set_language (language, objfile->partial_symtabs->obstack ());
-  psymbol.ginfo.compute_and_set_names (name, copy_name, objfile->per_bfd);
+
+  if (no_demangle)
+    {
+      /* NAME is already demangled.  */
+      const char *stored_name;
+      if (copy_name)
+	stored_name = ((const char *) objfile->per_bfd->filename_cache
+		       .insert (name.data (), name.size ()));
+      else
+	stored_name = name.data ();
+
+      psymbol.ginfo.m_name = stored_name;
+      symbol_set_demangled_name (&psymbol.ginfo, nullptr,
+				 &objfile->per_bfd->storage_obstack);
+    }
+  else
+    psymbol.ginfo.compute_and_set_names (name, copy_name, objfile->per_bfd);
 
   /* Stash the partial symbol away in the cache.  */
   return ((struct partial_symbol *)
@@ -1608,7 +1624,8 @@  add_psymbol_to_list (gdb::string_view name, bool copy_name,
 		     short section,
 		     psymbol_placement where,
 		     CORE_ADDR coreaddr,
-		     enum language language, struct objfile *objfile)
+		     enum language language, struct objfile *objfile,
+		     bool no_demangle)
 {
   struct partial_symbol *psym;
 
@@ -1616,7 +1633,8 @@  add_psymbol_to_list (gdb::string_view name, bool copy_name,
 
   /* Stash the partial symbol away in the cache.  */
   psym = add_psymbol_to_bcache (name, copy_name, domain, theclass,
-				section, coreaddr, language, objfile, &added);
+				section, coreaddr, language, objfile,
+				no_demangle, &added);
 
   /* Do not duplicate global partial symbols.  */
   if (where == psymbol_placement::GLOBAL && !added)