Re: [PATCH v2] Speed up next/step while debugging multithreaded programs on AIX.

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2026-09-10 06:16, 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. A simple
> walk through a function took around a minute. In compiler codes this is
> slow.
> 
> TO measure this properly I wrote a benchmark pasted below. The program spawns
> 20 worker threads , then stops inside a function with 10 simple
> that GDB steps over one by one. The GDB batch script breaks at that

Seems to be missing a word after "simple".  Statements?

> 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.
> 
> Now why was it slow?
> 
> Every time the inferior stops -- even for a single-step trap from next
> - the AIX thread layer called pd_update(), which did two very expensive
> things unconditionally:
> 
> 1. pthdb_session_update ()
> 
> This function walks through all of the pthread library's internal
> data structures inside the inferior process, reading them out via
> pdc_read_data() callbacks. With 20 threads that is over 300 memory
> reads from the inferior on every single stop, even when absolutely
> nothing about the threads has changed.
> 
> 2. sync_threadlists()
> 
> This follows the session update with a full rebuild of GDB's thread
> list: iterate every thread via pthdb_pthread(), call
> pthdb_pthread_tid() for each one, allocate two sorted arrays, diff
> them against GDB's internal list, and reconcile the differences.
> Again, all of this on every stop, even when the thread population
> has not changed at all since the last stop.
> 
> On top of that, get_signaled_thread() was calling getthrds() asking for
> one kernel thread descriptor at a time, which costs one syscall per
> thread in the process.
> 
> None of this work is wrong but it is needed when threads actually come
> and go.  But during a next or step sequence through a function, the
> thread list does not change. Paying for a full rescan on every stop is
> something not needed I think.

I mean, it *could* change, you could do "next" over a function that
spawns a thread, a background thread (one other than the one you step)
could spawn a thread, another thread could have exited, etc.

> So what I changed in this patch is:
> 
> 1. Batch getthrds() calls get_signaled_thread ()
> 
> The original code passed 1 as the count argument to getthrds(), so
> it did one syscall per kernel thread to find the one that stopped.
> Changed to fetch 64 descriptors per call and scan the batch in a
> loop. This alone cuts the getthrds() cost from O(N) syscalls to
> O(N/64).

It's unrelated to this patch, but I happen to be looking at aix-thread.c
because of it and I must ask if these things are still relevant, given
the AIX versions and configurations you support nowadays:

1. #if !HAVE_DECL_GETTHRDS
   extern int getthrds (pid_t, struct thrdsinfo64 *, int, tid_t *, int);
   #endif

   The commit message that added this is:

     On newer versions of AIX (6.x and later), this function is actually
     declared in procinfo.h, thus causing a compilation warning when we
     re-declare it ourselves. This patch adds a configure check for that
     function allowing us to declare the function only if the declaration
     isn't already present in one of procinfo system header.

2. /* In AIX 5.1, functions use pthdb_tid_t instead of tid_t.  */
   #ifndef PTHDB_VERSION_3
   #define pthdb_tid_t	tid_t
   #endif

3. #ifdef HAVE_PTRACE64
   # define ptracex(request, pid, addr, data, buf) \
         ptrace64 (request, pid, addr, data, buf)
   #endif

> 2. Track the thread count - aix_thread_variables, pd_activate
> 
> Added a last_thread_count field to aix_thread_variables.  It records
> how many threads libpthdebug reported the last time a full sync was
> done. Initialised to -1 in the constructor so the very
> first stop always does a full sync. Also reset to -1 in pd_activate()
> so that whenever a new debug session is opened the first stop is
> always fully synced regardless of the stop kind.

Ulrich already pointed out that you could have one thread appear and one
disappear, and the count will not change.

> 3. Skip the sync on clean step stops
> 
> pd_update() now takes a bool step_stop parameter.  When that is true
> and last_thread_count is not -1, it skips pthdb_session_update() and
> sync_threadlists() entirely.
> 
> When a sync is needed (not a step stop, or first stop after
> activation), a new helper count_pthdb_threads() does a lightweight
> pass through pthdb_pthread() just to count threads. It only calls
> sync_threadlists() if that count differs from last_thread_count.
> This avoids the expensive pthdb_pthread_tid() calls and array
> allocations on stops where the thread count has not changed.

So yeah, what about doing "next" over a call that spawns a thread, or a
background thread existing while you do a next?

> 4. Detect step stops in wait() and pass the flag down
> 
> wait() already has the stop kind and signal number before it calls
> pd_update(). A TARGET_WAITKIND_STOPPED with GDB_SIGNAL_TRAP is what
> the kernel reports for a completed single-step. wait() now computes
> step_stop from those two fields and passes it to pd_update().

