[review] Demangle minsyms in parallel

[review] Demangle minsyms in parallel

[Tom Tromey (Code Review)]

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

Demangle minsyms in parallel

This patch introduces a simple parallel for_each and changes the
minimal symbol reader to use it when computing the demangled name for
a minimal symbol.  This yields a speedup when reading minimal symbols.

gdb/ChangeLog
2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* minsyms.c (minimal_symbol_reader::install): Use
	parallel_for_each.
	* gdbsupport/parallel-for.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.

Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/parallel-for.h
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
6 files changed, 154 insertions(+), 17 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ab67332..f4ec322 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,14 @@
 2019-10-19  Christian Biesinger  <cbiesinger@google.com>
 	    Tom Tromey  <tom@tromey.com>
 
+	* minsyms.c (minimal_symbol_reader::install): Use
+	parallel_for_each.
+	* gdbsupport/parallel-for.h: New file.
+	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.
+
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
 	* gdbsupport/thread-pool.h: New file.
 	* gdbsupport/thread-pool.c: New file.
 	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3be798a..0d98c21 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1477,6 +1477,7 @@
 	gdbsupport/common-inferior.h \
 	gdbsupport/netstuff.h \
 	gdbsupport/host-defs.h \
+	gdbsupport/parallel-for.h \
 	gdbsupport/pathstuff.h \
 	gdbsupport/print-utils.h \
 	gdbsupport/ptid.h \
diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
new file mode 100644
index 0000000..52a5edf
--- /dev/null
+++ b/gdb/gdbsupport/parallel-for.h
@@ -0,0 +1,82 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_PARALLEL_FOR_H
+#define GDBSUPPORT_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <system_error>
+#include <thread>
+#include "gdbsupport/thread-pool.h"
+#endif
+
+namespace gdb
+{
+
+/* A very simple "parallel for".  This splits the range of iterators
+   into subranges, and then passes each subrange to the callback.  The
+   work may or may not be done in separate threads.
+
+   This approach was chosen over having the callback work on single
+   items because it makes it simple for the caller to do
+   once-per-subrange initialization and destruction.  */
+
+template<class RandomIt, class RangeFunction>
+void
+parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
+{
+#if CXX_STD_THREAD
+  /* So we can use a local array below.  */
+  const size_t local_max = 16;
+  size_t n_threads = std::min (thread_pool::g_thread_pool.count (), local_max);
+  size_t n_actual_threads = 0;
+  std::future<void> futures[local_max];
+
+  size_t n_elements = last - first;
+  if (n_threads > 1 && 2 * n_threads <= n_elements)
+    {
+      size_t elts_per_thread = n_elements / n_threads;
+      n_actual_threads = n_threads - 1;
+      for (int i = 0; i < n_actual_threads; ++i)
+	{
+	  RandomIt end = first + elts_per_thread;
+	  auto task = [=] ()
+		      {
+			callback (first, end);
+		      };
+
+	  futures[i] = gdb::thread_pool::g_thread_pool.post_task (task);
+	  first = end;
+	}
+    }
+#endif /* CXX_STD_THREAD */
+
+  /* Process all the remaining elements in the main thread.  */
+  callback (first, last);
+
+#if CXX_STD_THREAD
+  for (int i = 0; i < n_actual_threads; ++i)
+    futures[i].wait ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* GDBSUPPORT_PARALLEL_FOR_H */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 4e6bd39..bfcd5d5 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -53,6 +53,11 @@
 #include "gdbsupport/symbol.h"
 #include <algorithm>
 #include "safe-ctype.h"
+#include "gdbsupport/parallel-for.h"
+
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* See minsyms.h.  */
 
@@ -1354,17 +1359,44 @@
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+#if CXX_STD_THREAD
+      /* Mutex that is used when modifying or accessing the demangled
+	 hash table.  */
+      std::mutex demangled_mutex;
+#endif
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
-      for (int i = 0; i < mcount; ++i)
-	{
-	  if (!msymbols[i].name_set)
-	    {
-	      symbol_set_names (&msymbols[i], msymbols[i].name,
-				strlen (msymbols[i].name), 0,
-				m_objfile->per_bfd);
-	      msymbols[i].name_set = 1;
-	    }
-	}
+      gdb::parallel_for_each
+	(&msymbols[0], &msymbols[mcount],
+	 [&] (minimal_symbol *start, minimal_symbol *end)
+	 {
+	   for (minimal_symbol *msym = start; msym < end; ++msym)
+	     {
+	       if (!msym->name_set)
+		 {
+		   /* This will be freed later, by symbol_set_names.  */
+		   char *demangled_name
+		     = symbol_find_demangled_name (msym, msym->name);
+		   symbol_set_demangled_name
+		     (msym, demangled_name,
+		      &m_objfile->per_bfd->storage_obstack);
+		   msym->name_set = 1;
+		 }
+	     }
+	   {
+	     /* To limit how long we hold the lock, we only acquire it here
+	        and not while we demangle the names above.  */
+#if CXX_STD_THREAD
+	     std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+	     for (minimal_symbol *msym = start; msym < end; ++msym)
+	       {
+		 symbol_set_names (msym, msym->name,
+				   strlen (msym->name), 0,
+				   m_objfile->per_bfd);
+	       }
+	   }
+	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index fc736fd..da7b703 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -759,13 +759,9 @@
      NULL, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -867,8 +863,16 @@
       || (gsymbol->language == language_go
 	  && (*slot)->demangled[0] == '\0'))
     {
+      /* The const_cast is safe because the only reason it is already
+         initialized is if we purposefully set it from a background
+         thread to avoid doing the work here.  However, it is still
+         allocated from the heap and needs to be freed by us, just
+         like if we called symbol_find_demangled_name here.  */
       char *demangled_name_ptr
-	= symbol_find_demangled_name (gsymbol, linkage_name_copy);
+        = (gsymbol->language_specific.demangled_name
+	   ? const_cast<char *> (gsymbol->language_specific.demangled_name)
+	   : symbol_find_demangled_name (gsymbol, linkage_name_copy));
+
       gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
       int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 8552bf1..7d41de9 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -495,6 +495,16 @@
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must

[review] Demangle minsyms in parallel

[Christian Biesinger (Code Review)]

Christian Biesinger has posted comments on this change.

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


Patch Set 1: Code-Review+1

This looks good to me. Updated performance numbers, with my usual "attach to chrome" test:

Before this patch:
real	0m36.525s
user	0m28.044s
sys	0m8.481s
(this is faster than it used to be because of a change I made to chrome's new objfile handler & because of https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/124)

After this patch set:
real	0m18.179s
user	0m28.180s
sys	0m4.612s

So takes half the time now! The various other patches I've send to the list bring it down to ~10 sec, I will upload them to gerrit later.

[review v2] Demangle minsyms in parallel

[Tom Tromey (Code Review)]

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

Demangle minsyms in parallel

This patch introduces a simple parallel for_each and changes the
minimal symbol reader to use it when computing the demangled name for
a minimal symbol.  This yields a speedup when reading minimal symbols.

gdb/ChangeLog
2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* minsyms.c (minimal_symbol_reader::install): Use
	parallel_for_each.
	* gdbsupport/parallel-for.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.

Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/parallel-for.h
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
6 files changed, 155 insertions(+), 17 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 78585e1..400ccc1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,14 @@
 2019-10-19  Christian Biesinger  <cbiesinger@google.com>
 	    Tom Tromey  <tom@tromey.com>
 
+	* minsyms.c (minimal_symbol_reader::install): Use
+	parallel_for_each.
+	* gdbsupport/parallel-for.h: New file.
+	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.
+
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
 	* gdbsupport/thread-pool.h: New file.
 	* gdbsupport/thread-pool.c: New file.
 	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3be798a..0d98c21 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1477,6 +1477,7 @@
 	gdbsupport/common-inferior.h \
 	gdbsupport/netstuff.h \
 	gdbsupport/host-defs.h \
+	gdbsupport/parallel-for.h \
 	gdbsupport/pathstuff.h \
 	gdbsupport/print-utils.h \
 	gdbsupport/ptid.h \
diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
new file mode 100644
index 0000000..21f1a1e
--- /dev/null
+++ b/gdb/gdbsupport/parallel-for.h
@@ -0,0 +1,83 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_PARALLEL_FOR_H
+#define GDBSUPPORT_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <system_error>
+#include <thread>
+#include "gdbsupport/thread-pool.h"
+#endif
+
+namespace gdb
+{
+
+/* A very simple "parallel for".  This splits the range of iterators
+   into subranges, and then passes each subrange to the callback.  The
+   work may or may not be done in separate threads.
+
+   This approach was chosen over having the callback work on single
+   items because it makes it simple for the caller to do
+   once-per-subrange initialization and destruction.  */
+
+template<class RandomIt, class RangeFunction>
+void
+parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
+{
+#if CXX_STD_THREAD
+  /* So we can use a local array below.  */
+  const size_t local_max = 16;
+  size_t n_threads = std::min (thread_pool::g_thread_pool->count (),
+			       local_max);
+  size_t n_actual_threads = 0;
+  std::future<void> futures[local_max];
+
+  size_t n_elements = last - first;
+  if (n_threads > 1 && 2 * n_threads <= n_elements)
+    {
+      size_t elts_per_thread = n_elements / n_threads;
+      n_actual_threads = n_threads - 1;
+      for (int i = 0; i < n_actual_threads; ++i)
+	{
+	  RandomIt end = first + elts_per_thread;
+	  auto task = [=] ()
+		      {
+			callback (first, end);
+		      };
+
+	  futures[i] = gdb::thread_pool::g_thread_pool->post_task (task);
+	  first = end;
+	}
+    }
+#endif /* CXX_STD_THREAD */
+
+  /* Process all the remaining elements in the main thread.  */
+  callback (first, last);
+
+#if CXX_STD_THREAD
+  for (int i = 0; i < n_actual_threads; ++i)
+    futures[i].wait ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* GDBSUPPORT_PARALLEL_FOR_H */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 4e6bd39..bfcd5d5 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -53,6 +53,11 @@
 #include "gdbsupport/symbol.h"
 #include <algorithm>
 #include "safe-ctype.h"
+#include "gdbsupport/parallel-for.h"
+
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* See minsyms.h.  */
 
@@ -1354,17 +1359,44 @@
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+#if CXX_STD_THREAD
+      /* Mutex that is used when modifying or accessing the demangled
+	 hash table.  */
+      std::mutex demangled_mutex;
+#endif
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
-      for (int i = 0; i < mcount; ++i)
-	{
-	  if (!msymbols[i].name_set)
-	    {
-	      symbol_set_names (&msymbols[i], msymbols[i].name,
-				strlen (msymbols[i].name), 0,
-				m_objfile->per_bfd);
-	      msymbols[i].name_set = 1;
-	    }
-	}
+      gdb::parallel_for_each
+	(&msymbols[0], &msymbols[mcount],
+	 [&] (minimal_symbol *start, minimal_symbol *end)
+	 {
+	   for (minimal_symbol *msym = start; msym < end; ++msym)
+	     {
+	       if (!msym->name_set)
+		 {
+		   /* This will be freed later, by symbol_set_names.  */
+		   char *demangled_name
+		     = symbol_find_demangled_name (msym, msym->name);
+		   symbol_set_demangled_name
+		     (msym, demangled_name,
+		      &m_objfile->per_bfd->storage_obstack);
+		   msym->name_set = 1;
+		 }
+	     }
+	   {
+	     /* To limit how long we hold the lock, we only acquire it here
+	        and not while we demangle the names above.  */
+#if CXX_STD_THREAD
+	     std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+	     for (minimal_symbol *msym = start; msym < end; ++msym)
+	       {
+		 symbol_set_names (msym, msym->name,
+				   strlen (msym->name), 0,
+				   m_objfile->per_bfd);
+	       }
+	   }
+	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index fc736fd..da7b703 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -759,13 +759,9 @@
      NULL, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -867,8 +863,16 @@
       || (gsymbol->language == language_go
 	  && (*slot)->demangled[0] == '\0'))
     {
+      /* The const_cast is safe because the only reason it is already
+         initialized is if we purposefully set it from a background
+         thread to avoid doing the work here.  However, it is still
+         allocated from the heap and needs to be freed by us, just
+         like if we called symbol_find_demangled_name here.  */
       char *demangled_name_ptr
-	= symbol_find_demangled_name (gsymbol, linkage_name_copy);
+        = (gsymbol->language_specific.demangled_name
+	   ? const_cast<char *> (gsymbol->language_specific.demangled_name)
+	   : symbol_find_demangled_name (gsymbol, linkage_name_copy));
+
       gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
       int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 8552bf1..7d41de9 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -495,6 +495,16 @@
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must

[review v3] Demangle minsyms in parallel

[Tom Tromey (Code Review)]

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

Demangle minsyms in parallel

This patch introduces a simple parallel for_each and changes the
minimal symbol reader to use it when computing the demangled name for
a minimal symbol.  This yields a speedup when reading minimal symbols.

2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* minsyms.c (minimal_symbol_reader::install): Use
	parallel_for_each.
	* gdbsupport/parallel-for.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.

Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/parallel-for.h
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
6 files changed, 155 insertions(+), 18 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 273345b..065c752 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,14 @@
 2019-10-19  Christian Biesinger  <cbiesinger@google.com>
 	    Tom Tromey  <tom@tromey.com>
 
+	* minsyms.c (minimal_symbol_reader::install): Use
+	parallel_for_each.
+	* gdbsupport/parallel-for.h: New file.
+	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.
+
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
 	* gdbsupport/thread-pool.h: New file.
 	* gdbsupport/thread-pool.c: New file.
 	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index b42e8d1..9f9c802 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1487,6 +1487,7 @@
 	gdbsupport/common-inferior.h \
 	gdbsupport/netstuff.h \
 	gdbsupport/host-defs.h \
+	gdbsupport/parallel-for.h \
 	gdbsupport/pathstuff.h \
 	gdbsupport/print-utils.h \
 	gdbsupport/ptid.h \
diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
new file mode 100644
index 0000000..21f1a1e
--- /dev/null
+++ b/gdb/gdbsupport/parallel-for.h
@@ -0,0 +1,83 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_PARALLEL_FOR_H
+#define GDBSUPPORT_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <system_error>
+#include <thread>
+#include "gdbsupport/thread-pool.h"
+#endif
+
+namespace gdb
+{
+
+/* A very simple "parallel for".  This splits the range of iterators
+   into subranges, and then passes each subrange to the callback.  The
+   work may or may not be done in separate threads.
+
+   This approach was chosen over having the callback work on single
+   items because it makes it simple for the caller to do
+   once-per-subrange initialization and destruction.  */
+
+template<class RandomIt, class RangeFunction>
+void
+parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
+{
+#if CXX_STD_THREAD
+  /* So we can use a local array below.  */
+  const size_t local_max = 16;
+  size_t n_threads = std::min (thread_pool::g_thread_pool->count (),
+			       local_max);
+  size_t n_actual_threads = 0;
+  std::future<void> futures[local_max];
+
+  size_t n_elements = last - first;
+  if (n_threads > 1 && 2 * n_threads <= n_elements)
+    {
+      size_t elts_per_thread = n_elements / n_threads;
+      n_actual_threads = n_threads - 1;
+      for (int i = 0; i < n_actual_threads; ++i)
+	{
+	  RandomIt end = first + elts_per_thread;
+	  auto task = [=] ()
+		      {
+			callback (first, end);
+		      };
+
+	  futures[i] = gdb::thread_pool::g_thread_pool->post_task (task);
+	  first = end;
+	}
+    }
+#endif /* CXX_STD_THREAD */
+
+  /* Process all the remaining elements in the main thread.  */
+  callback (first, last);
+
+#if CXX_STD_THREAD
+  for (int i = 0; i < n_actual_threads; ++i)
+    futures[i].wait ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* GDBSUPPORT_PARALLEL_FOR_H */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 3af6e2e..f9d1172 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -53,6 +53,11 @@
 #include "gdbsupport/symbol.h"
 #include <algorithm>
 #include "safe-ctype.h"
+#include "gdbsupport/parallel-for.h"
+
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* See minsyms.h.  */
 
@@ -1359,16 +1364,43 @@
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+#if CXX_STD_THREAD
+      /* Mutex that is used when modifying or accessing the demangled
+	 hash table.  */
+      std::mutex demangled_mutex;
+#endif
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
-      for (int i = 0; i < mcount; ++i)
-	{
-	  if (!msymbols[i].name_set)
-	    {
-	      symbol_set_names (&msymbols[i], msymbols[i].name,
-				false, m_objfile->per_bfd);
-	      msymbols[i].name_set = 1;
-	    }
-	}
+      gdb::parallel_for_each
+	(&msymbols[0], &msymbols[mcount],
+	 [&] (minimal_symbol *start, minimal_symbol *end)
+	 {
+	   for (minimal_symbol *msym = start; msym < end; ++msym)
+	     {
+	       if (!msym->name_set)
+		 {
+		   /* This will be freed later, by symbol_set_names.  */
+		   char *demangled_name
+		     = symbol_find_demangled_name (msym, msym->name);
+		   symbol_set_demangled_name
+		     (msym, demangled_name,
+		      &m_objfile->per_bfd->storage_obstack);
+		   msym->name_set = 1;
+		 }
+	     }
+	   {
+	     /* To limit how long we hold the lock, we only acquire it here
+	        and not while we demangle the names above.  */
+#if CXX_STD_THREAD
+	     std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+	     for (minimal_symbol *msym = start; msym < end; ++msym)
+	       {
+		 symbol_set_names (msym, msym->name, false,
+				   m_objfile->per_bfd);
+	       }
+	   }
+	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 060e676..3502827 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -776,13 +776,9 @@
      free_demangled_name_entry, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -883,8 +879,15 @@
       else
 	linkage_name_copy = linkage_name;
 
-      gdb::unique_xmalloc_ptr<char> demangled_name_ptr
-	(symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
+      /* The const_cast is safe because the only reason it is already
+         initialized is if we purposefully set it from a background
+         thread to avoid doing the work here.  However, it is still
+         allocated from the heap and needs to be freed by us, just
+         like if we called symbol_find_demangled_name here.  */
+      gdb::unique_xmalloc_ptr<char> demangled_name
+	(gsymbol->language_specific.demangled_name
+	 ? const_cast<char *> (gsymbol->language_specific.demangled_name)
+	 : symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
 
       /* Suppose we have demangled_name==NULL, copy_name==0, and
 	 linkage_name_copy==linkage_name.  In this case, we already have the
@@ -918,7 +921,7 @@
 	  new (*slot) demangled_name_entry
 	    (gdb::string_view (mangled_ptr, linkage_name.length ()));
 	}
-      (*slot)->demangled = std::move (demangled_name_ptr);
+      (*slot)->demangled = std::move (demangled_name);
       (*slot)->language = gsymbol->language;
     }
   else if (gsymbol->language == language_unknown
diff --git a/gdb/symtab.h b/gdb/symtab.h
index ede2600..4172a25 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -496,6 +496,16 @@
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must

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

[review v3] Demangle minsyms in parallel

[Tom Tromey (Code Review)]

Tom Tromey has posted comments on this change.

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


Patch Set 3:

(1 comment)

| --- /dev/null
| +++ gdb/gdbsupport/parallel-for.h
| @@ -1,0 +45,19 @@ parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
| +#if CXX_STD_THREAD
| +  /* So we can use a local array below.  */
| +  const size_t local_max = 16;
| +  size_t n_threads = std::min (thread_pool::g_thread_pool->count (),
| +			       local_max);
| +  size_t n_actual_threads = 0;
| +  std::future<void> futures[local_max];
| +
| +  size_t n_elements = last - first;
| +  if (n_threads > 1 && 2 * n_threads <= n_elements)

PS3, Line 54:

I think this check doesn't really make sense -- if one has many
elements but also many threads available, the answer shouldn't be
to give up.

In the next revision, I've changed it to reduce the number of threads
used when there aren't many elements per thread.  I picked an
arbitrary cutoff of a minimum of 10 elements.

| +    {
| +      size_t elts_per_thread = n_elements / n_threads;
| +      n_actual_threads = n_threads - 1;
| +      for (int i = 0; i < n_actual_threads; ++i)
| +	{
| +	  RandomIt end = first + elts_per_thread;
| +	  auto task = [=] ()
| +		      {
| +			callback (first, end);

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
Gerrit-Change-Number: 173
Gerrit-PatchSet: 3
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Fri, 22 Nov 2019 23:45:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review v4] Demangle minsyms in parallel

[Tom Tromey (Code Review)]

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

Demangle minsyms in parallel

This patch introduces a simple parallel for_each and changes the
minimal symbol reader to use it when computing the demangled name for
a minimal symbol.  This yields a speedup when reading minimal symbols.

2019-10-19  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* minsyms.c (minimal_symbol_reader::install): Use
	parallel_for_each.
	* gdbsupport/parallel-for.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.

Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/parallel-for.h
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
6 files changed, 159 insertions(+), 18 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7b004bc..7640945 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,14 @@
 2019-10-19  Christian Biesinger  <cbiesinger@google.com>
 	    Tom Tromey  <tom@tromey.com>
 
+	* minsyms.c (minimal_symbol_reader::install): Use
+	parallel_for_each.
+	* gdbsupport/parallel-for.h: New file.
+	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.
+
+2019-10-19  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
 	* gdbsupport/thread-pool.h: New file.
 	* gdbsupport/thread-pool.c: New file.
 	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index b07b11e..58f5f93 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1491,6 +1491,7 @@
 	gdbsupport/common-inferior.h \
 	gdbsupport/netstuff.h \
 	gdbsupport/host-defs.h \
+	gdbsupport/parallel-for.h \
 	gdbsupport/pathstuff.h \
 	gdbsupport/print-utils.h \
 	gdbsupport/ptid.h \
diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
new file mode 100644
index 0000000..ee91819
--- /dev/null
+++ b/gdb/gdbsupport/parallel-for.h
@@ -0,0 +1,87 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_PARALLEL_FOR_H
+#define GDBSUPPORT_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <system_error>
+#include <thread>
+#include "gdbsupport/thread-pool.h"
+#endif
+
+namespace gdb
+{
+
+/* A very simple "parallel for".  This splits the range of iterators
+   into subranges, and then passes each subrange to the callback.  The
+   work may or may not be done in separate threads.
+
+   This approach was chosen over having the callback work on single
+   items because it makes it simple for the caller to do
+   once-per-subrange initialization and destruction.  */
+
+template<class RandomIt, class RangeFunction>
+void
+parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
+{
+#if CXX_STD_THREAD
+  /* So we can use a local array below.  */
+  const size_t local_max = 16;
+  size_t n_threads = std::min (thread_pool::g_thread_pool->thread_count (),
+			       local_max);
+  size_t n_actual_threads = 0;
+  std::future<void> futures[local_max];
+
+  size_t n_elements = last - first;
+  if (n_threads > 1)
+    {
+      /* Arbitrarily require that there should be at least 10 elements
+	 in a thread.  */
+      if (n_elements / n_threads < 10)
+	n_threads = n_elements / 10;
+      size_t elts_per_thread = n_elements / n_threads;
+      n_actual_threads = n_threads - 1;
+      for (int i = 0; i < n_actual_threads; ++i)
+	{
+	  RandomIt end = first + elts_per_thread;
+	  auto task = [=] ()
+		      {
+			callback (first, end);
+		      };
+
+	  futures[i] = gdb::thread_pool::g_thread_pool->post_task (task);
+	  first = end;
+	}
+    }
+#endif /* CXX_STD_THREAD */
+
+  /* Process all the remaining elements in the main thread.  */
+  callback (first, last);
+
+#if CXX_STD_THREAD
+  for (int i = 0; i < n_actual_threads; ++i)
+    futures[i].wait ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* GDBSUPPORT_PARALLEL_FOR_H */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 6e7021a..03a1932 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -53,6 +53,11 @@
 #include "gdbsupport/symbol.h"
 #include <algorithm>
 #include "safe-ctype.h"
+#include "gdbsupport/parallel-for.h"
+
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* See minsyms.h.  */
 
@@ -1359,16 +1364,43 @@
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+#if CXX_STD_THREAD
+      /* Mutex that is used when modifying or accessing the demangled
+	 hash table.  */
+      std::mutex demangled_mutex;
+#endif
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
-      for (int i = 0; i < mcount; ++i)
-	{
-	  if (!msymbols[i].name_set)
-	    {
-	      symbol_set_names (&msymbols[i], msymbols[i].name,
-				false, m_objfile->per_bfd);
-	      msymbols[i].name_set = 1;
-	    }
-	}
+      gdb::parallel_for_each
+	(&msymbols[0], &msymbols[mcount],
+	 [&] (minimal_symbol *start, minimal_symbol *end)
+	 {
+	   for (minimal_symbol *msym = start; msym < end; ++msym)
+	     {
+	       if (!msym->name_set)
+		 {
+		   /* This will be freed later, by symbol_set_names.  */
+		   char *demangled_name
+		     = symbol_find_demangled_name (msym, msym->name);
+		   symbol_set_demangled_name
+		     (msym, demangled_name,
+		      &m_objfile->per_bfd->storage_obstack);
+		   msym->name_set = 1;
+		 }
+	     }
+	   {
+	     /* To limit how long we hold the lock, we only acquire it here
+	        and not while we demangle the names above.  */
+#if CXX_STD_THREAD
+	     std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+	     for (minimal_symbol *msym = start; msym < end; ++msym)
+	       {
+		 symbol_set_names (msym, msym->name, false,
+				   m_objfile->per_bfd);
+	       }
+	   }
+	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 2e8ae23..1eadd68 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -787,13 +787,9 @@
      free_demangled_name_entry, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -894,8 +890,15 @@
       else
 	linkage_name_copy = linkage_name;
 
-      gdb::unique_xmalloc_ptr<char> demangled_name_ptr
-	(symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
+      /* The const_cast is safe because the only reason it is already
+         initialized is if we purposefully set it from a background
+         thread to avoid doing the work here.  However, it is still
+         allocated from the heap and needs to be freed by us, just
+         like if we called symbol_find_demangled_name here.  */
+      gdb::unique_xmalloc_ptr<char> demangled_name
+	(gsymbol->language_specific.demangled_name
+	 ? const_cast<char *> (gsymbol->language_specific.demangled_name)
+	 : symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
 
       /* Suppose we have demangled_name==NULL, copy_name==0, and
 	 linkage_name_copy==linkage_name.  In this case, we already have the
@@ -929,7 +932,7 @@
 	  new (*slot) demangled_name_entry
 	    (gdb::string_view (mangled_ptr, linkage_name.length ()));
 	}
-      (*slot)->demangled = std::move (demangled_name_ptr);
+      (*slot)->demangled = std::move (demangled_name);
       (*slot)->language = gsymbol->language;
     }
   else if (gsymbol->language == language_unknown
diff --git a/gdb/symtab.h b/gdb/symtab.h
index bcbc9c8..9c2aea7 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -528,6 +528,16 @@
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must

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

[review v4] Demangle minsyms in parallel

[Pedro Alves (Code Review)]

Pedro Alves has posted comments on this change.

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


Patch Set 4:

(2 comments)

Sending these review comments I had on patchset 3.  I'll take a look at v4.

| --- /dev/null
| +++ gdb/gdbsupport/parallel-for.h
| @@ -1,0 +16,19 @@ /* Parallel for loops
| +
| +   You should have received a copy of the GNU General Public License
| +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
| +
| +#ifndef GDBSUPPORT_PARALLEL_FOR_H
| +#define GDBSUPPORT_PARALLEL_FOR_H
| +
| +#include <algorithm>
| +#if CXX_STD_THREAD
| +#include <system_error>

PS3, Line 25:

Is <system_error> necessary?

| +#include <thread>
| +#include "gdbsupport/thread-pool.h"
| +#endif
| +
| +namespace gdb
| +{
| +
| +/* A very simple "parallel for".  This splits the range of iterators
| +   into subranges, and then passes each subrange to the callback.  The

 ...

| @@ -1,0 +45,19 @@ parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
| +#if CXX_STD_THREAD
| +  /* So we can use a local array below.  */
| +  const size_t local_max = 16;
| +  size_t n_threads = std::min (thread_pool::g_thread_pool->count (),
| +			       local_max);
| +  size_t n_actual_threads = 0;
| +  std::future<void> futures[local_max];
| +
| +  size_t n_elements = last - first;
| +  if (n_threads > 1 && 2 * n_threads <= n_elements)

PS3, Line 54:

This "2 * n_threads <= n_elements" gave me pause.  I think it warrants
a comment.

(I see you've addressed this already.)

| +    {
| +      size_t elts_per_thread = n_elements / n_threads;
| +      n_actual_threads = n_threads - 1;
| +      for (int i = 0; i < n_actual_threads; ++i)
| +	{
| +	  RandomIt end = first + elts_per_thread;
| +	  auto task = [=] ()
| +		      {
| +			callback (first, end);

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
Gerrit-Change-Number: 173
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Tue, 26 Nov 2019 19:06:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review v4] Demangle minsyms in parallel

[Pedro Alves (Code Review)]

Pedro Alves has posted comments on this change.

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


Patch Set 4: Code-Review+2

(1 comment)

LGTM.  I'm just curious on the <system_error> include.

| --- /dev/null
| +++ gdb/gdbsupport/parallel-for.h
| @@ -1,0 +16,19 @@ /* Parallel for loops
| +
| +   You should have received a copy of the GNU General Public License
| +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
| +
| +#ifndef GDBSUPPORT_PARALLEL_FOR_H
| +#define GDBSUPPORT_PARALLEL_FOR_H
| +
| +#include <algorithm>
| +#if CXX_STD_THREAD
| +#include <system_error>

PS4, Line 25:

Same comment as v3, do we need this?

| +#include <thread>
| +#include "gdbsupport/thread-pool.h"
| +#endif
| +
| +namespace gdb
| +{
| +
| +/* A very simple "parallel for".  This splits the range of iterators
| +   into subranges, and then passes each subrange to the callback.  The

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
Gerrit-Change-Number: 173
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 26 Nov 2019 19:11:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[review v4] Demangle minsyms in parallel

[Tom Tromey (Code Review)]

Tom Tromey has posted comments on this change.

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


Patch Set 4:

(1 comment)

| --- /dev/null
| +++ gdb/gdbsupport/parallel-for.h
| @@ -1,0 +16,19 @@ /* Parallel for loops
| +
| +   You should have received a copy of the GNU General Public License
| +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
| +
| +#ifndef GDBSUPPORT_PARALLEL_FOR_H
| +#define GDBSUPPORT_PARALLEL_FOR_H
| +
| +#include <algorithm>
| +#if CXX_STD_THREAD
| +#include <system_error>

PS4, Line 25:

> Same comment as v3, do we need this?

Nope, I've removed it.

| +#include <thread>
| +#include "gdbsupport/thread-pool.h"
| +#endif
| +
| +namespace gdb
| +{
| +
| +/* A very simple "parallel for".  This splits the range of iterators
| +   into subranges, and then passes each subrange to the callback.  The

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
Gerrit-Change-Number: 173
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 26 Nov 2019 19:42:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: comment

[review v4] Demangle minsyms in parallel

[Tom Tromey (Code Review)]

Tom Tromey has posted comments on this change.

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


Patch Set 4:

(1 comment)

| --- /dev/null
| +++ gdb/gdbsupport/parallel-for.h
| @@ -1,0 +50,19 @@ #if CXX_STD_THREAD
| +  size_t n_actual_threads = 0;
| +  std::future<void> futures[local_max];
| +
| +  size_t n_elements = last - first;
| +  if (n_threads > 1)
| +    {
| +      /* Arbitrarily require that there should be at least 10 elements
| +	 in a thread.  */
| +      if (n_elements / n_threads < 10)
| +	n_threads = n_elements / 10;

PS4, Line 59:

This could yield zero, causing a SIGFPE.
I've fixed this now.

| +      size_t elts_per_thread = n_elements / n_threads;
| +      n_actual_threads = n_threads - 1;
| +      for (int i = 0; i < n_actual_threads; ++i)
| +	{
| +	  RandomIt end = first + elts_per_thread;
| +	  auto task = [=] ()
| +		      {
| +			callback (first, end);
| +		      };

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
Gerrit-Change-Number: 173
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 26 Nov 2019 20:50:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[pushed] Demangle minsyms in parallel

[Sourceware to Gerrit sync (Code Review)]

The original change was created by Tom Tromey.

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

Demangle minsyms in parallel

This patch introduces a simple parallel for_each and changes the
minimal symbol reader to use it when computing the demangled name for
a minimal symbol.  This yields a speedup when reading minimal symbols.

2019-11-26  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* minsyms.c (minimal_symbol_reader::install): Use
	parallel_for_each.
	* gdbsupport/parallel-for.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.

Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/parallel-for.h
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
6 files changed, 158 insertions(+), 18 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7c4e2b8..b73ae8d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,14 @@
 2019-11-26  Christian Biesinger  <cbiesinger@google.com>
 	    Tom Tromey  <tom@tromey.com>
 
+	* minsyms.c (minimal_symbol_reader::install): Use
+	parallel_for_each.
+	* gdbsupport/parallel-for.h: New file.
+	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.
+
+2019-11-26  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
 	* gdbsupport/thread-pool.h: New file.
 	* gdbsupport/thread-pool.c: New file.
 	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index b07b11e..58f5f93 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1491,6 +1491,7 @@
 	gdbsupport/common-inferior.h \
 	gdbsupport/netstuff.h \
 	gdbsupport/host-defs.h \
+	gdbsupport/parallel-for.h \
 	gdbsupport/pathstuff.h \
 	gdbsupport/print-utils.h \
 	gdbsupport/ptid.h \
diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
new file mode 100644
index 0000000..70fdacd
--- /dev/null
+++ b/gdb/gdbsupport/parallel-for.h
@@ -0,0 +1,86 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_PARALLEL_FOR_H
+#define GDBSUPPORT_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <thread>
+#include "gdbsupport/thread-pool.h"
+#endif
+
+namespace gdb
+{
+
+/* A very simple "parallel for".  This splits the range of iterators
+   into subranges, and then passes each subrange to the callback.  The
+   work may or may not be done in separate threads.
+
+   This approach was chosen over having the callback work on single
+   items because it makes it simple for the caller to do
+   once-per-subrange initialization and destruction.  */
+
+template<class RandomIt, class RangeFunction>
+void
+parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
+{
+#if CXX_STD_THREAD
+  /* So we can use a local array below.  */
+  const size_t local_max = 16;
+  size_t n_threads = std::min (thread_pool::g_thread_pool->thread_count (),
+			       local_max);
+  size_t n_actual_threads = 0;
+  std::future<void> futures[local_max];
+
+  size_t n_elements = last - first;
+  if (n_threads > 1)
+    {
+      /* Arbitrarily require that there should be at least 10 elements
+	 in a thread.  */
+      if (n_elements / n_threads < 10)
+	n_threads = std::max (n_elements / 10, (size_t) 1);
+      size_t elts_per_thread = n_elements / n_threads;
+      n_actual_threads = n_threads - 1;
+      for (int i = 0; i < n_actual_threads; ++i)
+	{
+	  RandomIt end = first + elts_per_thread;
+	  auto task = [=] ()
+		      {
+			callback (first, end);
+		      };
+
+	  futures[i] = gdb::thread_pool::g_thread_pool->post_task (task);
+	  first = end;
+	}
+    }
+#endif /* CXX_STD_THREAD */
+
+  /* Process all the remaining elements in the main thread.  */
+  callback (first, last);
+
+#if CXX_STD_THREAD
+  for (int i = 0; i < n_actual_threads; ++i)
+    futures[i].wait ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* GDBSUPPORT_PARALLEL_FOR_H */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 6e7021a..03a1932 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -53,6 +53,11 @@
 #include "gdbsupport/symbol.h"
 #include <algorithm>
 #include "safe-ctype.h"
+#include "gdbsupport/parallel-for.h"
+
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* See minsyms.h.  */
 
@@ -1359,16 +1364,43 @@
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+#if CXX_STD_THREAD
+      /* Mutex that is used when modifying or accessing the demangled
+	 hash table.  */
+      std::mutex demangled_mutex;
+#endif
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
-      for (int i = 0; i < mcount; ++i)
-	{
-	  if (!msymbols[i].name_set)
-	    {
-	      symbol_set_names (&msymbols[i], msymbols[i].name,
-				false, m_objfile->per_bfd);
-	      msymbols[i].name_set = 1;
-	    }
-	}
+      gdb::parallel_for_each
+	(&msymbols[0], &msymbols[mcount],
+	 [&] (minimal_symbol *start, minimal_symbol *end)
+	 {
+	   for (minimal_symbol *msym = start; msym < end; ++msym)
+	     {
+	       if (!msym->name_set)
+		 {
+		   /* This will be freed later, by symbol_set_names.  */
+		   char *demangled_name
+		     = symbol_find_demangled_name (msym, msym->name);
+		   symbol_set_demangled_name
+		     (msym, demangled_name,
+		      &m_objfile->per_bfd->storage_obstack);
+		   msym->name_set = 1;
+		 }
+	     }
+	   {
+	     /* To limit how long we hold the lock, we only acquire it here
+	        and not while we demangle the names above.  */
+#if CXX_STD_THREAD
+	     std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+	     for (minimal_symbol *msym = start; msym < end; ++msym)
+	       {
+		 symbol_set_names (msym, msym->name, false,
+				   m_objfile->per_bfd);
+	       }
+	   }
+	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 711f8ef..b5c8109 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -787,13 +787,9 @@
      free_demangled_name_entry, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -894,8 +890,15 @@
       else
 	linkage_name_copy = linkage_name;
 
-      gdb::unique_xmalloc_ptr<char> demangled_name_ptr
-	(symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
+      /* The const_cast is safe because the only reason it is already
+         initialized is if we purposefully set it from a background
+         thread to avoid doing the work here.  However, it is still
+         allocated from the heap and needs to be freed by us, just
+         like if we called symbol_find_demangled_name here.  */
+      gdb::unique_xmalloc_ptr<char> demangled_name
+	(gsymbol->language_specific.demangled_name
+	 ? const_cast<char *> (gsymbol->language_specific.demangled_name)
+	 : symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
 
       /* Suppose we have demangled_name==NULL, copy_name==0, and
 	 linkage_name_copy==linkage_name.  In this case, we already have the
@@ -929,7 +932,7 @@
 	  new (*slot) demangled_name_entry
 	    (gdb::string_view (mangled_ptr, linkage_name.length ()));
 	}
-      (*slot)->demangled = std::move (demangled_name_ptr);
+      (*slot)->demangled = std::move (demangled_name);
       (*slot)->language = gsymbol->language;
     }
   else if (gsymbol->language == language_unknown
diff --git a/gdb/symtab.h b/gdb/symtab.h
index bcbc9c8..9c2aea7 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -528,6 +528,16 @@
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
Gerrit-Change-Number: 173
Gerrit-PatchSet: 5
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

[pushed] Demangle minsyms in parallel

[Sourceware to Gerrit sync (Code Review)]

Sourceware to Gerrit sync has submitted this change.

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

Demangle minsyms in parallel

This patch introduces a simple parallel for_each and changes the
minimal symbol reader to use it when computing the demangled name for
a minimal symbol.  This yields a speedup when reading minimal symbols.

2019-11-26  Christian Biesinger  <cbiesinger@google.com>
	    Tom Tromey  <tom@tromey.com>

	* minsyms.c (minimal_symbol_reader::install): Use
	parallel_for_each.
	* gdbsupport/parallel-for.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.

Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
---
M gdb/ChangeLog
M gdb/Makefile.in
A gdb/gdbsupport/parallel-for.h
M gdb/minsyms.c
M gdb/symtab.c
M gdb/symtab.h
6 files changed, 158 insertions(+), 18 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7c4e2b8..b73ae8d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,14 @@
 2019-11-26  Christian Biesinger  <cbiesinger@google.com>
 	    Tom Tromey  <tom@tromey.com>
 
+	* minsyms.c (minimal_symbol_reader::install): Use
+	parallel_for_each.
+	* gdbsupport/parallel-for.h: New file.
+	* Makefile.in (HFILES_NO_SRCDIR): Add gdbsupport/parallel-for.h.
+
+2019-11-26  Christian Biesinger  <cbiesinger@google.com>
+	    Tom Tromey  <tom@tromey.com>
+
 	* gdbsupport/thread-pool.h: New file.
 	* gdbsupport/thread-pool.c: New file.
 	* Makefile.in (COMMON_SFILES): Add thread-pool.c.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index b07b11e..58f5f93 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1491,6 +1491,7 @@
 	gdbsupport/common-inferior.h \
 	gdbsupport/netstuff.h \
 	gdbsupport/host-defs.h \
+	gdbsupport/parallel-for.h \
 	gdbsupport/pathstuff.h \
 	gdbsupport/print-utils.h \
 	gdbsupport/ptid.h \
diff --git a/gdb/gdbsupport/parallel-for.h b/gdb/gdbsupport/parallel-for.h
new file mode 100644
index 0000000..70fdacd
--- /dev/null
+++ b/gdb/gdbsupport/parallel-for.h
@@ -0,0 +1,86 @@
+/* Parallel for loops
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_PARALLEL_FOR_H
+#define GDBSUPPORT_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <thread>
+#include "gdbsupport/thread-pool.h"
+#endif
+
+namespace gdb
+{
+
+/* A very simple "parallel for".  This splits the range of iterators
+   into subranges, and then passes each subrange to the callback.  The
+   work may or may not be done in separate threads.
+
+   This approach was chosen over having the callback work on single
+   items because it makes it simple for the caller to do
+   once-per-subrange initialization and destruction.  */
+
+template<class RandomIt, class RangeFunction>
+void
+parallel_for_each (RandomIt first, RandomIt last, RangeFunction callback)
+{
+#if CXX_STD_THREAD
+  /* So we can use a local array below.  */
+  const size_t local_max = 16;
+  size_t n_threads = std::min (thread_pool::g_thread_pool->thread_count (),
+			       local_max);
+  size_t n_actual_threads = 0;
+  std::future<void> futures[local_max];
+
+  size_t n_elements = last - first;
+  if (n_threads > 1)
+    {
+      /* Arbitrarily require that there should be at least 10 elements
+	 in a thread.  */
+      if (n_elements / n_threads < 10)
+	n_threads = std::max (n_elements / 10, (size_t) 1);
+      size_t elts_per_thread = n_elements / n_threads;
+      n_actual_threads = n_threads - 1;
+      for (int i = 0; i < n_actual_threads; ++i)
+	{
+	  RandomIt end = first + elts_per_thread;
+	  auto task = [=] ()
+		      {
+			callback (first, end);
+		      };
+
+	  futures[i] = gdb::thread_pool::g_thread_pool->post_task (task);
+	  first = end;
+	}
+    }
+#endif /* CXX_STD_THREAD */
+
+  /* Process all the remaining elements in the main thread.  */
+  callback (first, last);
+
+#if CXX_STD_THREAD
+  for (int i = 0; i < n_actual_threads; ++i)
+    futures[i].wait ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* GDBSUPPORT_PARALLEL_FOR_H */
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 6e7021a..03a1932 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -53,6 +53,11 @@
 #include "gdbsupport/symbol.h"
 #include <algorithm>
 #include "safe-ctype.h"
+#include "gdbsupport/parallel-for.h"
+
+#if CXX_STD_THREAD
+#include <mutex>
+#endif
 
 /* See minsyms.h.  */
 
@@ -1359,16 +1364,43 @@
       m_objfile->per_bfd->minimal_symbol_count = mcount;
       m_objfile->per_bfd->msymbols = std::move (msym_holder);
 
+#if CXX_STD_THREAD
+      /* Mutex that is used when modifying or accessing the demangled
+	 hash table.  */
+      std::mutex demangled_mutex;
+#endif
+
       msymbols = m_objfile->per_bfd->msymbols.get ();
-      for (int i = 0; i < mcount; ++i)
-	{
-	  if (!msymbols[i].name_set)
-	    {
-	      symbol_set_names (&msymbols[i], msymbols[i].name,
-				false, m_objfile->per_bfd);
-	      msymbols[i].name_set = 1;
-	    }
-	}
+      gdb::parallel_for_each
+	(&msymbols[0], &msymbols[mcount],
+	 [&] (minimal_symbol *start, minimal_symbol *end)
+	 {
+	   for (minimal_symbol *msym = start; msym < end; ++msym)
+	     {
+	       if (!msym->name_set)
+		 {
+		   /* This will be freed later, by symbol_set_names.  */
+		   char *demangled_name
+		     = symbol_find_demangled_name (msym, msym->name);
+		   symbol_set_demangled_name
+		     (msym, demangled_name,
+		      &m_objfile->per_bfd->storage_obstack);
+		   msym->name_set = 1;
+		 }
+	     }
+	   {
+	     /* To limit how long we hold the lock, we only acquire it here
+	        and not while we demangle the names above.  */
+#if CXX_STD_THREAD
+	     std::lock_guard<std::mutex> guard (demangled_mutex);
+#endif
+	     for (minimal_symbol *msym = start; msym < end; ++msym)
+	       {
+		 symbol_set_names (msym, msym->name, false,
+				   m_objfile->per_bfd);
+	       }
+	   }
+	 });
 
       build_minimal_symbol_hash_tables (m_objfile);
     }
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 711f8ef..b5c8109 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -787,13 +787,9 @@
      free_demangled_name_entry, xcalloc, xfree));
 }
 
-/* Try to determine the demangled name for a symbol, based on the
-   language of that symbol.  If the language is set to language_auto,
-   it will attempt to find any demangling algorithm that works and
-   then set the language appropriately.  The returned name is allocated
-   by the demangler and should be xfree'd.  */
+/* See symtab.h  */
 
-static char *
+char *
 symbol_find_demangled_name (struct general_symbol_info *gsymbol,
 			    const char *mangled)
 {
@@ -894,8 +890,15 @@
       else
 	linkage_name_copy = linkage_name;
 
-      gdb::unique_xmalloc_ptr<char> demangled_name_ptr
-	(symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
+      /* The const_cast is safe because the only reason it is already
+         initialized is if we purposefully set it from a background
+         thread to avoid doing the work here.  However, it is still
+         allocated from the heap and needs to be freed by us, just
+         like if we called symbol_find_demangled_name here.  */
+      gdb::unique_xmalloc_ptr<char> demangled_name
+	(gsymbol->language_specific.demangled_name
+	 ? const_cast<char *> (gsymbol->language_specific.demangled_name)
+	 : symbol_find_demangled_name (gsymbol, linkage_name_copy.data ()));
 
       /* Suppose we have demangled_name==NULL, copy_name==0, and
 	 linkage_name_copy==linkage_name.  In this case, we already have the
@@ -929,7 +932,7 @@
 	  new (*slot) demangled_name_entry
 	    (gdb::string_view (mangled_ptr, linkage_name.length ()));
 	}
-      (*slot)->demangled = std::move (demangled_name_ptr);
+      (*slot)->demangled = std::move (demangled_name);
       (*slot)->language = gsymbol->language;
     }
   else if (gsymbol->language == language_unknown
diff --git a/gdb/symtab.h b/gdb/symtab.h
index bcbc9c8..9c2aea7 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -528,6 +528,16 @@
                                  enum language language,
 				 struct obstack *obstack);
 
+
+/* Try to determine the demangled name for a symbol, based on the
+   language of that symbol.  If the language is set to language_auto,
+   it will attempt to find any demangling algorithm that works and
+   then set the language appropriately.  The returned name is allocated
+   by the demangler and should be xfree'd.  */
+
+extern char *symbol_find_demangled_name (struct general_symbol_info *gsymbol,
+					 const char *mangled);
+
 /* Set just the linkage name of a symbol; do not try to demangle
    it.  Used for constructs which do not have a mangled name,
    e.g. struct tags.  Unlike SYMBOL_SET_NAMES, linkage_name must

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I220341f70e94dd02df5dd424272c50a5afb64978
Gerrit-Change-Number: 173
Gerrit-PatchSet: 5
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged