From a66f72981979a1bda60805b8554e0c78c4a39a21 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 21 Jun 2021 14:26:36 -0400 Subject: 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/thread.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) (limited to 'gdb/thread.c') diff --git a/gdb/thread.c b/gdb/thread.c index 289d33c..74d6b90 100644 --- a/gdb/thread.c +++ b/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. */ @@ -296,12 +307,38 @@ 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) { gdb_assert (!this->has_pending_waitstatus ()); 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; } -- cgit v1.1