From: Simon Marchi <simon.marchi@polymtl.ca>
To: Aditya Vidyadhar Kamath <akamath996@gmail.com>,
ulrich.weigand@de.ibm.com, tom@tromey.com
Cc: gdb-patches@sourceware.org, Aditya.Kamath1@ibm.com,
sangamesh.swamy@in.ibm.com
Subject: Re: [PATCH v3 3/3][RFC] Speed up next/step while debugging multithreaded programs on AIX.
Date: Wed, 16 Sep 2026 15:06:29 -0400 [thread overview]
Message-ID: <acfea0a3-20ba-4737-a88f-a337afebe083@polymtl.ca> (raw)
In-Reply-To: <20260916114931.17516-3-akamath996@gmail.com>
On 2026-09-16 07:49, Aditya Vidyadhar Kamath wrote:
> From: Aditya Vidyadhar Kamath <aditya.kamath1@ibm.com>
>
> During go compiler debugging on AIX my collegue and I noticed that
> the next command was pretty slow compared to Linux - around 7 seconds
> per step when program has atleast 3 or more threads running. Debugging
> 10 - 12 line function took around a minute. In compiler codes this is
> slow.
>
> To measure this properly I wrote a benchmark pasted below. The program spawns
> 3 worker threads , then stops inside a function with 10 simple
> that GDB steps over one by one. The GDB batch script breaks at that
"10 simple" what?
> function and issues 10 next commands and then quits. Timing the whole
> run gives a fair picture of how much overhead each next carries.
> Both the .c file and batch file are pasted below.
>
> Before this patch, on AIX 7.3 ppc64 with 20 background threads:
>
> real 1m10.05s
> user 0m17.59s
> sys 0m20.43s
>
> That is roughly 7 seconds per `next` step.
>
> After this patch:
>
> real 0m20.37s
> user 0m5.94s
> sys 0m6.53s
>
> About 2 seconds per step. 3.5x faster for the same workload.
>
> Every stop including single-step traps from next/step called
> pd_update(), which unconditionally ran pthdb_session_update() followed
> by sync_threadlists(). The scoped_time_it instrumentation added in the
> previous commit shows that the entire cost is in sync_threadlists(),
> dominated by the first pthdb_pthread(PTHDB_LIST_FIRST) call.
>
> This cost is the same regardless of thread count.
> So it is a fixed per-stop overhead paid on every single-step trap.
>
> During a next/step sequence I assume,
> the stepped thread is running a single instruction, so it cannot call
> pthread_create or pthread_exit. It is therefore safe to skip both
> pthdb_session_update() and sync_threadlists() when we know this stop
> is the result of a software single-step.
Again, I am not sure that's true. There is certainly one single
instruction (possibly a syscall) inside pthread_create that commits the
thread creation, after which the new thread will be visible. If you
single-step that instruction, you'll have one more thread after stepping
than you had before.
And with "set scheduler-locking off", threads other than the one being
stepped run freely during the step. One of those threads could exit
during your instruction single step, so you'd miss that.
If you know that new threads can only appear during a syscall
instruction (I have no idea, just speculating), then you could perhaps
check what is the instruction about to be stepped. If it's not a
syscall instruction, and if the scheduler-locking setting is "on" or
"step" (nothing else than this thread will run during the step), then
perhaps it would be safe to skip the thread update. But you'd have to
check how it really works under the hood.
> In AIX, single-stepping is implemented in software: GDB inserts
> breakpoints at the next instruction before resuming with step=0.
> A completed software single-step and a user breakpoint both arrive as
> TARGET_WAITKIND_STOPPED/GDB_SIGNAL_TRAP and are otherwise
> indistinguishable at the target layer. The only reliable indicator is
> whether single-step breakpoints were inserted for the resumed thread
> just before the resume. Which the code is doing.
Regardless of the problems I stated above: do you actually need to
record this during resume? Couldn't you check
thread_has_single_step_breakpoints_set in wait to know if the signalled
thread has single-step breakpoints set? I assume it would still return
true, because infrun wouldn't have removed it yet.
> For now this patch is request for comment / draft. Once feedback is
> recieved from maintainers I will remove this paragraph and benchmark results
> pasted below.
>
> ==================
> Benchmark program (bench_next.c):
>
> /* Spawns NUM_THREADS worker threads that spin, then steps through
> 10 assignments in do_steps() so GDB can time each next.
>
> Also tests two scenarios from the bug report:
> 1. "next" over a call that spawns a thread (spawn_thread_here()).
> 2. A background thread exits while "next" is in progress
> (exit_thread_here()). */
> #include <stdio.h>
> #include <stdlib.h>
> #include <pthread.h>
> #include <unistd.h>
>
> #ifndef NUM_THREADS
> #define NUM_THREADS 3
> #endif
>
> static volatile int keep_running = 1;
> static volatile int extra_thread_started = 0;
>
> static void *
> worker (void *arg)
> {
> while (keep_running)
> sched_yield ();
> return NULL;
> }
>
> /* Thread that exits on its own after signalling it has started. */
> static void *
> short_lived (void *arg)
> {
> extra_thread_started = 1;
> return NULL;
> }
>
> /* GDB will "next" over this call -- a new thread is spawned inside. */
> static void
> spawn_thread_here (void)
> {
> pthread_t t;
> pthread_create (&t, NULL, short_lived, NULL);
> pthread_join (t, NULL);
> }
>
> /* GDB will "next" over this call -- the short-lived background thread
> is in the process of exiting while we step. */
> static void
> exit_thread_here (void)
> {
> /* Just a visible marker so GDB can stop here. */
> volatile int x = 42;
> (void)x;
> }
>
> void
> do_steps (void)
> {
> volatile int a = 1;
> volatile int b = 2;
> volatile int c = a + b;
> volatile int d = c * 2;
> volatile int e = d - a;
> volatile int f = e + c;
> volatile int g = f / 2;
> volatile int h = g + 1;
> volatile int i2 = h * h;
> volatile int j = i2 - b;
> (void)j;
> }
>
> int
> main (void)
> {
> pthread_t threads[NUM_THREADS];
> pthread_t bg;
> int i;
>
> /* Start background spinning threads. */
> for (i = 0; i < NUM_THREADS; i++)
> pthread_create (&threads[i], NULL, worker, NULL);
>
> /* Next over a call that spawns a thread */
> spawn_thread_here ();
>
> /* Next while a background thread is exiting
> Launch a thread that exits quickly; "next" through exit_thread_here()
> while that exit is in flight. */
> pthread_create (&bg, NULL, short_lived, NULL);
> exit_thread_here ();
> pthread_join (bg, NULL);
>
> do_steps ();
>
> keep_running = 0;
> for (i = 0; i < NUM_THREADS; i++)
> pthread_join (threads[i], NULL);
>
> return 0;
> }
>
> ====================
> Build done with
> gcc -O0 -g -gdwarf -maix64 -DNUM_THREADS=3 -o bench_next_bin bench_next.c -lpthread
> =====================
> GDB batch script (bench_next.gdb):
>
> set pagination off
> set confirm off
> maintenance set per-command time on
>
> # Scenario 1: next over a call that spawns a thread
> break spawn_thread_here
> run
> next
>
> # Scenario 2: next while a background thread is exiting
> break exit_thread_here
> continue
> next
>
> break do_steps
> continue
>
> next
> next
> next
> next
> next
> next
> next
> next
> next
> next
>
> quit
> ========================
> Then run,
> time gdb -batch -x bench_next.gdb ./bench_next_bin
>
> =========================================
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 0.941, user 0.094, sys 0.096, user+sys 0.190, 20.2 % CPU
> Time for "pd_update total": wall 0.941, user 0.094, sys 0.096, user+sys 0.190, 20.2 % CPU
> 25 volatile int a = 1;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 0.938, user 0.093, sys 0.096, user+sys 0.189, 20.1 % CPU
> Time for "pd_update total": wall 0.938, user 0.093, sys 0.096, user+sys 0.189, 20.1 % CPU
> 26 volatile int b = 2;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.149, user 0.082, sys 0.090, user+sys 0.172, 15.0 % CPU
> Time for "pd_update total": wall 1.149, user 0.082, sys 0.090, user+sys 0.172, 15.0 % CPU
> 27 volatile int c = a + b;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.262, user 0.075, sys 0.087, user+sys 0.162, 12.8 % CPU
> Time for "pd_update total": wall 1.262, user 0.075, sys 0.087, user+sys 0.162, 12.8 % CPU
> 28 volatile int d = c * 2;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.281, user 0.078, sys 0.091, user+sys 0.169, 13.2 % CPU
> Time for "pd_update total": wall 1.281, user 0.078, sys 0.091, user+sys 0.169, 13.2 % CPU
> 29 volatile int e = d - a;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.264, user 0.076, sys 0.087, user+sys 0.163, 12.9 % CPU
> Time for "pd_update total": wall 1.264, user 0.076, sys 0.087, user+sys 0.163, 12.9 % CPU
> 30 volatile int f = e + c;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.262, user 0.075, sys 0.086, user+sys 0.161, 12.8 % CPU
> Time for "pd_update total": wall 1.262, user 0.075, sys 0.086, user+sys 0.161, 12.8 % CPU
> 31 volatile int g = f / 2;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.220, user 0.086, sys 0.098, user+sys 0.184, 15.1 % CPU
> Time for "pd_update total": wall 1.220, user 0.086, sys 0.098, user+sys 0.184, 15.1 % CPU
> 32 volatile int h = g + 1;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.071, user 0.091, sys 0.098, user+sys 0.189, 17.6 % CPU
> Time for "pd_update total": wall 1.071, user 0.091, sys 0.098, user+sys 0.189, 17.6 % CPU
> 33 volatile int i2 = h * h;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.276, user 0.077, sys 0.089, user+sys 0.166, 13.0 % CPU
> Time for "pd_update total": wall 1.276, user 0.077, sys 0.089, user+sys 0.166, 13.0 % CPU
> 34 volatile int j = i2 - b;
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pd_update total": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "pthdb_session_update": wall 0.000, user 0.000, sys 0.000, user+sys 0.000, NaNQ % CPU
> Time for "sync_threadlists": wall 1.278, user 0.077, sys 0.091, user+sys 0.168, 13.1 % CPU
> Time for "pd_update total": wall 1.278, user 0.077, sys 0.091, user+sys 0.168, 13.1 % CPU
When you show benchmark numbers, you should show both the before and
after numbers. The after numbers in isolation mean nothing.
Also, try to trim the logs a bit to make it easier to understand for
others. You don't need to paste 10 steps, just choose one that is
representative.
You also mentioned above that the especially costly call was the first
pthdb_pthread one, but it doesn't even appear in this log, even though
there is a "scoped_time_it" for it.
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 2355f87db54..72a26153aae 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -165,6 +165,11 @@ static pthdb_callbacks_t pd_callbacks = {
> /* Aix variable structure. */
> struct aix_thread_variables
> {
> + aix_thread_variables ()
> + : pd_able (0), pd_active (0), pd_session (0), pd_brk_addr (0),
> + arch64 (0), last_resume_step (0)
> + {}
Initialize the fields directly where they are declared in the struct,
like this:
/* Whether the current application is debuggable by pthdb. */
int pd_able = 0;
/* Whether a threaded application is being debugged. */
int pd_active = 0;
Is it an existing bug that the existing fields are not initialized?
This structure is allocated by registry::try_emplace, and I don't see
anything that zeroes the structure. In any case that could be a patch
that you could send separately, it should be trivial. And while at it,
some of these fields could be changed from int to bool.
> @@ -184,6 +189,14 @@ struct aix_thread_variables
>
> /* Describes the number of thread exit events reported. */
> std::unordered_set<pthdb_pthread_t> exited_threads;
> +
> + /* Set to non-zero by resume() when the resume was a software
> + single-step, i.e. single-step breakpoints were inserted for the
> + resumed thread before the inferior was set running. Cleared to
> + zero for any other kind of resume. Used by wait() to distinguish
> + a SIGTRAP from a completed single-step from one caused by a
> + breakpoint, since both look identical at the signal level. */
> + int last_resume_step;
This should be a bool, and the comment should read "Set to true" instead
of "non-zero".
Simon
prev parent reply other threads:[~2026-09-16 19:07 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-16 11:49 Aditya Vidyadhar Kamath
2026-09-16 19:06 ` Simon Marchi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acfea0a3-20ba-4737-a88f-a337afebe083@polymtl.ca \
--to=simon.marchi@polymtl.ca \
--cc=Aditya.Kamath1@ibm.com \
--cc=akamath996@gmail.com \
--cc=gdb-patches@sourceware.org \
--cc=sangamesh.swamy@in.ibm.com \
--cc=tom@tromey.com \
--cc=ulrich.weigand@de.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox