From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id UC8/Lo2eoWrehToAWB0awg (envelope-from ) for ; Wed, 09 Sep 2026 13:59:41 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=Oen2Qw+n; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 9B4F01E09E; Wed, 09 Sep 2026 13:59:41 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id E2E501E033 for ; Wed, 09 Sep 2026 13:59:38 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E63A84BB3BA9 for ; Wed, 9 Sep 2026 17:59:37 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E63A84BB3BA9 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=Oen2Qw+n Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id EFD944B99F79 for ; Wed, 9 Sep 2026 17:59:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EFD944B99F79 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org EFD944B99F79 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=148.163.156.1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1788976747; cv=none; b=LTMklfnkrp7ML0fCsjrYIg3VcrhlY+Lh3mBXYfS6CKjFsUUx8s0ackizL3y+k09HlCIgkKQprJdic1wbCQ4QcvLDe5rizdyerhUk2BaL95fnXmlU0VgNYB3qn/YYMFlSVy787Mz31NmyNzVTAkp/5EWa4LKF1MBT4JMfCfUkYcw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1788976747; c=relaxed/simple; bh=qmXZjafvXtgSw/CGlksamZDsEj7CF5ObsEb0CN5P4vU=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=DKrwAqqebtjHTUIarJaWNsENgenP6j/puh4yz7NoZDAElIX0dqEYea2Re74zOMn12wi9qrCybhepMT+GlEDrYAzlllxSi71q4eJpAd14bCB4qt/ACkH92+GuCxUdrMEAV7RzhUGHt+8YbvG57/eyY8YDQD7o4WLcenKzHQIJgRw= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=Oen2Qw+n DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EFD944B99F79 Received: from pps.filterd (m0356517.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 689HVjgg443143; Wed, 9 Sep 2026 17:59:03 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=cc :content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to; s=pp1; bh=fjMSXTnNwnverWXUQD1ekbtg5tSlv4 O4NJF7LAI8TME=; b=Oen2Qw+niPqUp+ZuYVvbnSDqolei8LgEULyo6ktYv0ZkbC pOnDh3e4eAW865O+ilBTc3bfCLaJZhVb5elsqoE3V/pOa1n2kIP7gDgwSedaNGPR uKJznsapV/gM12at3iGIPaxmR4n5je1svJN6UbqRvO4xvE9HzIiaBfdaL1OwST0Y xsXxhDR1ESH4dvOiL5VkbhAs94BDZTJ+J5zTgMgjc77DH0XueDDMWyxMTxe0MFZF dQgYurBEDM6L/Ah1ByDAwqkxuXPxvR/VGRi3wT834pzrMgyq2QPwVHaEKgUe9D+7 NxW9VXAIuA8Ts1O++F6RMrgjyH9oEylprOiQ4E1Q== Received: from ppma21.wdc07v.mail.ibm.com (5b.69.3da9.ip4.static.sl-reverse.com [169.61.105.91]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 4ggbhkyn9k-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 09 Sep 2026 17:59:02 +0000 (GMT) Received: from pps.filterd (ppma21.wdc07v.mail.ibm.com [127.0.0.1]) by ppma21.wdc07v.mail.ibm.com (8.18.1.7/8.18.1.7) with ESMTP id 689HuJRs013958; Wed, 9 Sep 2026 17:59:01 GMT Received: from smtprelay06.dal12v.mail.ibm.com ([172.16.1.8]) by ppma21.wdc07v.mail.ibm.com (PPS) with ESMTPS id 4ggxdk3rpw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 09 Sep 2026 17:59:01 +0000 (GMT) Received: from smtpav02.wdc07v.mail.ibm.com (smtpav02.wdc07v.mail.ibm.com [10.39.53.229]) by smtprelay06.dal12v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 689HwwG815270614 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 9 Sep 2026 17:58:59 GMT Received: from smtpav02.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id BE2A758059; Wed, 9 Sep 2026 17:58:58 +0000 (GMT) Received: from smtpav02.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 628785805B; Wed, 9 Sep 2026 17:58:55 +0000 (GMT) Received: from [9.111.203.169] (unknown [9.111.203.169]) by smtpav02.wdc07v.mail.ibm.com (Postfix) with ESMTP; Wed, 9 Sep 2026 17:58:54 +0000 (GMT) Content-Type: multipart/alternative; boundary="------------sUNVgJP2m6u6fraFyiERWSuy" Message-ID: Date: Wed, 9 Sep 2026 23:28:53 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1][RFC] Speed up next/step while debugging multithreaded programs on AIX. To: Aditya Vidyadhar Kamath , ulrich.weigand@de.ibm.com, simon.marchi@polymtl.ca, tom@tromey.com Cc: gdb-patches@sourceware.org, Aditya.Kamath1@ibm.com, sangamesh.swamy@in.ibm.com References: <20260909054306.73173-2-akamath996@gmail.com> From: Abhay Kandpal Content-Language: en-GB In-Reply-To: <20260909054306.73173-2-akamath996@gmail.com> X-TM-AS-GCONF: 00 X-Proofpoint-Reinject: loops=2 maxloops=12 X-Proofpoint-Spam-Info: AW1haW4tMjYwOTA5MDE5NSBTYWx0ZWRfX+M/k6Dvb3ksU nTnxO68PWwl+URbbgvOgkTUryc5ST5F/wofL8pIfJbWcDTnJWN+gMhPT3aPj+9OQhTHr9jo/PF2 kwj21t+alPPHH2kR0/4QIkJBlnKOpO4= X-Proofpoint-ORIG-GUID: RiLfIqpZpRGKi1c6nhmGP5x_N0xCzdKM X-Authority-Analysis: v=2.4 cv=NMDlPU6g c=1 sm=1 tr=0 ts=6aa19e67 cx=c_pps a=GFwsV6G8L6GxiO2Y/PsHdQ==:117 a=GFwsV6G8L6GxiO2Y/PsHdQ==:17 a=VdqzKS8jKosA:10 a=VkNPw1HP01LnGYTKEx00:22 a=RnoormkPH1_aCDwRdu11:22 a=U7nrCbtTmkRpXpFmAIza:22 a=r77TgQKjGQsHNAKrUKIA:9 a=VnNF1IyMAAAA:8 a=mc9R8THh_amOhPhcuX8A:9 a=QEXdDO2ut3YA:10 a=pGLkceISAAAA:8 a=GnVoJuMo5STP1W1EZ3cA:9 a=WAPGGm9kvH46qhAD:21 a=_W_S_7VecoQA:10 a=lqcHg5cX4UMA:10 X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwOTA5MDE5NSBTYWx0ZWRfX5LKzv0uJ3QUo 6ANmzmW/o6r8RfNVICc6fL57ZpjNpFNwx1t2ckqtB/CAjY9KOeBAjpCpok5NynMpOwzdzxEYxhY AEXEkeh/0Fee8l9Usi7bCtqFqd4IAhZN1NRnRgzymI4rvqVbgKNBuca6pRsyhos9wrjH+RNISnP FE53uAFvvUeXXkVX29b7dACrycFIgsKzyd+3PytIRXyQ+GCnIH0hcXFNBlW5MjZaC6/IigMtXlb vK49vaB3z3BAJR/O27h0e73/XD+puxz/eZxiqxsB4hkyKeUQSuDgWm6P5AEfKYa2gK2vnYD0nC6 +SqW/sysWYpVnW8QDMkoT+qh2ji36un6SBcyPexCUW9gcfSTkZL+2q0DEo+cVRQJshTnDOjBMVZ v5T20SPCSHkTaLRhXPtY4M/d6gmlawE6jdMTZQf4lau+tOinbD5Z1SB8HMWnyWq2K5BGw+vEFad z8KBhRgRCY++Cd6Veww== X-Proofpoint-GUID: crpVv2DxffSR5izpw0V9kt9ukcM4EQG4 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1176,Hydra:6.1.134,FMLib:17.12.100.49 definitions=2026-09-08_03,2026-09-09_02,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 spamscore=0 lowpriorityscore=0 clxscore=1011 adultscore=0 impostorscore=0 bulkscore=0 malwarescore=0 priorityscore=1501 suspectscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2606150000 definitions=main-2609090195 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org This is a multi-part message in MIME format. --------------sUNVgJP2m6u6fraFyiERWSuy Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Aditya, On 09/09/26 11:13, Aditya Vidyadhar Kamath wrote: > From: Aditya Vidyadhar Kamath > > 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 > 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. > > So what changes this patch does 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). > > 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. > > 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. > > 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(). > > 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. > > So these are the things I thought and did. Let me know what you think. > > ================== > 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 > #include > #include > #include > > #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 | 117 +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 94 insertions(+), 23 deletions(-) > > diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c > index 5ac71d22237..5c43e1130e4 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) > + {} > + > /* Whether the current application is debuggable by pthdb. */ > int pd_able; > > @@ -192,6 +197,11 @@ struct aix_thread_variables > > /* Describes the number of thread exit events reported. */ > std::unordered_set 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; > }; > > /* Key to our per-inferior data. */ > @@ -733,6 +743,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. */ > +#define 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. */ > @@ -740,20 +755,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) > { > - 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; } > /* Didn't find any thread stopped on a SIGTRAP signal. */ @@ -861,13 > +874,39 @@ sync_threadlists (pid_t pid) } } +/* Count the number of > live pthreads visible to libpthdebug. + Used before sync_threadlists() > to check whether the thread population + has changed since the last > stop. */ + +static int +count_pthdb_threads (struct > aix_thread_variables *data) +{ + pthdb_pthread_t pdtid; + int cmd, n = > 0; + + for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT) + { + int > status = pthdb_pthread (data->pd_session, &pdtid, cmd); > + if (status != PTHDB_SUCCESS || pdtid == PTHDB_INVALID_PTHREAD) > + break; > + n++; > + } > + return n; > +} > + > /* Synchronize libpthdebug's state with the inferior and with GDB, > generate a composite process/thread for the current thread, > - Return the ptid of the event thread if one can be found, else > - return a pid-only ptid with PID. */ > + return the ptid of the event thread if one can be found, else > + return a pid-only ptid with PID. > + > + STEP_STOP should be set when the stop is a single-step SIGTRAP. > + In that case, if the thread list was already synced at a previous > + stop, both pthdb_session_update() and sync_threadlists() are skipped. > + Those two calls dominate the cost of every stop in a multithreaded > + program, so skipping them during stepping gives a large speedup. */ > > static ptid_t > -pd_update (pid_t pid) > +pd_update (pid_t pid, bool step_stop = false) > { > int status; > ptid_t ptid; > @@ -880,15 +919,36 @@ pd_update (pid_t pid) > if (!data->pd_active) > return ptid_t (pid); > > - status = pthdb_session_update (data->pd_session); > - if (status != PTHDB_SUCCESS) > - return ptid_t (pid); > - > - /* Attempt to sync_threadlists () only when debugging object files > - and not core files since list of threads never change for core > - files. */ > - if (target_has_execution ()) > - sync_threadlists (pid); > + /* Skip the session update and thread list sync when stepping. > + pthdb_session_update() reads through all pthread data structures in > + the inferior on every call, and sync_threadlists() follows it with > + O(N) kernel calls per thread. During a next/step sequence the thread > + list does not change, so both calls can be omitted as long as we have > + done at least one full sync since activation (last_thread_count >= 0). > + Any other stop kind (breakpoints, real signals) always does a full > + sync because threads may have been created or destroyed. */ > + bool need_sync = !step_stop || (data->last_thread_count < 0); > + > + if (need_sync) > + { > + status = pthdb_session_update (data->pd_session); > + if (status != PTHDB_SUCCESS) > + return ptid_t (pid); > + > + /* Attempt to sync_threadlists() only when debugging object files > + and not core files since the list of threads never changes for > + core files. Even for live inferiors, skip the sync when the > + thread count matches the previous stop. */ > + if (target_has_execution ()) > + { > + int current_count = count_pthdb_threads (data); > + if (current_count != data->last_thread_count) > + { > + sync_threadlists (pid); > + data->last_thread_count = current_count; > + } > + } > + } > > /* Define "current thread" as one that just received a trap signal. */ > > @@ -921,7 +981,12 @@ pd_activate (pid_t pid) > PTHDB_FLAG_REGS, &pd_callbacks, > &data->pd_session); > if (status == PTHDB_SUCCESS) > - data->pd_active = 1; > + { > + data->pd_active = 1; > + /* Reset last_thread_count so that the very first stop after > + activation always does a full sync_threadlists() pass. */ > + data->last_thread_count = -1; > + } > } > > /* AIX implementation of update_thread_list. */ > @@ -1125,7 +1190,13 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status, > pd_activate (ptid.pid ()); > } > > - return pd_update (ptid.pid ()); > + /* Tell pd_update() whether this is a single-step SIGTRAP so it can > + skip the expensive pthdb_session_update() and sync_threadlists() > + calls. Breakpoints, real signals and other stop kinds always get > + the full sync. */ > + bool step_stop = (status->kind () == TARGET_WAITKIND_STOPPED > + && status->sig () == GDB_SIGNAL_TRAP); > + return pd_update (ptid.pid (), step_stop); Doesn't a breakpoint stop also arrive as TARGET_WAITKIND_STOPPED with GDB_SIGNAL_TRAP? get_signaled_thread() already looks for the thread stopped on SIGTRAP to find the current thread, and that has to work at breakpoint stops too, so I would expect a breakpoint hit to look the same as a single-step here. If so, breakpoint stops also get step_stop = true and skip the sync, which is the opposite of what the commit message says: "Any other stop kind (breakpoints, real signals) always does a full sync because threads may have been created or destroyed." The last_thread_count < 0 check only covers the first stop after pd_activate(). After that first sync, last_thread_count is >= 0 for the rest of the session and need_sync becomes just !step_stop. So from the second stop onwards a breakpoint hit skips the sync, and any threads created since the previous stop are missed. The benchmark won't show this: all 20 threads are created before do_steps() and there is only one breakpoint, so the thread list never changes after the first stop. resume() is already told whether GDB asked for a single step, and it already has the aix_thread_variables pointer in hand. Could you save the flag there and use it here instead of inferring it from the signal? /* aix_thread_target::resume (), after the existing data = get_thread_data_helper_for_ptid (ptid); */ data->last_resume_step = step; /* in wait () */ bool step_stop = (data->last_resume_step && status->kind () == TARGET_WAITKIND_STOPPED && status->sig () == GDB_SIGNAL_TRAP); That would also handle "next" over a function call, where GDB puts a temporary breakpoint at the return address and continues rather than single-stepping through the callee. resume() is called with step = 0 in that case, so the stop would correctly get a full sync even though the user typed "next". Regards, Abhay > } > > /* Supply AIX altivec registers, both 64 and 32 bit. */ --------------sUNVgJP2m6u6fraFyiERWSuy Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit 
Hi Aditya,
On 09/09/26 11:13, 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
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.

So what changes this patch does 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).

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.

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.

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

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.

So these are the  things I thought and did. Let me know what you think.

==================
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 | 117 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 94 insertions(+), 23 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 5ac71d22237..5c43e1130e4 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)
+  {}
+
   /* Whether the current application is debuggable by pthdb.  */
   int pd_able;
 
@@ -192,6 +197,11 @@ 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;
 };
 
 /* Key to our per-inferior data.  */
@@ -733,6 +743,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.  */
+#define 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.  */
@@ -740,20 +755,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)
     {
-      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;
     }
 
   /* Didn't find any thread stopped on a SIGTRAP signal.  */
@@ -861,13 +874,39 @@ sync_threadlists (pid_t pid)
       }
 }
 
+/* Count the number of live pthreads visible to libpthdebug.
+   Used before sync_threadlists() to check whether the thread population
+   has changed since the last stop.  */
+
+static int
+count_pthdb_threads (struct aix_thread_variables *data)
+{
+  pthdb_pthread_t pdtid;
+  int cmd, n = 0;
+
+  for (cmd = PTHDB_LIST_FIRST;; cmd = PTHDB_LIST_NEXT)
+    {
+      int status = pthdb_pthread (data->pd_session, &pdtid, cmd);
+      if (status != PTHDB_SUCCESS || pdtid == PTHDB_INVALID_PTHREAD)
+	break;
+      n++;
+    }
+  return n;
+}
+
 /* Synchronize libpthdebug's state with the inferior and with GDB,
    generate a composite process/thread <pid> for the current thread,
-   Return the ptid of the event thread if one can be found, else
-   return a pid-only ptid with PID.  */
+   return the ptid of the event thread if one can be found, else
+   return a pid-only ptid with PID.
+
+   STEP_STOP should be set when the stop is a single-step SIGTRAP.
+   In that case, if the thread list was already synced at a previous
+   stop, both pthdb_session_update() and sync_threadlists() are skipped.
+   Those two calls dominate the cost of every stop in a multithreaded
+   program, so skipping them during stepping gives a large speedup.  */
 
 static ptid_t
