Re: [PATCH v3 7/8] Demangle minsyms in parallel

[Pedro Alves <palves@redhat.com>]

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