[review] Add maint set/show worker-threads
Commit Message
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/174
......................................................................
Add maint set/show worker-threads
This adds maint commands to control the number of worker threads that
gdb can use.
gdb/ChangeLog
2019-10-19 Tom Tromey <tom@tromey.com>
* NEWS: Add entry.
* maint.c (_initialize_maint_cmds): Add "worker-threads" maint
commands. Call update_thread_pool_size.
(update_thread_pool_size, maintenance_set_worker_threads): New
functions.
(n_worker_threads): New global.
gdb/doc/ChangeLog
2019-10-19 Tom Tromey <tom@tromey.com>
* gdb.texinfo (Maintenance Commands): Document new maint
commands.
Change-Id: I4fb514faa05879d8afe62c77036a4469d57dca2a
---
M gdb/ChangeLog
M gdb/NEWS
M gdb/doc/ChangeLog
M gdb/doc/gdb.texinfo
M gdb/maint.c
5 files changed, 84 insertions(+), 0 deletions(-)
Comments
> Date: Sat, 19 Oct 2019 23:55:13 -0400
> From: "Tom Tromey (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
>
> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/174
> ......................................................................
>
> Add maint set/show worker-threads
>
> This adds maint commands to control the number of worker threads that
> gdb can use.
>
> gdb/ChangeLog
> 2019-10-19 Tom Tromey <tom@tromey.com>
>
> * NEWS: Add entry.
> * maint.c (_initialize_maint_cmds): Add "worker-threads" maint
> commands. Call update_thread_pool_size.
> (update_thread_pool_size, maintenance_set_worker_threads): New
> functions.
> (n_worker_threads): New global.
>
> gdb/doc/ChangeLog
> 2019-10-19 Tom Tromey <tom@tromey.com>
>
> * gdb.texinfo (Maintenance Commands): Document new maint
> commands.
I think this was already reviewed on the list, and I approved the
documentation changes at that time (this post doesn't say it is a new
version of the patch, so maybe I'm missing something). But in any
case, you have my approval for the documentation parts.
Thanks.
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/174
......................................................................
Patch Set 3:
(1 comment)
| --- gdb/maint.c
| +++ gdb/maint.c
| @@ -1311,5 +1346,18 @@ When enabled GDB is profiled."),
| maintenance_set_profile_cmd,
| show_maintenance_profile_p,
| &maintenance_set_cmdlist,
| &maintenance_show_cmdlist);
| +
| + add_setshow_zuinteger_unlimited_cmd ("worker-threads",
| + class_maintenance,
| + &n_worker_threads, _("\
| +Set the number of worker threads GDB can use."), _("\
| +Set the number of worker threads GDB can use."), _("\
PS3, Line 1355:
Should this be "Show"? I think this is the cause for the missing T
here:
(gdb) maint show worker-threads
He number of worker threads GDB can use is unlimited.
| +GDB may use multiple threads to speed up certain CPU-intensive operations,\n\
| +such as demangling symbol names."),
| + maintenance_set_worker_threads, NULL,
| + &maintenance_set_cmdlist,
| + &maintenance_show_cmdlist);
| +
| + update_thread_pool_size ();
| }
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/174
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
Minor comment below, but otherwise LGTM.
| --- gdb/maint.c
| +++ gdb/maint.c
| @@ -843,0 +854,19 @@ update_thread_pool_size ()
| +#if CXX_STD_THREAD
| + int n_threads = n_worker_threads;
| +
| + if (n_threads < 0)
| + {
| + n_threads = std::thread::hardware_concurrency ();
| + if (n_threads == 0)
| + {
| + /* Meh. */
| + n_threads = 2;
PS4, Line 863:
If we have no concurrency, then shouldn't we fallback to no extra
threads? Presumably the threads in the poll will be running CPU-bound
code, so more threads in the same hardware core/thread won't help,
right?
BTW, JIC, did you check if setting the number of threads to 0 behaves
OK?
| + }
| + }
| +
| + gdb::thread_pool::g_thread_pool->set_thread_count (n_threads);
| +#endif
| +}
| +
| +static void
| +maintenance_set_worker_threads (const char *args, int from_tty,
Tom Tromey has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/174
......................................................................
Patch Set 4:
(1 comment)
| --- gdb/maint.c
| +++ gdb/maint.c
| @@ -843,0 +854,19 @@ update_thread_pool_size ()
| +#if CXX_STD_THREAD
| + int n_threads = n_worker_threads;
| +
| + if (n_threads < 0)
| + {
| + n_threads = std::thread::hardware_concurrency ();
| + if (n_threads == 0)
| + {
| + /* Meh. */
| + n_threads = 2;
PS4, Line 863:
> If we have no concurrency, then shouldn't we fallback to no extra threads? Presumably the threads in the poll will be running CPU-bound code, so more threads in the same hardware core/thread won't help, right?
I changed this to let it just be 0.
> BTW, JIC, did you check if setting the number of threads to 0 behaves OK?
Yep.
| + }
| + }
| +
| + gdb::thread_pool::g_thread_pool->set_thread_count (n_threads);
| +#endif
| +}
| +
| +static void
| +maintenance_set_worker_threads (const char *args, int from_tty,
@@ -1,3 +1,12 @@
+2019-10-19 Tom Tromey <tom@tromey.com>
+
+ * NEWS: Add entry.
+ * maint.c (_initialize_maint_cmds): Add "worker-threads" maint
+ commands. Call update_thread_pool_size.
+ (update_thread_pool_size, maintenance_set_worker_threads): New
+ functions.
+ (n_worker_threads): New global.
+
2019-10-19 Christian Biesinger <cbiesinger@google.com>
Tom Tromey <tom@tromey.com>
@@ -134,6 +134,13 @@
set style highlight intensity VALUE
Control the styling of highlightings.
+maint set worker-threads
+maint show worker-threads
+ Control the number of worker threads that can be used by GDB. The
+ default is "unlimited", which lets GDB choose a number that is
+ reasonable. Currently worker threads are only used when demangling
+ the names of linker symbols.
+
maint set test-settings KIND
maint show test-settings KIND
A set of commands used by the testsuite for exercising the settings
@@ -1,3 +1,8 @@
+2019-10-19 Tom Tromey <tom@tromey.com>
+
+ * gdb.texinfo (Maintenance Commands): Document new maint
+ commands.
+
2019-10-07 Tom de Vries <tdevries@suse.de>
* gdb.texinfo: Fix typo.
@@ -37683,6 +37683,21 @@
If DWARF frame unwinders are not supported for a particular target
architecture, then enabling this flag does not cause them to be used.
+
+@kindex maint set worker-threads
+@kindex maint show worker-threads
+@item maint set worker-threads
+@item maint show worker-threads
+Control the number of worker threads that may be used by @value{GDBN}.
+On capable hosts, @value{GDBN} may use multiple threads to speed up
+certain CPU-intensive operations, such as demangling symbol names.
+While the number of threads used by @value{GDBN} may vary, this
+command can be used to set an upper bound on this number. The default
+is @code{unlimited}, which lets @value{GDBN} choose a reasonable
+number. Note that this only controls worker threads started by
+@value{GDBN} itself; libraries used by @value{GDBN} may start threads
+of their own.
+
@kindex maint set profile
@kindex maint show profile
@cindex profiling GDB
@@ -46,6 +46,10 @@
#include "cli/cli-setshow.h"
#include "cli/cli-cmds.h"
+#if CXX_STD_THREAD
+#include "gdbsupport/thread-pool.h"
+#endif
+
static void maintenance_do_deprecate (const char *, int);
/* Access the maintenance subcommands. */
@@ -840,6 +844,37 @@
error (_("Profiling support is not available on this system."));
}
#endif
+
+static int n_worker_threads = -1;
+
+/* Update the thread pool for the desired number of threads. */
+static void
+update_thread_pool_size ()
+{
+#if CXX_STD_THREAD
+ int n_threads = n_worker_threads;
+
+ if (n_threads < 0)
+ {
+ n_threads = std::thread::hardware_concurrency ();
+ if (n_threads == 0)
+ {
+ /* Meh. */
+ n_threads = 2;
+ }
+ }
+
+ gdb::thread_pool::g_thread_pool.set_thread_count (n_threads);
+#endif
+}
+
+static void
+maintenance_set_worker_threads (const char *args, int from_tty,
+ struct cmd_list_element *c)
+{
+ update_thread_pool_size ();
+}
+
/* If true, display time usage both at startup and for each command. */
@@ -1312,4 +1347,17 @@
show_maintenance_profile_p,
&maintenance_set_cmdlist,
&maintenance_show_cmdlist);
+
+ add_setshow_zuinteger_unlimited_cmd ("worker-threads",
+ class_maintenance,
+ &n_worker_threads, _("\
+Set the number of worker threads GDB can use."), _("\
+Set the number of worker threads GDB can use."), _("\
+GDB may use multiple threads to speed up certain CPU-intensive operations,\n\
+such as demangling symbol names."),
+ maintenance_set_worker_threads, NULL,
+ &maintenance_set_cmdlist,
+ &maintenance_show_cmdlist);
+
+ update_thread_pool_size ();
}