[v3,7/8] Demangle minsyms in parallel
Commit Message
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-05-29 Tom Tromey <tom@tromey.com>
* minsyms.c (minimal_symbol_reader::install): Use
parallel_for_each.
* common/parallel-for.h: New file.
* common/parallel-for.c: New file.
* Makefile.in (HFILES_NO_SRCDIR): Add common/parallel-for.h.
(COMMON_SFILES): Add common/parallel-for.c.
---
gdb/ChangeLog | 9 +++++
gdb/Makefile.in | 2 +
gdb/common/parallel-for.c | 27 +++++++++++++
gdb/common/parallel-for.h | 82 +++++++++++++++++++++++++++++++++++++++
gdb/minsyms.c | 23 ++++++-----
5 files changed, 133 insertions(+), 10 deletions(-)
create mode 100644 gdb/common/parallel-for.c
create mode 100644 gdb/common/parallel-for.h
Comments
On 5/29/19 10:29 PM, Tom Tromey wrote:
> 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-05-29 Tom Tromey <tom@tromey.com>
>
> * minsyms.c (minimal_symbol_reader::install): Use
> parallel_for_each.
> * common/parallel-for.h: New file.
> * common/parallel-for.c: New file.
> * Makefile.in (HFILES_NO_SRCDIR): Add common/parallel-for.h.
> (COMMON_SFILES): Add common/parallel-for.c.
> ---
> gdb/ChangeLog | 9 +++++
> gdb/Makefile.in | 2 +
> gdb/common/parallel-for.c | 27 +++++++++++++
> gdb/common/parallel-for.h | 82 +++++++++++++++++++++++++++++++++++++++
> gdb/minsyms.c | 23 ++++++-----
> 5 files changed, 133 insertions(+), 10 deletions(-)
> create mode 100644 gdb/common/parallel-for.c
> create mode 100644 gdb/common/parallel-for.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 2dd69f3f0ba..15c7a6e2536 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -971,6 +971,7 @@ COMMON_SFILES = \
> common/gdb_vecs.c \
> common/netstuff.c \
> common/new-op.c \
> + common/parallel-for.c \
> common/pathstuff.c \
> common/print-utils.c \
> common/ptid.c \
> @@ -1471,6 +1472,7 @@ HFILES_NO_SRCDIR = \
> common/common-inferior.h \
> common/netstuff.h \
> common/host-defs.h \
> + common/parallel-for.h \
> common/pathstuff.h \
> common/print-utils.h \
> common/ptid.h \
> diff --git a/gdb/common/parallel-for.c b/gdb/common/parallel-for.c
> new file mode 100644
> index 00000000000..0970cea882b
> --- /dev/null
> +++ b/gdb/common/parallel-for.c
> @@ -0,0 +1,27 @@
> +/* 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/>. */
> +
> +#include "common/common-defs.h"
> +#include "common/parallel-for.h"
> +
> +namespace gdb
> +{
> +/* See parallel-for.h. */
> +int max_threads = -1;
> +}
> diff --git a/gdb/common/parallel-for.h b/gdb/common/parallel-for.h
> new file mode 100644
> index 00000000000..6770f39a05f
> --- /dev/null
> +++ b/gdb/common/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 COMMON_PARALLEL_FOR_H
> +#define COMMON_PARALLEL_FOR_H
> +
> +#include <algorithm>
> +#if CXX_STD_THREAD
> +#include <thread>
> +#endif
> +
> +namespace gdb
> +{
> +
> +/* True if threading should be enabled. */
> +
> +extern int max_threads;
> +
> +/* A very simple "parallel for". This iterates over the elements
> + given by the range of iterators, which must be random access
> + iterators. For each element, it calls the callback function. The
> + work may or may not be done by separate threads. */
> +
> +template<class RandomIt, class UnaryFunction>
> +void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f)
> +{
> +#if CXX_STD_THREAD
> + int n_threads = std::thread::hardware_concurrency ();
> + /* So we can use a local array below. */
> + const int local_max = 16;
"16 cores ought to be enough for anybody", right? :-)
Just kidding. Longer term I could see this evolving into
pulling threads out of a worker thread pool shared by all
kinds of parallel_for_each calls in the tree, but I'm super
fine with the simple initial design.
LGTM.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> "16 cores ought to be enough for anybody", right? :-)
As long as it is more than what's on my personal desktop :)
Pedro> Just kidding. Longer term I could see this evolving into
Pedro> pulling threads out of a worker thread pool shared by all
Pedro> kinds of parallel_for_each calls in the tree, but I'm super
Pedro> fine with the simple initial design.
The thread pool approach is nice because it means there isn't thread
startup overhead -- just whatever overhead there is from pushing jobs
onto a queue.
On the down side, a thread pool is somewhat more complicated.
Still, if we want it eventually, maybe we should just start with it.
One possible bad part is that if we don't have std::thread then I wonder
if the other things like futures will be available.
Tom
@@ -971,6 +971,7 @@ COMMON_SFILES = \
common/gdb_vecs.c \
common/netstuff.c \
common/new-op.c \
+ common/parallel-for.c \
common/pathstuff.c \
common/print-utils.c \
common/ptid.c \
@@ -1471,6 +1472,7 @@ HFILES_NO_SRCDIR = \
common/common-inferior.h \
common/netstuff.h \
common/host-defs.h \
+ common/parallel-for.h \
common/pathstuff.h \
common/print-utils.h \
common/ptid.h \
new file mode 100644
@@ -0,0 +1,27 @@
+/* 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/>. */
+
+#include "common/common-defs.h"
+#include "common/parallel-for.h"
+
+namespace gdb
+{
+/* See parallel-for.h. */
+int max_threads = -1;
+}
new file mode 100644
@@ -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 COMMON_PARALLEL_FOR_H
+#define COMMON_PARALLEL_FOR_H
+
+#include <algorithm>
+#if CXX_STD_THREAD
+#include <thread>
+#endif
+
+namespace gdb
+{
+
+/* True if threading should be enabled. */
+
+extern int max_threads;
+
+/* A very simple "parallel for". This iterates over the elements
+ given by the range of iterators, which must be random access
+ iterators. For each element, it calls the callback function. The
+ work may or may not be done by separate threads. */
+
+template<class RandomIt, class UnaryFunction>
+void parallel_for_each (RandomIt first, RandomIt last, UnaryFunction f)
+{
+#if CXX_STD_THREAD
+ int n_threads = std::thread::hardware_concurrency ();
+ /* So we can use a local array below. */
+ const int local_max = 16;
+ /* Be sure to handle the "unlimited" case. */
+ if (max_threads >= 0 && n_threads > max_threads)
+ n_threads = max_threads;
+ if (n_threads > local_max)
+ n_threads = local_max;
+
+ if (n_threads <= 1 || last - first < 2 * n_threads)
+#endif /* CXX_STD_THREAD */
+ {
+ /* Don't bother. */
+ std::for_each (first, last, f);
+ return;
+ }
+
+#if CXX_STD_THREAD
+ auto body = [&] (RandomIt start)
+ {
+ for (; start < last; start += n_threads)
+ f (*start);
+ };
+
+ std::thread threads[local_max];
+ for (int i = 0; i < n_threads; ++i)
+ {
+ threads[i] = std::thread (body, first);
+ ++first;
+ }
+
+ for (int i = 0; i < n_threads; ++i)
+ threads[i].join ();
+#endif /* CXX_STD_THREAD */
+}
+
+}
+
+#endif /* COMMON_PARALLEL_FOR_H */
@@ -53,6 +53,7 @@
#include "common/symbol.h"
#include <algorithm>
#include "safe-ctype.h"
+#include "common/parallel-for.h"
/* See minsyms.h. */
@@ -1372,16 +1373,18 @@ minimal_symbol_reader::install ()
m_objfile->per_bfd->msymbols = std::move (msym_holder);
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 &msym)
+ {
+ if (!msym.name_set)
+ {
+ symbol_set_names (&msym, msym.name,
+ strlen (msym.name), 0,
+ m_objfile->per_bfd);
+ msym.name_set = 1;
+ }
+ });
build_minimal_symbol_hash_tables (m_objfile);
}