gdb: maintain per-process-target list of resumed threads with pending wait status

Looking up threads that are both resumed and have a pending wait
status to report is something that we do quite often in the fast path
and is expensive if there are many threads, since it currently requires
walking whole thread lists.

The first instance is in maybe_set_commit_resumed_all_targets.  This is
called after handling each event in fetch_inferior_event, to see if we
should ask targets to commit their resumed threads or not.  If at least
one thread is resumed but has a pending wait status, we don't ask the
targets to commit their resumed threads, because we want to consume and
handle the pending wait status first.

The second instance is in random_pending_event_thread, where we want to
select a random thread among all those that are resumed and have a
pending wait status.  This is called every time we try to consume
events, to see if there are any pending events that we we want to
consume, before asking the targets for more events.

To allow optimizing these cases, maintain a per-process-target list of
threads that are resumed and have a pending wait status.

In maybe_set_commit_resumed_all_targets, we'll be able to check in O(1)
if there are any such threads simply by checking whether the list is
empty.

In random_pending_event_thread, we'll be able to use that list, which
will be quicker than iterating the list of threads, especially when
there are no resumed with pending wait status threads.

About implementation details: using the new setters on class
thread_info, it's relatively easy to maintain that list.  Any time the
"resumed" or "pending wait status" property is changed, we check whether
that should cause the thread to be added or removed from the list.

In set_thread_exited, we try to remove the thread from the list, because
keeping an exited thread in that list would make no sense (especially if
the thread is freed).  My first implementation assumed that a process
stratum target was always present when set_thread_exited is called.
That's however, not the case: in some cases, targets unpush themselves
from an inferior and then call "exit_inferior", which exits all the
threads.  If the target is unpushed before set_thread_exited is called
on the threads, it means we could mistakenly leave some threads in the
list.  I tried to see how hard it would be to make it such that targets
have to exit all threads before unpushing themselves from the inferior
(that would seem logical to me, we don't want threads belonging to an
inferior that has no process target).  That seemed quite difficult and
not worth the time at the moment.  Instead, I changed
inferior::unpush_target to remove all threads of that inferior from the
list.

As of this patch, the list is not used, this is done in the subsequent
patches.

The debug messages in process-stratum-target.c need to print some ptids.
However, they can't use target_pid_to_str to print them without
introducing a dependency on the current inferior (the current inferior
is used to get the current target stack).  For debug messages, I find it
clearer to print the spelled out ptid anyway (the pid, lwp and tid
values).  Add a ptid_t::to_string method that returns a string
representation of the ptid that is meant for debug messages, a bit like
we already have frame_id::to_string.

Change-Id: Iad8f93db2d13984dd5aa5867db940ed1169dbb67

gdb/gdbthread.h

@@ -296,8 +296,7 @@ class thread_info : public refcounted_object,
   bool resumed () const
   { return m_resumed; }
 
-  void set_resumed (bool resumed)
-  { m_resumed = resumed; }
+  void set_resumed (bool resumed);
 
   /* Frontend view of the thread state.  Note that the THREAD_RUNNING/
      THREAD_STOPPED states are different from EXECUTING.  When the
@@ -470,6 +469,12 @@ class thread_info : public refcounted_object,
      linked.  */
   intrusive_list_node<thread_info> step_over_list_node;
 
+  /* Node for list of threads that are resumed and have a pending wait status.
+
+     The list head for this is in process_stratum_target, hence all threads in
+     this list belong to that process target.  */
+  intrusive_list_node<thread_info> resumed_with_pending_wait_status_node;
+
   /* Displaced-step state for this thread.  */
   displaced_step_thread_state displaced_step_state;
 
@@ -488,6 +493,13 @@ class thread_info : public refcounted_object,
   thread_suspend_state m_suspend;
 };
 
+using thread_info_resumed_with_pending_wait_status_node
+  = intrusive_member_node<thread_info,
+			  &thread_info::resumed_with_pending_wait_status_node>;
+using thread_info_resumed_with_pending_wait_status_list
+  = intrusive_list<thread_info,
+		   thread_info_resumed_with_pending_wait_status_node>;
+
 /* A gdb::ref_ptr pointer to a thread_info.  */
 
 using thread_info_ref

