[v3] gdb: Enable early init of thread pool size
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
fail
|
Testing failed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
fail
|
Testing failed
|
Commit Message
This commit enables the early initialization commands (92e4e97a9f5) to
modify the number of threads used by gdb's thread pool.
The motivation here is to prevent gdb from spawning a detrimental number
of threads on many-core systems under environments with restrictive
ulimits.
With gdb before this commit, the thread pool takes the following sizes:
1. Thread pool size is initialized to 0.
2. After the maintenance commands are defined, the thread pool size is
set to the number of system cores (if it has not already been set).
3. Using early initialization commands, the thread pool size can be
changed using "maint set worker-threads".
4. After the first prompt, the thread pool size can be changed as in the
previous step.
Therefore after step 2. gdb has potentially launched hundreds of threads
on a many-core system.
After this change, step 2 and 3 are reversed so there is an opportunity
to set the required number of threads without needing to default to the
number of system cores first.
There does exist a configure option (added in 261b07488b9) to disable
multithreading, but this does not allow for an already deployed gdb to
be configured.
GDB built with GCC 13.
No test suite regressions detected. Compilers: GCC, ACfL, Intel, Intel
LLVM, NVHPC; Platforms: x86_64, aarch64.
The scenario that interests me the most involves preventing GDB from
spawning any worker threads at all. This was tested by counting the
number of clones observed by strace:
strace -e clone,clone3 gdb/gdb -q \
--early-init-eval-command="maint set worker-threads 0" \
-ex q ./gdb/gdb |& grep --count clone
The new test relies on "gdb: install CLI uiout while processing early
init files" developed by Andrew Burgess. This patch will need pushing
prior to this change.
---
gdb/main.c | 4 +++
gdb/maint.c | 7 ++--
gdb/maint.h | 4 +++
gdb/testsuite/gdb.base/early-init-file.exp | 40 ++++++++++++++++++++++
gdbsupport/thread-pool.cc | 4 +++
gdbsupport/thread-pool.h | 1 +
6 files changed, 56 insertions(+), 4 deletions(-)
Comments
>>>>> "Richard" == Richard Bunt <richard.bunt@linaro.org> writes:
Richard> This commit enables the early initialization commands (92e4e97a9f5) to
Richard> modify the number of threads used by gdb's thread pool.
Richard> The motivation here is to prevent gdb from spawning a detrimental number
Richard> of threads on many-core systems under environments with restrictive
Richard> ulimits.
Thank you. I think this is ok.
Approved-By: Tom Tromey <tom@tromey.com>
Richard> Therefore after step 2. gdb has potentially launched hundreds of threads
Richard> on a many-core system.
I wonder if we should throttle the default size. In my testing, I
didn't see any scaling benefits past n=8 or maybe even n=5:
https://sourceware.org/bugzilla/show_bug.cgi?id=29959
Tom
@@ -1058,6 +1058,10 @@ captured_main_1 (struct captured_main_args *context)
execute_cmdargs (&cmdarg_vec, CMDARG_EARLYINIT_FILE,
CMDARG_EARLYINIT_COMMAND, &ret);
+ /* Set the thread pool size here, so the size can be influenced by the
+ early initialization commands. */
+ update_thread_pool_size ();
+
/* Initialize the extension languages. */
ext_lang_initialization ();
@@ -844,8 +844,9 @@ maintenance_set_profile_cmd (const char *args, int from_tty,
static int n_worker_threads = -1;
-/* Update the thread pool for the desired number of threads. */
-static void
+/* See maint.h. */
+
+void
update_thread_pool_size ()
{
#if CXX_STD_THREAD
@@ -1474,6 +1475,4 @@ such as demangling symbol names."),
maintenance_selftest_option_defs,
&set_selftest_cmdlist,
&show_selftest_cmdlist);
-
- update_thread_pool_size ();
}
@@ -26,6 +26,10 @@ extern void set_per_command_time (int);
extern void set_per_command_space (int);
+/* Update the thread pool for the desired number of threads. */
+
+extern void update_thread_pool_size ();
+
/* Records a run time and space usage to be used as a base for
reporting elapsed time or change in space. */
@@ -99,6 +99,25 @@ proc check_gdb_startups_up_quietly { message } {
}
}
+# Restart GDB and check that the size of the thread pool has not been
+# adjusted to match the number of machine cores at early init time.
+proc check_gdb_maint_show { message } {
+ global gdb_prompt
+ global gdb_sanitizer_msg_re
+ gdb_exit
+ gdb_spawn
+ set setshowprefix "The number of worker threads GDB can use is"
+ set unset "$setshowprefix unlimited \\\(currently 0\\\)."
+ set final "$setshowprefix 1."
+ # Output when CXX_STD_THREAD is undefined.
+ set off "$setshowprefix 0."
+ gdb_test_multiple "" $message {
+ -re "^(${gdb_sanitizer_msg_re})?($unset|$off)\r\n($final|$off)\r\n$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+ }
+}
+
with_ansi_styling_terminal {
# Start GDB and confirm that the version string is styled.
@@ -164,4 +183,25 @@ with_ansi_styling_terminal {
check_gdb_startups_up_quietly \
"check GDB starts quietly using XDG_CONFIG_HOME"
}
+
+ # Create fake home directory for the thread pool size check.
+ set dirs [setup_home_directories "maint-show" \
+ [multi_line_input \
+ "set startup-quietly on" \
+ "maint show worker-threads" \
+ "maint set worker-threads 1" \
+ "maint show worker-threads"]]
+
+ set home_dir [lindex $dirs 0]
+
+ # Now arrange to use the fake home directory startup file.
+ save_vars { INTERNAL_GDBFLAGS env(HOME) env(XDG_CONFIG_HOME) } {
+ set INTERNAL_GDBFLAGS [string map {"-nx" "" "-q" ""} $INTERNAL_GDBFLAGS]
+
+ # Now test GDB when using the HOME directory.
+ set env(HOME) $home_dir
+ unset -nocomplain env(XDG_CONFIG_HOME)
+ check_gdb_maint_show \
+ "check early init of thread pool size"
+ }
}
@@ -154,6 +154,7 @@ thread_pool::set_thread_count (size_t num_threads)
{
#if CXX_STD_THREAD
std::lock_guard<std::mutex> guard (m_tasks_mutex);
+ m_sized_at_least_once = true;
/* If the new size is larger, start some new threads. */
if (m_thread_count < num_threads)
@@ -197,6 +198,9 @@ thread_pool::set_thread_count (size_t num_threads)
void
thread_pool::do_post_task (std::packaged_task<void ()> &&func)
{
+ /* This assert is here to check that no tasks are posted to the pool between
+ its initialization and sizing. */
+ gdb_assert (m_sized_at_least_once);
std::packaged_task<void ()> t (std::move (func));
if (m_thread_count != 0)
@@ -204,6 +204,7 @@ class thread_pool
between the main thread and the worker threads. */
std::condition_variable m_tasks_cv;
std::mutex m_tasks_mutex;
+ bool m_sized_at_least_once = false;
#endif /* CXX_STD_THREAD */
};