-pd_update (pid_t pid)
+pd_update (pid_t pid, bool step_stop = false)
 {
   int status;
   ptid_t ptid;
@@ -880,15 +919,36 @@ pd_update (pid_t pid)
   if (!data->pd_active)
     return ptid_t (pid);
 
-  status = pthdb_session_update (data->pd_session);
-  if (status != PTHDB_SUCCESS)
-    return ptid_t (pid);
-
-  /* Attempt to sync_threadlists () only when debugging object files
-     and not core files since list of threads never change for core
-     files.  */
-  if (target_has_execution ())
-    sync_threadlists (pid);
+  /* Skip the session update and thread list sync when stepping.
+     pthdb_session_update() reads through all pthread data structures in
+     the inferior on every call, and sync_threadlists() follows it with
+     O(N) kernel calls per thread.  During a next/step sequence the thread
+     list does not change, so both calls can be omitted as long as we have
+     done at least one full sync since activation (last_thread_count >= 0).
+     Any other stop kind (breakpoints, real signals) always does a full
+     sync because threads may have been created or destroyed.  */
+  bool need_sync = !step_stop || (data->last_thread_count < 0);
+
+  if (need_sync)
+    {
+      status = pthdb_session_update (data->pd_session);
+      if (status != PTHDB_SUCCESS)
+	return ptid_t (pid);
+
+      /* Attempt to sync_threadlists() only when debugging object files
+	 and not core files since the list of threads never changes for
+	 core files.  Even for live inferiors, skip the sync when the
+	 thread count matches the previous stop.  */
+      if (target_has_execution ())
+	{
+	  int current_count = count_pthdb_threads (data);
+	  if (current_count != data->last_thread_count)
+	    {
+	      sync_threadlists (pid);
+	      data->last_thread_count = current_count;
+	    }
+	}
+    }
 
   /* Define "current thread" as one that just received a trap signal.  */
 
@@ -921,7 +981,12 @@ pd_activate (pid_t pid)
 			       PTHDB_FLAG_REGS, &pd_callbacks,
 			       &data->pd_session);
   if (status == PTHDB_SUCCESS)
