public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Sourceware to Gerrit sync (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Christian Biesinger <cbiesinger@google.com>,	gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@sourceware.org>
Subject: [pushed] Precompute hash value for symbol_set_names
Date: Wed, 27 Nov 2019 21:40:00 -0000	[thread overview]
Message-ID: <20191127214007.3D3C928173@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1572031795000.I044449e7eb60cffc1c43efd3412f2b485bd9faac@gnutoolchain-gerrit.osci.io>

Sourceware to Gerrit sync has submitted this change.

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-11-27  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/ChangeLog
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
4 files changed, 42 insertions(+), 6 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0816e73..c278a77 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-11-27  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.
+
 2019-11-27  Kevin Buettner  <kevinb@redhat.com>
 
 	* dwarf2read.c (inherit_abstract_dies): Ensure that delayed
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 03a1932..141c3d2 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -1258,6 +1258,16 @@
     }
 }
 
+/* This struct is used to store values we compute for msymbols on the
+   background threads but don't need to keep around long term.  */
+struct computed_hash_values
+{
+  /* Length of the linkage_name of the symbol.  */
+  size_t name_length;
+  /* Hash code (using fast_hash) of the linkage_name.  */
+  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.  */
@@ -1370,6 +1380,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],
@@ -1377,6 +1389,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.  */
@@ -1386,6 +1400,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);
 		 }
 	     }
 	   {
@@ -1396,8 +1413,14 @@
 #endif
 	     for (minimal_symbol *msym = start; msym < end; ++msym)
 	       {
-		 symbol_set_names (msym, msym->name, false,
-				   m_objfile->per_bfd);
+		 size_t idx = msym - msymbols;
+		 symbol_set_names
+		   (msym,
+		    gdb::string_view(msym->name,
+				     hash_values[idx].name_length),
+		    false,
+		    m_objfile->per_bfd,
+		    hash_values[idx].mangled_name_hash);
 	       }
 	   }
 	 });
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8f46321..894a323 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -836,7 +836,8 @@
 void
 symbol_set_names (struct general_symbol_info *gsymbol,
 		  gdb::string_view linkage_name, 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;
 
@@ -864,9 +865,11 @@
     create_demangled_names_hash (per_bfd);
 
   struct demangled_name_entry entry (linkage_name);
+  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 7a51456..4cfdf06 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -553,7 +553,9 @@
 		    (objfile)->per_bfd)
 extern void symbol_set_names (struct general_symbol_info *symbol,
 			      gdb::string_view linkage_name, 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> ());
 
 /* Return true if NAME matches the "search" name of SYMBOL, according
    to the symbol's language.  */

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

  parent reply	other threads:[~2019-11-27 21:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-25 19:30 [review] " Christian Biesinger (Code Review)
2019-10-29 19:28 ` Tom Tromey (Code Review)
2019-10-29 21:56 ` [review v2] " Christian Biesinger (Code Review)
2019-10-29 21:56 ` Christian Biesinger (Code Review)
2019-10-31  0:23 ` [review v3] " Christian Biesinger (Code Review)
2019-11-26 22:08 ` [review v4] " Tom Tromey (Code Review)
2019-11-26 22:23 ` [review v5] " Christian Biesinger (Code Review)
2019-11-26 22:24 ` Christian Biesinger (Code Review)
2019-11-27 18:03 ` Tom Tromey (Code Review)
2019-11-27 21:40 ` Sourceware to Gerrit sync (Code Review) [this message]
2019-11-27 21:40 ` [pushed] " Sourceware to Gerrit sync (Code Review)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191127214007.3D3C928173@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=cbiesinger@google.com \
    --cc=gdb-patches@sourceware.org \
    --cc=noreply@gnutoolchain-gerrit.osci.io \
    --cc=tromey@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).