[review v2] Precompute hash value for symbol_set_names

["Christian Biesinger (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307
......................................................................

Precompute hash value for symbol_set_names

We can also compute the hash for the mangled name on a background
thread so make this function even faster (about a 7% speedup).

gdb/ChangeLog:

2019-10-03  Christian Biesinger  <cbiesinger@google.com>

	* minsyms.c (minimal_symbol_reader::install): Also compute the hash
	of the mangled name on the background thread.
	* symtab.c (symbol_set_names): Allow passing in the hash of the
	linkage_name.
	* symtab.h (symbol_set_names): Likewise.

Change-Id: I044449e7eb60cffc1c43efd3412f2b485bd9faac
---
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
3 files changed, 28 insertions(+), 7 deletions(-)



diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index bfcd5d5..c53094b 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1249,6 +1249,12 @@
   return (mcount);
 }
 
+struct computed_hash_values
+{
+  size_t name_length;
+  hashval_t mangled_name_hash;
+};
+
 /* Build (or rebuild) the minimal symbol hash tables.  This is necessary
    after compacting or sorting the table since the entries move around
    thus causing the internal minimal_symbol pointers to become jumbled.  */
@@ -1365,6 +1371,8 @@
       std::mutex demangled_mutex;
 #endif
 
+      std::vector<computed_hash_values> hash_values (mcount);
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
       gdb::parallel_for_each
 	(&msymbols[0], &msymbols[mcount],
@@ -1372,6 +1380,8 @@
 	 {
 	   for (minimal_symbol *msym = start; msym < end; ++msym)
 	     {
+	       size_t idx = msym - msymbols;
+	       hash_values[idx].name_length = strlen (msym->name);
 	       if (!msym->name_set)
 		 {
 		   /* This will be freed later, by symbol_set_names.  */
@@ -1381,6 +1391,9 @@
 		     (msym, demangled_name,
 		      &m_objfile->per_bfd->storage_obstack);
 		   msym->name_set = 1;
+
+		   hash_values[idx].mangled_name_hash
+		     = fast_hash (msym->name, hash_values[idx].name_length);
 		 }
 	     }
 	   {
@@ -1391,9 +1404,11 @@
 #endif
 	     for (minimal_symbol *msym = start; msym < end; ++msym)
 	       {
+		 size_t idx = msym - msymbols;
 		 symbol_set_names (msym, msym->name,
-				   strlen (msym->name), 0,
-				   m_objfile->per_bfd);
+				   hash_values[idx].name_length, 0,
+				   m_objfile->per_bfd,
+				   hash_values[idx].mangled_name_hash);
 	       }
 	   }
 	 });
diff --git a/gdb/symtab.c b/gdb/symtab.c
index abc6a77..c2701db 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -825,7 +825,8 @@
 void
 symbol_set_names (struct general_symbol_info *gsymbol,
 		  const char *linkage_name, int len, bool copy_name,
-		  struct objfile_per_bfd_storage *per_bfd)
+		  struct objfile_per_bfd_storage *per_bfd,
+		  gdb::optional<hashval_t> hash)
 {
   struct demangled_name_entry **slot;
   /* A 0-terminated copy of the linkage name.  */
@@ -868,9 +869,11 @@
     linkage_name_copy = linkage_name;
 
   struct demangled_name_entry entry (gdb::string_view (linkage_name_copy, len));
+  if (!hash.has_value ())
+    hash = hash_demangled_name_entry (&entry);
   slot = ((struct demangled_name_entry **)
-	  htab_find_slot (per_bfd->demangled_names_hash.get (),
-			  &entry, INSERT));
+          htab_find_slot_with_hash (per_bfd->demangled_names_hash.get (),
+				    &entry, *hash, INSERT));
 
   /* If this name is not in the hash table, add it.  */
   if (*slot == NULL
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 7d41de9..e3a8d89 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -514,13 +514,16 @@
   (symbol)->ginfo.name = (linkage_name)
 
 /* Set the linkage and natural names of a symbol, by demangling
-   the linkage name.  */
+   the linkage name.  Optionally, HASH can be set to the value
+   of fast_hash (linkage_name), to speed up this function.  */
 #define SYMBOL_SET_NAMES(symbol,linkage_name,len,copy_name,objfile)	\
   symbol_set_names (&(symbol)->ginfo, linkage_name, len, copy_name, \
 		    (objfile)->per_bfd)
 extern void symbol_set_names (struct general_symbol_info *symbol,
 			      const char *linkage_name, int len, bool copy_name,
-			      struct objfile_per_bfd_storage *per_bfd);
+			      struct objfile_per_bfd_storage *per_bfd,
+			      gdb::optional<hashval_t> hash
+			        = gdb::optional<hashval_t> ());
 
 /* Now come lots of name accessor macros.  Short version as to when to
    use which: Use SYMBOL_NATURAL_NAME to refer to the name of the

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I044449e7eb60cffc1c43efd3412f2b485bd9faac
Gerrit-Change-Number: 307
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset