Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
To: gdb-patches@sourceware.org
Subject: [PATCH 09/11] gdb: optimize selection of resumed thread with pending event
Date: Tue, 22 Jun 2021 12:57:02 -0400	[thread overview]
Message-ID: <20210622165704.2404007-10-simon.marchi@polymtl.ca> (raw)
In-Reply-To: <20210622165704.2404007-1-simon.marchi@polymtl.ca>

Consider a case where many threads (thousands) keep hitting a breakpoint
whose condition evaluates to false.  random_pending_event_thread is
responsible for selecting a thread from an inferior among all that are
resumed with a pending wait status.  It is currently implemented by
walking the inferior's thread list twice: once to count the number of
candidates and once to select a random one.

Since we now maintain a per target list of resumed threads with pending
event, we can implement this more efficiently by walking that list and
selecting the first thread that matches the criteria
(random_pending_event_thread looks for an thread from a specific
inferior, and possibly a filter ptid).  It will be faster especially in
the common case where there isn't any resumed thread with pending
event.  Currently, we have to iterate the thread list to figure this
out.  With this patch, the list of resumed threads with pending event
will be empty, so it's quick to figure out.

The random selection is kept, but is moved to
process_stratum_target::random_resumed_with_pending_wait_status.  The
same technique is used: do a first pass to count the number of
candidates, and do a second pass to select a random one.  But given that
the list of resumed threads with pending wait statuses will generally be
short, or at least shorter than the full thread list, it should be
quicker.

Note that this isn't completely true, in case there are multiple
inferiors on the same target.  Imagine that inferior A has 10k resumed
threads with pending wait statuses, and random_pending_event_thread is
called with inferior B.  We'll need to go through the list that contains
inferior A's threads to realize that inferior B has no resumed threads
with pending wait status.  But I think that this is a corner /
pathological case.  And a possible fix for this situation would be to
make random_pending_event_thread work per-process-target, rather than
per-inferior.

gdb/ChangeLog:

	* process-stratum-target.h (class process_stratum_target)
	<random_resumed_with_pending_wait_status>: New.
	* process-stratum-target.c
	(process_stratum_target::random_resumed_with_pending_wait_status):
	New.
	* infrun.c (random_pending_event_thread): Use
	random_resumed_with_pending_wait_status.

Change-Id: I1b71d01beaa500a148b5b9797745103e13917325
---
 gdb/infrun.c                 | 40 +++++++++------------------------
 gdb/process-stratum-target.c | 43 ++++++++++++++++++++++++++++++++++++
 gdb/process-stratum-target.h |  5 +++++
 3 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 657f97e23fbb..80834fed1e3b 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3493,39 +3493,21 @@ print_target_wait_results (ptid_t waiton_ptid, ptid_t result_ptid,
 static struct thread_info *
 random_pending_event_thread (inferior *inf, ptid_t waiton_ptid)
 {
-  int num_events = 0;
+  process_stratum_target *proc_target = inf->process_target ();
+  thread_info *thread
+    = proc_target->random_resumed_with_pending_wait_status (inf, waiton_ptid);
 
-  auto has_event = [&] (thread_info *tp)
+  if (thread == nullptr)
     {
-      return (tp->ptid.matches (waiton_ptid)
-	      && tp->resumed ()
-	      && tp->has_pending_waitstatus ());
-    };
-
-  /* First see how many events we have.  Count only resumed threads
-     that have an event pending.  */
-  for (thread_info *tp : inf->non_exited_threads ())
-    if (has_event (tp))
-      num_events++;
-
-  if (num_events == 0)
-    return NULL;
-
-  /* Now randomly pick a thread out of those that have had events.  */
-  int random_selector = (int) ((num_events * (double) rand ())
-			       / (RAND_MAX + 1.0));
-
-  if (num_events > 1)
-    infrun_debug_printf ("Found %d events, selecting #%d",
-			 num_events, random_selector);
+      infrun_debug_printf ("None found.");
+      return nullptr;
+    }
 
-  /* Select the Nth thread that has had an event.  */
-  for (thread_info *tp : inf->non_exited_threads ())
-    if (has_event (tp))
-      if (random_selector-- == 0)
-	return tp;
+  infrun_debug_printf ("Found %s.", target_pid_to_str (thread->ptid).c_str ());
+  gdb_assert (thread->resumed ());
+  gdb_assert (thread->has_pending_waitstatus ());
 
-  gdb_assert_not_reached ("event thread not found");
+  return thread;
 }
 
 /* Wrapper for target_wait that first checks whether threads have
diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c
index 44e138273b2f..c828da0f3bca 100644
--- a/gdb/process-stratum-target.c
+++ b/gdb/process-stratum-target.c
@@ -20,6 +20,7 @@
 #include "defs.h"
 #include "process-stratum-target.h"
 #include "inferior.h"
+#include <algorithm>
 
 process_stratum_target::~process_stratum_target ()
 {
@@ -140,6 +141,48 @@ process_stratum_target::maybe_remove_resumed_with_pending_wait_status
 
 /* See process-stratum-target.h.  */
 
