From patchwork Tue Oct 22 15:02:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rohr, Stephan" X-Patchwork-Id: 99331 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9EFE23858408 for ; Tue, 22 Oct 2024 15:03:21 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) by sourceware.org (Postfix) with ESMTPS id 7CE433858D20 for ; Tue, 22 Oct 2024 15:02:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7CE433858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=intel.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7CE433858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=198.175.65.10 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729609362; cv=none; b=V+c8glFx0gJgY6O+JkVuUyxodVdejxUixq0ztRsqj91jTWXsmVsjSAOFsI+DrA45pGYUyWp4NEHOz1eo6vX4vbGqvPM0745fFtFv1hbhBmNu2XauQI3XhhF25bP2Z0+tkPP6hvGuiAqWEZkEz7gzaQa05sAOsyQ/JdZ1ug4ApKM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1729609362; c=relaxed/simple; bh=+s14zKWoPOahN699+WWkMCOocJZygsaOTAeNu8AcSf4=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=VbTymKKEB+ULvhz3wbF9z7H8MqfrK8RmIcyg1tB4ax/1RwDd3IOyIi0iLS9K0u/vd+wCjnu0ri5fSRAOAQYjAQ4VcGh4lYP8UJMO4m0owt1OsMeHuicLksrCsWZvERtIgr/BAB5GqMo5kgDTAlY8VvgqpziQa6nexiLGYhw6ajw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1729609355; x=1761145355; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=+s14zKWoPOahN699+WWkMCOocJZygsaOTAeNu8AcSf4=; b=DsYr/ZFb2TMfekBVCYc8sgLyQLLt2SgyY2WCJwiilKNTO1EZOysdrDf3 AjoQ39tzj/Bzu2wV+6ohIsfXnSiJg0GFLK33EJOhzKbOyQo55tvRBT0R3 im8uouSQ1RF3GLQ0rOisLC0viSYkSyjQUHeTIw2G51JPS3xaJ5xSqvT7M VhLbVsFS7rAf9lPZeTSBLm/Ft0ltOy98BC25dzyQqcXCGP4nlQFRf9VGw 2AFgKxIa1R0sWKNYT0ktAKr4XumJRU7KSoM+LjN8pwwyOyxlFDYKjuqn+ rGNDTFeiMROP3Te7fYKx29/9+vGdu3Z3p7uPwuTpLjUjs256epPmxE39u g==; X-CSE-ConnectionGUID: CiZa80DwRyeukYC+TPvS0w== X-CSE-MsgGUID: qpe0XGWoSWmEhkJhbQZqzA== X-IronPort-AV: E=McAfee;i="6700,10204,11222"; a="46615052" X-IronPort-AV: E=Sophos;i="6.11,199,1725346800"; d="scan'208";a="46615052" Received: from fmviesa004.fm.intel.com ([10.60.135.144]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Oct 2024 08:02:34 -0700 X-CSE-ConnectionGUID: T0nBgACOTAaqGmKvGFbnuA== X-CSE-MsgGUID: Elv6m6XDSAarfZsw2B8o4A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,223,1725346800"; d="scan'208";a="84510172" Received: from 984fee0031af.jf.intel.com (HELO localhost) ([10.165.54.65]) by fmviesa004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Oct 2024 08:02:33 -0700 From: Stephan Rohr To: gdb-patches@sourceware.org Cc: simark@simark.ca, aburgess@redhat.com Subject: [PATCH v4 2/2] gdbserver: add process specific thread list and map Date: Tue, 22 Oct 2024 08:02:21 -0700 Message-Id: <20241022150221.2125847-3-stephan.rohr@intel.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20241022150221.2125847-1-stephan.rohr@intel.com> References: <20241022150221.2125847-1-stephan.rohr@intel.com> MIME-Version: 1.0 X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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~patchwork=sourceware.org@sourceware.org Replace the servers global thread list with a process specific thread list and a ptid -> thread map similar to 'inferior::ptid_thread_map' on GDB side. Optimize the 'find_thread' and 'find_thread_ptid' functions to use std::unordered_map::find for faster lookup of threads without iterating over all processes and threads, if applicable. This becomes important when debugging applications with a large thread count, e.g., in the context of GPU debugging. --- gdbserver/gdbthread.h | 24 ++++-- gdbserver/inferiors.cc | 173 ++++++++++++++++++++++++++++++++--------- gdbserver/inferiors.h | 41 +++++++++- gdbserver/server.cc | 54 +++++++++++-- 4 files changed, 237 insertions(+), 55 deletions(-) diff --git a/gdbserver/gdbthread.h b/gdbserver/gdbthread.h index 1c380d5ff22..8e317724b4d 100644 --- a/gdbserver/gdbthread.h +++ b/gdbserver/gdbthread.h @@ -21,7 +21,7 @@ #include "gdbsupport/common-gdbthread.h" #include "gdbsupport/function-view.h" -#include "gdbsupport/owning_intrusive_list.h" +#include "gdbsupport/intrusive_list.h" struct btrace_target_info; struct regcache; @@ -84,8 +84,6 @@ struct thread_info : public intrusive_list_node gdb_thread_options thread_options = 0; }; -extern owning_intrusive_list all_threads; - void remove_thread (struct thread_info *thread); struct thread_info *add_thread (ptid_t ptid, void *target_data); @@ -97,10 +95,11 @@ struct thread_info *find_thread_ptid (ptid_t ptid); /* Find any thread of the PID process. Returns NULL if none is found. */ + struct thread_info *find_any_thread_of_pid (int pid); -/* Find the first thread for which FUNC returns true. Return NULL if no thread - satisfying FUNC is found. */ +/* Find the first thread for which FUNC returns true. Return NULL if + no thread satisfying FUNC is found. */ thread_info * find_thread (gdb::function_view func); @@ -126,12 +125,23 @@ for_each_thread (gdb::function_view func); void for_each_thread (int pid, gdb::function_view func); -/* Find the a random thread for which FUNC (THREAD) returns true. If - no entry is found then return NULL. */ +/* Like the above, but only consider threads matching PTID. */ + +void +for_each_thread (ptid_t ptid, gdb::function_view func); + +/* Find a random thread that matches PTID and FUNC (THREAD) + returns true. If no entry is found then return NULL. */ thread_info * find_thread_in_random (gdb::function_view func); +/* Like the above, but only consider threads matching PTID. */ + +thread_info * +find_thread_in_random (ptid_t ptid, + gdb::function_view func); + /* Get current thread ID (Linux task ID). */ #define current_ptid (current_thread->id) diff --git a/gdbserver/inferiors.cc b/gdbserver/inferiors.cc index 1d39a8d1a86..c862e9653bb 100644 --- a/gdbserver/inferiors.cc +++ b/gdbserver/inferiors.cc @@ -24,7 +24,6 @@ #include "dll.h" owning_intrusive_list all_processes; -owning_intrusive_list all_threads; /* The current process. */ static process_info *current_process_; @@ -41,7 +40,15 @@ static std::string current_inferior_cwd; struct thread_info * add_thread (ptid_t thread_id, void *target_data) { - auto &new_thread = all_threads.emplace_back (thread_id, target_data); + process_info *process = find_process_pid (thread_id.pid ()); + gdb_assert (process != nullptr); + + auto &new_thread = process->thread_list ().emplace_back (thread_id, target_data); + auto map_elem = + process->thread_map ().insert ({thread_id, &new_thread}); + + /* A thread with this ptid should not exist in the map yet. */ + gdb_assert (map_elem.second); if (current_thread == NULL) switch_to_thread (&new_thread); @@ -54,18 +61,26 @@ add_thread (ptid_t thread_id, void *target_data) struct thread_info * get_first_thread (void) { - if (!all_threads.empty ()) - return &(all_threads.front ()); - else - return NULL; + return find_thread ([] (thread_info *thread) + { + return true; + }); } struct thread_info * find_thread_ptid (ptid_t ptid) { - return find_thread ([&] (thread_info *thread) { - return thread->id == ptid; - }); + process_info *process = find_process_pid (ptid.pid ()); + if (process == nullptr) + return nullptr; + + auto &thread_map = process->thread_map (); + + if (auto it = thread_map.find (ptid); + it != thread_map.end ()) + return it->second; + + return nullptr; } /* Find a thread associated with the given PROCESS, or NULL if no @@ -94,7 +109,16 @@ remove_thread (struct thread_info *thread) target_disable_btrace (thread->btrace); discard_queued_stop_replies (ptid_of (thread)); - all_threads.erase (all_threads.iterator_to (*thread)); + process_info *process = get_thread_process (thread); + gdb_assert (process != nullptr); + + /* We should not try to remove a thread that was not added. */ + int num_erased = process->thread_map ().erase (thread->id); + gdb_assert (num_erased > 0); + + process->thread_list ().erase ( + process->thread_list ().iterator_to (*thread)); + if (current_thread == thread) switch_to_thread (nullptr); } @@ -120,9 +144,11 @@ set_thread_regcache_data (struct thread_info *thread, struct regcache *data) void clear_inferiors (void) { - all_threads.clear (); - - clear_dlls (); + for_each_process ([&] (process_info *process) + { + process->thread_list ().clear (); + process->thread_map ().clear (); + }); switch_to_thread (nullptr); current_process_ = nullptr; @@ -230,14 +256,30 @@ find_process (gdb::function_view func) return NULL; } +/* See inferiors.h. */ + +thread_info * +process_info::find_thread (gdb::function_view func) +{ + for (thread_info &thread : m_thread_list) + { + if (func (&thread)) + return &thread; + } + return nullptr; +} + /* See gdbthread.h. */ thread_info * find_thread (gdb::function_view func) { - for (thread_info &thread : all_threads) - if (func (&thread)) - return &thread; + for (process_info &process : all_processes) + { + thread_info *thread = process.find_thread (func); + if (thread != nullptr) + return thread; + } return NULL; } @@ -247,10 +289,11 @@ find_thread (gdb::function_view func) thread_info * find_thread (int pid, gdb::function_view func) { - return find_thread ([&] (thread_info *thread) - { - return thread->id.pid () == pid && func (thread); - }); + process_info *process = find_process_pid (pid); + if (process == nullptr) + return nullptr; + + return process->find_thread (func); } /* See gdbthread.h. */ @@ -258,9 +301,23 @@ find_thread (int pid, gdb::function_view func) thread_info * find_thread (ptid_t filter, gdb::function_view func) { - return find_thread ([&] (thread_info *thread) { - return thread->id.matches (filter) && func (thread); - }); + if (filter == minus_one_ptid) + return find_thread (func); + + process_info *process = find_process_pid (filter.pid ()); + if (process == nullptr) + return nullptr; + + if (filter.is_pid ()) + return process->find_thread (func); + + auto &thread_map = process->thread_map (); + + if (auto it = thread_map.find (filter); + it != thread_map.end () && func (it->second)) + return it->second; + + return nullptr; } /* See gdbthread.h. */ @@ -268,10 +325,23 @@ find_thread (ptid_t filter, gdb::function_view func) void for_each_thread (gdb::function_view func) { - owning_intrusive_list::iterator next, cur = all_threads.begin (); + for_each_process ([&] (process_info *proc) + { + proc->for_each_thread (func); + }); +} + +/* See inferiors.h. */ + +void +process_info::for_each_thread (gdb::function_view func) +{ + owning_intrusive_list::iterator next, cur + = m_thread_list.begin (); - while (cur != all_threads.end ()) + while (cur != m_thread_list.end ()) { + /* FUNC may alter the current iterator. */ next = cur; next++; func (&*cur); @@ -284,43 +354,70 @@ for_each_thread (gdb::function_view func) void for_each_thread (int pid, gdb::function_view func) { - for_each_thread ([&] (thread_info *thread) - { - if (pid == thread->id.pid ()) + process_info *process = find_process_pid (pid); + if (process == nullptr) + return; + + process->for_each_thread (func); +} + +/* See gdbthread.h. */ + +void +for_each_thread (ptid_t ptid, gdb::function_view func) +{ + if (ptid == minus_one_ptid) + for_each_thread (func); + else if (ptid.is_pid ()) + for_each_thread (ptid.pid (), func); + else + find_thread (ptid, [func] (thread_info *thread) + { func (thread); - }); + return false; + }); } /* See gdbthread.h. */ thread_info * -find_thread_in_random (gdb::function_view func) +find_thread_in_random (ptid_t ptid, + gdb::function_view func) { int count = 0; int random_selector; /* First count how many interesting entries we have. */ - for_each_thread ([&] (thread_info *thread) { - if (func (thread)) - count++; - }); + for_each_thread (ptid, [&] (thread_info *thread) + { + if (func (thread)) + count++; + }); if (count == 0) - return NULL; + return nullptr; /* Now randomly pick an entry out of those. */ random_selector = (int) ((count * (double) rand ()) / (RAND_MAX + 1.0)); - thread_info *thread = find_thread ([&] (thread_info *thr_arg) { - return func (thr_arg) && (random_selector-- == 0); - }); + thread_info *thread = find_thread (ptid, [&] (thread_info *thr_arg) + { + return func (thr_arg) && (random_selector-- == 0); + }); gdb_assert (thread != NULL); return thread; } +/* See gdbthread.h. */ + +thread_info * +find_thread_in_random (gdb::function_view func) +{ + return find_thread_in_random (minus_one_ptid, func); +} /* See gdbsupport/common-gdbthread.h. */ diff --git a/gdbserver/inferiors.h b/gdbserver/inferiors.h index 3f2c5c70a39..21bafde1fcd 100644 --- a/gdbserver/inferiors.h +++ b/gdbserver/inferiors.h @@ -24,6 +24,8 @@ #include "dll.h" +#include + struct thread_info; struct regcache; struct target_desc; @@ -32,9 +34,13 @@ struct breakpoint; struct raw_breakpoint; struct fast_tracepoint_jump; struct process_info_private; +struct process_info; + +extern owning_intrusive_list all_processes; -struct process_info : public intrusive_list_node +class process_info : public intrusive_list_node { +public: process_info (int pid_, int attached_) : pid (pid_), attached (attached_) {} @@ -83,6 +89,33 @@ struct process_info : public intrusive_list_node not access inferior memory or registers, as we haven't determined the target architecture/description. */ bool starting_up = false; + + /* Return a reference to the private thread list. */ + owning_intrusive_list &thread_list () + { + return m_thread_list; + } + + /* Return a reference to the private thread map. */ + std::unordered_map &thread_map () + { + return m_ptid_thread_map; + } + + /* Find the first thread for which FUNC returns true. Return NULL if no + such thread is found. */ + thread_info *find_thread (gdb::function_view func); + + /* Invoke FUNC for each thread. */ + void for_each_thread (gdb::function_view func); + +private: + /* This processes' thread list, sorted by creation order. */ + owning_intrusive_list m_thread_list; + + /* A map of ptid_t to thread_info*, for average O(1) ptid_t lookup. + Exited threads do not appear in the map. */ + std::unordered_map m_ptid_thread_map; }; /* Get the pid of PROC. */ @@ -93,9 +126,9 @@ pid_of (const process_info *proc) return proc->pid; } -/* Return a pointer to the process that corresponds to the current - thread (current_thread). It is an error to call this if there is - no current thread selected. */ +/* Return a pointer to the current process. Note that the current + process may be non-null while the current thread (current_thread) + is null. */ struct process_info *current_process (void); struct process_info *get_thread_process (const struct thread_info *); diff --git a/gdbserver/server.cc b/gdbserver/server.cc index 5058428f58b..3086f8b31a1 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -1355,7 +1355,7 @@ handle_detach (char *own_buf) another process might delete the next thread in the iteration, which is the one saved by the safe iterator. We will never delete the currently iterated on thread, so standard iteration should be safe. */ - for (thread_info &thread : all_threads) + for (thread_info &thread : process->thread_list ()) { /* Only threads that are of the process we are detaching. */ if (thread.id.pid () != pid) @@ -2523,8 +2523,48 @@ static void handle_query (char *own_buf, int packet_len, int *new_packet_len_p) { client_state &cs = get_client_state (); + static owning_intrusive_list::iterator process_iter; static owning_intrusive_list::iterator thread_iter; + auto init_thread_iter = [&] () + { + process_iter = all_processes.begin (); + owning_intrusive_list *thread_list; + + for (; process_iter != all_processes.end (); ++process_iter) + { + thread_list = &(process_iter->thread_list ()); + thread_iter = thread_list->begin (); + if (thread_iter != thread_list->end ()) + break; + } + /* Make sure that there is at least one thread to iterate. */ + gdb_assert (process_iter != all_processes.end ()); + gdb_assert (thread_iter != thread_list->end ()); + }; + + auto advance_thread_iter = [&] () + { + /* The loop below is written in the natural way as-if we'd always + start at the beginning of the inferior list. This fast forwards + the algorithm to the actual current position. */ + owning_intrusive_list *thread_list + = &(process_iter->thread_list ()); + goto start; + + for (; process_iter != all_processes.end (); ++process_iter) + { + thread_list = &(process_iter->thread_list ()); + thread_iter = thread_list->begin (); + while (thread_iter != thread_list->end ()) + { + return; + start: + ++thread_iter; + } + } + }; + /* Reply the current thread id. */ if (strcmp ("qC", own_buf) == 0 && !disable_packet_qC) { @@ -2535,7 +2575,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) ptid = cs.general_thread; else { - thread_iter = all_threads.begin (); + init_thread_iter (); ptid = thread_iter->id; } @@ -2596,24 +2636,26 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (strcmp ("qfThreadInfo", own_buf) == 0) { require_running_or_return (own_buf); - thread_iter = all_threads.begin (); + init_thread_iter (); *own_buf++ = 'm'; ptid_t ptid = thread_iter->id; write_ptid (own_buf, ptid); - thread_iter++; + advance_thread_iter (); return; } if (strcmp ("qsThreadInfo", own_buf) == 0) { require_running_or_return (own_buf); - if (thread_iter != all_threads.end ()) + /* We're done if the process iterator hit the end of the + process list. */ + if (process_iter != all_processes.end ()) { *own_buf++ = 'm'; ptid_t ptid = thread_iter->id; write_ptid (own_buf, ptid); - thread_iter++; + advance_thread_iter (); return; } else