This sounds wrong, as a regular breakpoint is also (typically) reported
as a TARGET_WAITKIND_STOPPED with a SIGTRAP.  In the implementation, you
also check for data->last_resume_step, which is not mentioned here.

> int current_count = count_pthdb_threads (data);
>           if (current_count != data->last_thread_count)
> 
> This makes sure we never miss any genuinely coming new threads and are
> in sync.
> 
> ==================
> 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.  */
>     #include <stdio.h>
>     #include <stdlib.h>
>     #include <pthread.h>
>     #include <unistd.h>
> 
>     #ifndef NUM_THREADS
>     #define NUM_THREADS 20
>     #endif
> 
>     static volatile int keep_running = 1;
> 
>     static void *
>     worker (void *arg)
>     {
>       while (keep_running)
>         sched_yield ();
>       return NULL;
>     }
> 
>     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];
>       int i;
> 
>       for (i = 0; i < NUM_THREADS; i++)
>         pthread_create (&threads[i], NULL, worker, 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=20 -o bench_next_bin bench_next.c -lpthread
> =====================
> GDB batch script (bench_next.gdb):
> 
>     set pagination off
>     set confirm off
> 
>     break do_steps
>     run
> 
>     next
>     next
>     next
>     next
>     next
>     next
>     next
>     next
>     next
>     next
> 
>     quit
> ========================
> Then run,
> time gdb -batch -x bench_next.gdb ./bench_next_bin
> ---
>  gdb/aix-thread.c | 153 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 130 insertions(+), 23 deletions(-)
> 
> diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
> index 5ac71d22237..3370736810a 100644
> --- a/gdb/aix-thread.c
> +++ b/gdb/aix-thread.c
> @@ -173,6 +173,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_thread_count (-1), last_resume_step (0)
> +  {}
> +
>    /* Whether the current application is debuggable by pthdb.  */
>    int pd_able;
>  
> @@ -192,6 +197,19 @@ struct aix_thread_variables
>  
>    /* Describes the number of thread exit events reported.  */
>    std::unordered_set<pthdb_pthread_t> exited_threads;
> +
> +  /* Last known libpthdebug thread count.  Used to skip sync_threadlists()
> +     when the thread population has not changed since the previous stop,
> +     which is the common case during stepping.  -1 means unknown/force sync.  */
> +  int last_thread_count;
> +
> +  /* Set to non-zero by resume() when the resume was a software
> +     single-step, i.e. single-step breakpoints were inserted for the
> +     current 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;
>  };
>  
>  /* Key to our per-inferior data.  */
> @@ -733,6 +751,11 @@ state2str (pthdb_state_t state)
>      }
>  }
>  
> +/* Number of thread descriptors to fetch per getthrds() call.  The original
> +   code fetched one at a time, which costs one syscall per thread.  Fetching
> +   in batches reduces that to one syscall per GETTHRDS_BATCH threads.  */

No need to document what the original code did.  Just keep the first
sentence.

> +#define GETTHRDS_BATCH 64

Prefer:

constexpr int GETTHRDS_BATCH = 64;

> +
>  /* Search through the list of all kernel threads for the thread
>     that has stopped on a SIGTRAP signal, and return its TID.
>     Return 0 if none found.  */

This predates your patch, but is the comment for the get_signaled_thread
function accurate?  It says it looks for a thread that has stopped on a
SIGTRAP signal, but the implementation seems to look for any signal, not
just SIGTRAP.

> @@ -740,20 +763,18 @@ state2str (pthdb_state_t state)
>  static pthdb_tid_t
>  get_signaled_thread (int pid)
>  {
> -  struct thrdsinfo64 thrinf;
> +  struct thrdsinfo64 thrinf[GETTHRDS_BATCH];
>    tid_t ktid = 0;
> +  int count, i;
>  
> -  while (1)
> +  while ((count = getthrds (pid, thrinf, sizeof (thrinf[0]),
> +			    &ktid, GETTHRDS_BATCH)) > 0)

Prefer declaring the variable in the while statement, and separating the
comparison from the assignment.  This should work:

  while (int count = getthrds (pid, thrinf, sizeof (thrinf[0]), &ktid,
			       GETTHRDS_BATCH));
	 count > 0)

>      {
> -      if (getthrds (pid, &thrinf,
> -		    sizeof (thrinf), &ktid, 1) != 1)
> -	break;
> -
>        /* We also need to keep in mind Trap and interrupt or any
>  	 signal that needs to be handled in pd_update ().  */
> -
> -      if (thrinf.ti_cursig)
> -	return thrinf.ti_tid;
> +      for (i = 0; i < count; i++)
> +	if (thrinf[i].ti_cursig)
> +	  return thrinf[i].ti_tid;

Declare variable `i` in the for loop.

Simon