gdb/inferior.c

@@ -89,6 +89,27 @@ inferior::inferior (int pid_)
   m_target_stack.push (get_dummy_target ());
 }
 
+/* See inferior.h.  */
+
+int
+inferior::unpush_target (struct target_ops *t)
+{
+  /* If unpushing the process stratum target from the inferior while threads
+     exist in the inferior, ensure that we don't leave any threads of the
+     inferior in the target's "resumed with pending wait status" list.
+
+     See also the comment in set_thread_exited.  */
+  if (t->stratum () == process_stratum)
+    {
+      process_stratum_target *proc_target = as_process_stratum_target (t);
+
+      for (thread_info *thread : this->non_exited_threads ())
+	proc_target->maybe_remove_resumed_with_pending_wait_status (thread);
+    }
+
+  return m_target_stack.unpush (t);
+}
+
 void
 inferior::set_tty (const char *terminal_name)
 {

gdb/inferior.h

@@ -362,8 +362,7 @@ class inferior : public refcounted_object,
   }
 
   /* Unpush T from this inferior's target stack.  */
-  int unpush_target (struct target_ops *t)
-  { return m_target_stack.unpush (t); }
+  int unpush_target (struct target_ops *t);
 
   /* Returns true if T is pushed in this inferior's target stack.  */
   bool target_is_pushed (target_ops *t)

gdb/process-stratum-target.c