+thread_info *
+process_stratum_target::random_resumed_with_pending_wait_status
+  (inferior *inf, ptid_t filter_ptid)
+{
+  auto matches = [inf, filter_ptid] (const thread_info &thread)
+    {
+      return thread.inf == inf && thread.ptid.matches (filter_ptid);
+    };
+
+  /* First see how many matching events we have.  */
+  const auto &l = m_resumed_with_pending_wait_status;
+  unsigned int count = std::count_if (l.begin (), l.end (), matches);
+
+  if (count == 0)
+    return nullptr;
+
+  /* Now randomly pick a thread out of those that match the criteria.  */
+  int random_selector
+    = (int) ((count * (double) rand ()) / (RAND_MAX + 1.0));
+
+  if (count > 1)
+    infrun_debug_printf ("Found %u events, selecting #%d",
+			 count, random_selector);
+
+  /* Select the Nth thread that matches.  */
+  auto it = std::find_if (l.begin (), l.end (),
+			  [&random_selector, &matches]
+			  (const thread_info &thread)
+    {
+      if (!matches (thread))
+	return false;
+
+      return random_selector-- == 0;
+    });
+
+  gdb_assert (it != l.end ());
+
+  return &*it;
+}
+
+/* See process-stratum-target.h.  */
+
 std::set<process_stratum_target *>
 all_non_exited_process_targets ()
 {
diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h
index c73207e0531e..4d1d14f7a94f 100644
--- a/gdb/process-stratum-target.h
+++ b/gdb/process-stratum-target.h
@@ -93,6 +93,11 @@ class process_stratum_target : public target_ops
   bool has_resumed_with_pending_wait_status () const
   { return !m_resumed_with_pending_wait_status.empty (); }
 
+  /* Return a random resumed thread with pending wait status belonging to INF
+     and matching FILTER_PTID.  */
+  thread_info *random_resumed_with_pending_wait_status
+    (inferior *inf, ptid_t filter_ptid);
+
   /* The connection number.  Visible in "info connections".  */
   int connection_number = 0;
 
-- 
2.32.0


  parent reply	other threads:[~2021-06-22 17:02 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-22 16:56 [PATCH 00/11] Various thread lists optimizations Simon Marchi via Gdb-patches
2021-06-22 16:56 ` [PATCH 01/11] gdb: introduce iterator_range, remove next_adapter Simon Marchi via Gdb-patches
2021-07-05 15:41   ` Pedro Alves
2021-07-06 19:16     ` Simon Marchi via Gdb-patches
2021-06-22 16:56 ` [PATCH 02/11] gdb: introduce intrusive_list, make thread_info use it Simon Marchi via Gdb-patches
2021-06-22 23:13   ` Lancelot SIX via Gdb-patches
2021-06-23  0:48     ` Simon Marchi via Gdb-patches
2021-07-05 15:44   ` Pedro Alves
2021-07-06 19:38     ` Simon Marchi via Gdb-patches
2021-07-06 20:45       ` Simon Marchi via Gdb-patches
2021-07-06 21:04         ` Pedro Alves
2021-07-06 21:38           ` Simon Marchi via Gdb-patches
2021-07-06 21:02       ` Pedro Alves
2021-07-06 21:45         ` Simon Marchi via Gdb-patches
2021-07-07 11:46           ` Pedro Alves
2021-07-07 13:52             ` Simon Marchi via Gdb-patches
2021-06-22 16:56 ` [PATCH 03/11] gdb: make inferior_list use intrusive_list Simon Marchi via Gdb-patches
2021-07-05 15:44   ` Pedro Alves
2021-07-14  6:34     ` Tom de Vries
2021-07-14 16:11       ` Simon Marchi via Gdb-patches
2021-07-14 20:15         ` [PATCH] gdb: make all_inferiors_safe actually work Simon Marchi via Gdb-patches
2021-07-15 10:15           ` Tom de Vries
2021-07-17 12:54             ` Simon Marchi via Gdb-patches
2021-06-22 16:56 ` [PATCH 04/11] gdb: use intrusive list for step-over chain Simon Marchi via Gdb-patches
2021-07-05 15:45   ` Pedro Alves
2021-07-06 20:59     ` Simon Marchi via Gdb-patches
2021-06-22 16:56 ` [PATCH 05/11] gdb: add setter / getter for thread_info resumed state Simon Marchi via Gdb-patches
2021-07-05 15:45   ` Pedro Alves
2021-06-22 16:56 ` [PATCH 06/11] gdb: make thread_info::suspend private, add getters / setters Simon Marchi via Gdb-patches
2021-07-05 15:45   ` Pedro Alves
2021-06-22 16:57 ` [PATCH 07/11] gdb: maintain per-process-target list of resumed threads with pending wait status Simon Marchi via Gdb-patches
2021-07-05 15:51   ` Pedro Alves
2021-07-06 21:25     ` Simon Marchi via Gdb-patches
2021-07-07 12:01       ` Pedro Alves
2021-07-12 22:28         ` Simon Marchi via Gdb-patches
2021-07-12 22:34           ` Simon Marchi via Gdb-patches
2021-07-13 12:21           ` Pedro Alves
2021-06-22 16:57 ` [PATCH 08/11] gdb: optimize check for resumed threads with pending wait status in maybe_set_commit_resumed_all_targets Simon Marchi via Gdb-patches
2021-07-05 15:51   ` Pedro Alves
2021-06-22 16:57 ` Simon Marchi via Gdb-patches [this message]
2021-07-05 15:51   ` [PATCH 09/11] gdb: optimize selection of resumed thread with pending event Pedro Alves
2021-06-22 16:57 ` [PATCH 10/11] gdb: maintain ptid -> thread map, optimize find_thread_ptid Simon Marchi via Gdb-patches
2021-07-05 15:52   ` Pedro Alves
2021-07-06 21:31     ` Simon Marchi via Gdb-patches
2021-07-07 12:13       ` Pedro Alves
2021-06-22 16:57 ` [PATCH 11/11] gdb: optimize all_matching_threads_iterator Simon Marchi via Gdb-patches
2021-07-05 15:52   ` Pedro Alves
2021-07-14  9:40     ` Tom de Vries
2021-07-13  0:47 ` [PATCH 00/11] Various thread lists optimizations Simon Marchi via Gdb-patches

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=20210622165704.2404007-10-simon.marchi@polymtl.ca \
    --to=gdb-patches@sourceware.org \
    --cc=simon.marchi@polymtl.ca \
    /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