-    data->pd_active = 1;
+    {
+      data->pd_active = 1;
+      /* Reset last_thread_count so that the very first stop after
+	 activation always does a full sync_threadlists() pass.  */
+      data->last_thread_count = -1;
+    }
 }
 
 /* AIX implementation of update_thread_list.  */
@@ -1125,7 +1190,13 @@ aix_thread_target::wait (ptid_t ptid, struct target_waitstatus *status,
 	pd_activate (ptid.pid ());
     }
 
-  return pd_update (ptid.pid ());
+  /* Tell pd_update() whether this is a single-step SIGTRAP so it can
+     skip the expensive pthdb_session_update() and sync_threadlists()
+     calls.  Breakpoints, real signals and other stop kinds always get
+     the full sync.  */
+  bool step_stop = (status->kind () == TARGET_WAITKIND_STOPPED
+		    && status->sig () == GDB_SIGNAL_TRAP);
+  return pd_update (ptid.pid (), step_stop);
Doesn't a breakpoint stop also arrive as TARGET_WAITKIND_STOPPED with GDB_SIGNAL_TRAP?
get_signaled_thread() already looks for the thread stopped on SIGTRAP to find the
current thread, and that has to work at breakpoint stops too, so I would expect 
a breakpoint hit to look the same as a single-step here.
If so, breakpoint stops also get step_stop = true and skip the sync, which is the
opposite of what the commit message says:
"Any other stop kind (breakpoints, real signals) always does a full sync
because threads may have been created or destroyed."
The last_thread_count < 0 check only covers the first stop after pd_activate().
After that first sync, last_thread_count is >= 0 for the rest of the session 
and need_sync becomes just !step_stop.  So from the second stop onwards 
a breakpoint hit skips the sync, and any threads created since the previous
stop are missed.
The benchmark won't show this: all 20 threads are created before do_steps()
and there is only one breakpoint, so the thread list never changes after the
first stop.
resume() is already told whether GDB asked for a single step, and it already
has the aix_thread_variables pointer in hand.  Could you save the flag there
and use it here instead of inferring it from the signal?
/* aix_thread_target::resume (), after the existing
data = get_thread_data_helper_for_ptid (ptid); */
data->last_resume_step = step;
/* in wait () */
bool step_stop = (data->last_resume_step 
&& status->kind () == TARGET_WAITKIND_STOPPED
&& status->sig () == GDB_SIGNAL_TRAP);
That would also handle "next" over a function call, where GDB puts a temporary
breakpoint at the return address and continues rather than single-stepping
through the callee.  resume() is called with step = 0 in that case, so the
stop would correctly get a full sync even though the user typed "next".

Regards,
Abhay
 }
 
 /* Supply AIX altivec registers, both 64 and 32 bit.  */
--------------sUNVgJP2m6u6fraFyiERWSuy--