public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: [PATCH v2 4/8] Lock the demangled hash table
Date: Sat, 18 May 2019 21:00:00 -0000	[thread overview]
Message-ID: <20190518210010.27697-5-tom@tromey.com> (raw)
In-Reply-To: <20190518210010.27697-1-tom@tromey.com>

This introduces a lock that is used when modifying the demangled hash
table.

gdb/ChangeLog
2019-05-18  Tom Tromey  <tom@tromey.com>

	* symtab.c (demangled_mutex): New global.
	(symbol_set_names): Use a lock_guard.
---
 gdb/ChangeLog |   5 ++
 gdb/symtab.c  | 134 ++++++++++++++++++++++++++++----------------------
 2 files changed, 81 insertions(+), 58 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 130d5cd48ff..6ad024a8a29 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -69,6 +69,9 @@
 #include "arch-utils.h"
 #include <algorithm>
 #include "common/pathstuff.h"
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* Forward declarations for local functions.  */
 
@@ -709,6 +712,11 @@ symbol_set_language (struct general_symbol_info *gsymbol,
 
 /* Functions to initialize a symbol's mangled name.  */
 
+#if CXX_STD_THREAD
+/* Mutex that is used when modifying the demangled hash table.  */
+static std::mutex demangled_mutex;
+#endif
+
 /* Objects of this type are stored in the demangled name hash table.  */
 struct demangled_name_entry
 {
@@ -837,9 +845,6 @@ symbol_set_names (struct general_symbol_info *gsymbol,
       return;
     }
 
-  if (per_bfd->demangled_names_hash == NULL)
-    create_demangled_names_hash (per_bfd);
-
   if (linkage_name[len] != '\0')
     {
       char *alloc_name;
@@ -858,64 +863,77 @@ symbol_set_names (struct general_symbol_info *gsymbol,
     = symbol_find_demangled_name (gsymbol, linkage_name_copy);
   gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
 
-  entry.mangled = linkage_name_copy;
-  slot = ((struct demangled_name_entry **)
-	  htab_find_slot (per_bfd->demangled_names_hash.get (),
-			  &entry, INSERT));
-
-  /* If this name is not in the hash table, add it.  */
-  if (*slot == NULL
-      /* A C version of the symbol may have already snuck into the table.
-	 This happens to, e.g., main.init (__go_init_main).  Cope.  */
-      || (gsymbol->language == language_go
-	  && (*slot)->demangled[0] == '\0'))
-    {
-      int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
-
-      /* Suppose we have demangled_name==NULL, copy_name==0, and
-	 linkage_name_copy==linkage_name.  In this case, we already have the
-	 mangled name saved, and we don't have a demangled name.  So,
-	 you might think we could save a little space by not recording
-	 this in the hash table at all.
+  struct demangled_name_entry *found_entry;
+
+  {
+#if CXX_STD_THREAD
+    std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+
+    if (per_bfd->demangled_names_hash == NULL)
+      create_demangled_names_hash (per_bfd);
+
+    entry.mangled = linkage_name_copy;
+    slot = ((struct demangled_name_entry **)
+	    htab_find_slot (per_bfd->demangled_names_hash.get (),
+			    &entry, INSERT));
+
+    /* If this name is not in the hash table, add it.  */
+    if (*slot == NULL
+	/* A C version of the symbol may have already snuck into the table.
+	   This happens to, e.g., main.init (__go_init_main).  Cope.  */
+	|| (gsymbol->language == language_go
+	    && (*slot)->demangled[0] == '\0'))
+      {
+	int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
+
+	/* Suppose we have demangled_name==NULL, copy_name==0, and
+	   linkage_name_copy==linkage_name.  In this case, we already have the
+	   mangled name saved, and we don't have a demangled name.  So,
+	   you might think we could save a little space by not recording
+	   this in the hash table at all.
 	 
-	 It turns out that it is actually important to still save such
-	 an entry in the hash table, because storing this name gives
-	 us better bcache hit rates for partial symbols.  */
-      if (!copy_name && linkage_name_copy == linkage_name)
-	{
-	  *slot
-	    = ((struct demangled_name_entry *)
-	       obstack_alloc (&per_bfd->storage_obstack,
-			      offsetof (struct demangled_name_entry, demangled)
-			      + demangled_len + 1));
-	  (*slot)->mangled = linkage_name;
-	}
-      else
-	{
-	  char *mangled_ptr;
-
-	  /* If we must copy the mangled name, put it directly after
-	     the demangled name so we can have a single
-	     allocation.  */
-	  *slot
-	    = ((struct demangled_name_entry *)
-	       obstack_alloc (&per_bfd->storage_obstack,
-			      offsetof (struct demangled_name_entry, demangled)
-			      + len + demangled_len + 2));
-	  mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
-	  strcpy (mangled_ptr, linkage_name_copy);
-	  (*slot)->mangled = mangled_ptr;
-	}
+	   It turns out that it is actually important to still save such
+	   an entry in the hash table, because storing this name gives
+	   us better bcache hit rates for partial symbols.  */
+	if (!copy_name && linkage_name_copy == linkage_name)
+	  {
+	    *slot
+	      = ((struct demangled_name_entry *)
+		 obstack_alloc (&per_bfd->storage_obstack,
+				offsetof (struct demangled_name_entry, demangled)
+				+ demangled_len + 1));
+	    (*slot)->mangled = linkage_name;
+	  }
+	else
+	  {
+	    char *mangled_ptr;
+
+	    /* If we must copy the mangled name, put it directly after
+	       the demangled name so we can have a single
+	       allocation.  */
+	    *slot
+	      = ((struct demangled_name_entry *)
+		 obstack_alloc (&per_bfd->storage_obstack,
+				offsetof (struct demangled_name_entry, demangled)
+				+ len + demangled_len + 2));
+	    mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
+	    strcpy (mangled_ptr, linkage_name_copy);
+	    (*slot)->mangled = mangled_ptr;
+	  }
 
-      if (demangled_name != NULL)
-	strcpy ((*slot)->demangled, demangled_name.get());
-      else
-	(*slot)->demangled[0] = '\0';
-    }
+	if (demangled_name != NULL)
+	  strcpy ((*slot)->demangled, demangled_name.get());
+	else
+	  (*slot)->demangled[0] = '\0';
+      }
+
+    found_entry = *slot;
+  }
 
-  gsymbol->name = (*slot)->mangled;
-  if ((*slot)->demangled[0] != '\0')
-    symbol_set_demangled_name (gsymbol, (*slot)->demangled,
+  gsymbol->name = found_entry->mangled;
+  if (found_entry->demangled[0] != '\0')
+    symbol_set_demangled_name (gsymbol, found_entry->demangled,
 			       &per_bfd->storage_obstack);
   else
     symbol_set_demangled_name (gsymbol, NULL, &per_bfd->storage_obstack);
-- 
2.17.2

  parent reply	other threads:[~2019-05-18 21:00 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-18 21:00 [PATCH v2 0/8] Demangle minimal symbol names in worker threads Tom Tromey
2019-05-18 21:00 ` [PATCH v2 2/8] Remove static buffer from ada_decode Tom Tromey
2019-05-18 21:00 ` [PATCH v2 5/8] Introduce run_on_main_thread Tom Tromey
2019-05-18 21:00 ` [PATCH v2 1/8] Defer minimal symbol name-setting Tom Tromey
2019-05-18 21:00 ` [PATCH v2 7/8] Demangle minsyms in parallel Tom Tromey
2019-05-18 21:00 ` [PATCH v2 3/8] Add configure check for std::thread Tom Tromey
2019-05-18 21:00 ` Tom Tromey [this message]
2019-05-18 21:00 ` [PATCH v2 8/8] Add maint set/show enable-threads Tom Tromey
2019-05-22  5:01   ` Eli Zaretskii
2019-05-26 20:46     ` Tom Tromey
2019-05-27  2:32       ` Eli Zaretskii
2019-05-18 21:00 ` [PATCH v2 6/8] Introduce thread-safe way to handle SIGSEGV Tom Tromey
2019-05-19 13:59 ` [PATCH v2 0/8] Demangle minimal symbol names in worker threads Philippe Waroquiers
2019-05-19 18:55   ` Tom Tromey
2019-05-21  0:35     ` Philippe Waroquiers
2019-05-21  7:35     ` Andrew Burgess
2019-05-21 15:45       ` Tom Tromey
2019-05-21 16:21         ` Andrew Burgess
2019-05-31  2:48     ` Tom Tromey
2019-05-31 17:13       ` Philippe Waroquiers
2019-09-29  0:35         ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
2019-09-30 14:18           ` Tom Tromey
2019-09-30 16:55             ` Christian Biesinger via gdb-patches
2019-10-02 17:18               ` Tom Tromey
2019-10-02 18:20                 ` Christian Biesinger via gdb-patches
2019-10-02 22:02                   ` Christian Biesinger via gdb-patches
2019-10-03 18:15                     ` [PATCH v2 1/2] " Christian Biesinger via gdb-patches
2019-10-03 18:15                     ` [PATCH v2 2/2] Precompute hash value for symbol_set_names Christian Biesinger via gdb-patches
2019-09-30 21:45             ` [PATCH] Don't use the mutex for each symbol_set_names call Christian Biesinger via gdb-patches
2019-10-01 17:02               ` Tom Tromey

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=20190518210010.27697-5-tom@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@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).