[review] Add maint set/show worker-threads

  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(-)
  

> 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,