From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by sourceware.org (Postfix) with ESMTPS id AD62C385DC22 for ; Fri, 24 Nov 2023 16:13:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AD62C385DC22 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org AD62C385DC22 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::434 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700842434; cv=none; b=uUemqlFvxjDZi69f8x/4kSotv4jBFVY2m/1f97+DdNnitIle3vpq43Q9jYymtQ5gxopQwcgtFyvWAP8jWU7iAUlknet5QOaLImiD9PXOknxBVf8IlYB0Wn0WtUbypDdHgKGbhu2izLrvGnX4e9ULCn/E/3+SFEfypnLEGgLUqfQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700842434; c=relaxed/simple; bh=Mas4+K8QUgFGom93pJyT2bk4ER76masf4oo2H6Pabqc=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=KEZ1pJ6ZcDCgRl1/NdO5ByA11WlhFCvfWWrqoafv3nm/bLA6Y1+KITqA3VZLPoad8smDQNUwBsjhwxy2GxS2DlM6xZXrwPllFvrf91a4wrMS7wqBlAQ2xHasJ3prohDJQdJrvA6HTkogon74yuW5fasor6KWypj97Ghgu4z4tos= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-x434.google.com with SMTP id ffacd0b85a97d-332e545e852so953531f8f.1 for ; Fri, 24 Nov 2023 08:13:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1700842431; x=1701447231; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=rAI99OPEoR96MCvrldBBp5G1uzFJeA/mnstiV++rG9Q=; b=bYnlyDrHdFU6sFVo73/mOjovghgeMe/ZIyXRZOuALmuWf2YjOr3M3HP/se1LwQlMU/ afreoX5qtw04oLt6A3VgOJS5VSWonjUIovMGOJNW+vRNEk21zVZAWSPLwbQ0aUNGdonm LcqBEIzzUQSI9Xmw3fCuDdsFCuMg3Vs9MBxdQYDpyMJZ799+YwxYQA95gWb2wbdqDw0m 5QYJBtI2UI5ltQNNuSbe3DbG8F8z8JzHqmtKP41yPNoqHByo9fjwaU6tDCR4B8VPHNM7 j3INoEQvNOQcmqsV/gxOhiFN0JGcpq8Fkv+jtU35EsSduL2U5477m528Rf3jpdmGCkA/ 5dsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700842431; x=1701447231; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=rAI99OPEoR96MCvrldBBp5G1uzFJeA/mnstiV++rG9Q=; b=Uu3S4vuYzmcyzJgxY4oNzgZ1v1j2XZX87Q84t3DFQO/vzSxJcAe078PjVgTJQ53fwt TDMybnZzmzNM52Xa0Ytup8RY3WcvKwV/f/xwfMINtJHVYuHVHBs3jJgnlkTlQrT4K5xe HHL31zY1gBXtyuJ0ZW+yChkFWng7p6qctyz4KGPyvnazcwnNmHvCF0jRjmEULZq6B2Wp hZeXH/qacVa2XyFqc7WPYlD1+eOvBo9YzYbruH7DglDOQHpyKgrrjPuZ7GUz+wDhp6vu ZSpC7eB4r77iAAh6rr9XmotSHfLlud8K62qw5TuMq6IkLn0bcnbwTeAu7hDYgP752cu+ 9QuA== X-Gm-Message-State: AOJu0Yx43HwkoPRJZHCxnbhTHKjJG3zHNZ9cblaBuKKtDQ3WY/omSK7G nnET9wKN1jM4w9SQuOikCrD/pp/b8uPcZ9teLrMZNw== X-Google-Smtp-Source: AGHT+IEMBcICVHPI68ZHLLoyBZr1y7Tpoj7odKGLj7FbGVigEuGYppS5ICjLrY9Ca/qyJoifa4V4fw== X-Received: by 2002:a5d:6b4b:0:b0:332:c12c:367c with SMTP id x11-20020a5d6b4b000000b00332c12c367cmr6318956wrw.30.1700842431281; Fri, 24 Nov 2023 08:13:51 -0800 (PST) Received: from whitegate.. (cpc77030-warw18-2-0-cust254.3-2.cable.virginm.net. [86.9.96.255]) by smtp.gmail.com with ESMTPSA id c13-20020adfe70d000000b003316aeb280esm4569150wrm.104.2023.11.24.08.13.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Nov 2023 08:13:50 -0800 (PST) From: Richard Bunt To: gdb-patches@sourceware.org Cc: Richard Bunt , Tom Tromey Subject: [PATCH v4 2/2] gdb: Enable early init of thread pool size Date: Fri, 24 Nov 2023 16:13:21 +0000 Message-Id: <20231124161321.572511-3-richard.bunt@linaro.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20231124161321.572511-1-richard.bunt@linaro.org> References: <20231124161321.572511-1-richard.bunt@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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. Additionally, the default number of worker threads is clamped at eight to control the number of worker threads spawned on many-core systems. This value was chosen as testing recorded on bugzilla issue 29959 indicates that parallel efficiency drops past this point. 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. The clamping was tested on machines with both 16 cores and a single core. "maint show worker-threads" correctly reported eight and one respectively. Approved-By: Tom Tromey --- gdb/main.c | 4 +++ gdb/maint.c | 18 ++++++---- 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, 65 insertions(+), 6 deletions(-) diff --git a/gdb/main.c b/gdb/main.c index eb11d6f2e46..688db7655a9 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -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 (); diff --git a/gdb/maint.c b/gdb/maint.c index c1154d0a54e..2e6754c9558 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -844,15 +844,23 @@ 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 int n_threads = n_worker_threads; if (n_threads < 0) - n_threads = std::thread::hardware_concurrency (); + { + const int hardware_threads = std::thread::hardware_concurrency (); + /* Testing in #29959 indicates that parallel efficiency drops between + n_threads=5 to 8. Therefore, clamp the default value to 8 to avoid an + excessive number of threads in the pool on many-core systems. */ + const int throttle = 8; + n_threads = std::clamp (hardware_threads, hardware_threads, throttle); + } gdb::thread_pool::g_thread_pool->set_thread_count (n_threads); #endif @@ -874,7 +882,7 @@ maintenance_show_worker_threads (struct ui_file *file, int from_tty, if (n_worker_threads == -1) { gdb_printf (file, _("The number of worker threads GDB " - "can use is unlimited (currently %zu).\n"), + "can use is the default (currently %zu).\n"), gdb::thread_pool::g_thread_pool->thread_count ()); return; } @@ -1474,6 +1482,4 @@ such as demangling symbol names."), maintenance_selftest_option_defs, &set_selftest_cmdlist, &show_selftest_cmdlist); - - update_thread_pool_size (); } diff --git a/gdb/maint.h b/gdb/maint.h index 1741d132567..2ec2493d43d 100644 --- a/gdb/maint.h +++ b/gdb/maint.h @@ -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. */ diff --git a/gdb/testsuite/gdb.base/early-init-file.exp b/gdb/testsuite/gdb.base/early-init-file.exp index 6426da8dacf..d09fd145cbf 100644 --- a/gdb/testsuite/gdb.base/early-init-file.exp +++ b/gdb/testsuite/gdb.base/early-init-file.exp @@ -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 the default \\\(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" + } } diff --git a/gdbsupport/thread-pool.cc b/gdbsupport/thread-pool.cc index bbe043dc0a3..0b11213729f 100644 --- a/gdbsupport/thread-pool.cc +++ b/gdbsupport/thread-pool.cc @@ -154,6 +154,7 @@ thread_pool::set_thread_count (size_t num_threads) { #if CXX_STD_THREAD std::lock_guard 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 &&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 t (std::move (func)); if (m_thread_count != 0) diff --git a/gdbsupport/thread-pool.h b/gdbsupport/thread-pool.h index d5e1dc7fce1..6959414476f 100644 --- a/gdbsupport/thread-pool.h +++ b/gdbsupport/thread-pool.h @@ -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 */ }; -- 2.32.0