@@ -106,6 +106,40 @@ process_stratum_target::follow_exec (inferior *follow_inf, ptid_t ptid,
 
 /* See process-stratum-target.h.  */
 
+void
+process_stratum_target::maybe_add_resumed_with_pending_wait_status
+  (thread_info *thread)
+{
+  gdb_assert (!thread->resumed_with_pending_wait_status_node.is_linked ());
+
+  if (thread->resumed () && thread->has_pending_waitstatus ())
+    {
+      infrun_debug_printf ("adding to resumed threads with event list: %s",
+			   thread->ptid.to_string ().c_str ());
+      m_resumed_with_pending_wait_status.push_back (*thread);
+    }
+}
+
+/* See process-stratum-target.h.  */
+
+void
+process_stratum_target::maybe_remove_resumed_with_pending_wait_status
+  (thread_info *thread)
+{
+  if (thread->resumed () && thread->has_pending_waitstatus ())
+    {
+      infrun_debug_printf ("removing from resumed threads with event list: %s",
+			   thread->ptid.to_string ().c_str ());
+      gdb_assert (thread->resumed_with_pending_wait_status_node.is_linked ());
+      auto it = m_resumed_with_pending_wait_status.iterator_to (*thread);
+      m_resumed_with_pending_wait_status.erase (it);
+    }
+  else
+    gdb_assert (!thread->resumed_with_pending_wait_status_node.is_linked ());
+}
+
+/* See process-stratum-target.h.  */
+
 std::set<process_stratum_target *>
 all_non_exited_process_targets ()
 {

gdb/process-stratum-target.h

@@ -22,6 +22,8 @@
 
 #include "target.h"
 #include <set>
+#include "gdbsupport/intrusive_list.h"
+#include "gdbthread.h"
 
 /* Abstract base class inherited by all process_stratum targets.  */
 
@@ -78,6 +80,14 @@ class process_stratum_target : public target_ops
      may have spawned new threads we haven't heard of yet.  */
   bool threads_executing = false;
 
+  /* If THREAD is resumed and has a pending wait status, add it to the
+     target's "resumed with pending wait status" list.  */
+  void maybe_add_resumed_with_pending_wait_status (thread_info *thread);
+
+  /* If THREAD is resumed and has a pending wait status, remove it from the
+     target's "resumed with pending wait status" list.  */
+  void maybe_remove_resumed_with_pending_wait_status (thread_info *thread);
+
   /* The connection number.  Visible in "info connections".  */
   int connection_number = 0;
 
@@ -112,6 +122,17 @@ class process_stratum_target : public target_ops
      coalesce multiple resumption requests in a single vCont
      packet.  */
   bool commit_resumed_state = false;
+
+private:
+  /* List of threads managed by this target which simultaneously are resumed
+     and have a pending wait status.
+
+     This is done for optimization reasons, it would be possible to walk the
+     inferior thread lists to find these threads.  But since this is something
+     we need to do quite frequently in the hot path, maintaining this list
+     avoids walking the thread lists repeatedly.  */
+  thread_info_resumed_with_pending_wait_status_list
+    m_resumed_with_pending_wait_status;
 };
 
 /* Downcast TARGET to process_stratum_target.  */

gdb/thread.c

@@ -188,6 +188,17 @@ set_thread_exited (thread_info *tp, bool silent)
 
   if (tp->state != THREAD_EXITED)
     {
+      process_stratum_target *proc_target = tp->inf->process_target ();
+
+      /* Some targets unpush themselves from the inferior's target stack before
+         clearing the inferior's thread list (which marks all threads as exited,
+         and therefore leads to this function).  In this case, the inferior's
+         process target will be nullptr when we arrive here.
+
+         See also the comment in inferior::unpush_target.  */
+      if (proc_target != nullptr)
+	proc_target->maybe_remove_resumed_with_pending_wait_status (tp);
+
       gdb::observers::thread_exit.notify (tp, silent);
 
       /* Tag it as exited.  */
@@ -295,6 +306,29 @@ thread_info::deletable () const
 
 /* See gdbthread.h.  */
 
+void
+thread_info::set_resumed (bool resumed)
+{
+  if (resumed == m_resumed)
+    return;
+
+  process_stratum_target *proc_target = this->inf->process_target ();
+
+  /* If we transition from resumed to not resumed, we might need to remove
+     the thread from the resumed threads with pending statuses list.  */
+  if (!resumed)
+    proc_target->maybe_remove_resumed_with_pending_wait_status (this);
+
+  m_resumed = resumed;
+
+  /* If we transition from not resumed to resumed, we might need to add
+     the thread to the resumed threads with pending statuses list.  */
+  if (resumed)
+    proc_target->maybe_add_resumed_with_pending_wait_status (this);
+}
+
+/* See gdbthread.h.  */
+
 void
 thread_info::set_pending_waitstatus (const target_waitstatus &ws)
 {
@@ -302,6 +336,9 @@ thread_info::set_pending_waitstatus (const target_waitstatus &ws)
 
   m_suspend.waitstatus = ws;
   m_suspend.waitstatus_pending_p = 1;
+
+  process_stratum_target *proc_target = this->inf->process_target ();
+  proc_target->maybe_add_resumed_with_pending_wait_status (this);
 }
 
 /* See gdbthread.h.  */
@@ -311,6 +348,9 @@ thread_info::clear_pending_waitstatus ()
 {
   gdb_assert (this->has_pending_waitstatus ());
 
+  process_stratum_target *proc_target = this->inf->process_target ();
+  proc_target->maybe_remove_resumed_with_pending_wait_status (this);
+
   m_suspend.waitstatus_pending_p = 0;
 }
 

gdbsupport/ptid.cc

@@ -24,3 +24,11 @@
 
 ptid_t const null_ptid = ptid_t::make_null ();
 ptid_t const minus_one_ptid = ptid_t::make_minus_one ();
+
+/* See ptid.h.  */
+
+std::string
+ptid_t::to_string () const
+{
+  return string_printf ("%d.%ld.%ld", m_pid, m_lwp, m_tid);
+}

gdbsupport/ptid.h

@@ -33,6 +33,7 @@
 */
 
 #include <functional>
+#include <string>
 
 class ptid_t
 {
@@ -124,6 +125,12 @@ class ptid_t
 	    || *this == filter);
   }
 
+  /* Return a string representation of the ptid.
+
+     This is only meant to be used in debug messages.  */
+
+  std::string to_string () const;
+
   /* Make a null ptid.  */
 
   static constexpr ptid_t make_null ()