From 922cc93d5da6a6dc422b7e7a09ee745414d67457 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 11 Jun 2021 18:29:33 -0400 Subject: [PATCH] gdb: maintain ptid -> thread map, optimize find_thread_ptid When debugging a large number of threads (thousands), looking up a thread by ptid_t using the inferior::thread_list linked list can add up. Add inferior::thread_map, an std::unordered_map indexed by ptid_t, and change the find_thread_ptid function to look up a thread using std::unordered_map::find, instead of iterating on all of the inferior's threads. This should make it faster to look up a thread from its ptid. Change-Id: I3a8da0a839e18dee5bb98b8b7dbeb7f3dfa8ae1c Co-Authored-By: Pedro Alves --- gdb/inferior.c | 1 + gdb/inferior.h | 6 ++++++ gdb/infrun.c | 10 ++++++++++ gdb/regcache.c | 5 +++++ gdb/scoped-mock-context.h | 1 + gdb/thread.c | 29 ++++++++++++++++++++++++----- 6 files changed, 47 insertions(+), 5 deletions(-) diff --git a/gdb/inferior.c b/gdb/inferior.c index 3c569f4f04f..74b1c65fa3d 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -186,6 +186,7 @@ inferior::clear_thread_list (bool silent) if (thr->deletable ()) delete thr; }); + ptid_thread_map.clear (); } void diff --git a/gdb/inferior.h b/gdb/inferior.h index 2bfe29afed3..6662a3bde46 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -63,6 +63,8 @@ struct thread_info; #include "process-stratum-target.h" #include "displaced-stepping.h" +#include + struct infcall_suspend_state; struct infcall_control_state; @@ -391,6 +393,10 @@ public: /* This inferior's thread list, sorted by creation order. */ intrusive_list 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 ptid_thread_map; + /* Returns a range adapter covering the inferior's threads, including exited threads. Used like this: diff --git a/gdb/infrun.c b/gdb/infrun.c index 93cb55e70c7..8eb8413f883 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -9418,8 +9418,13 @@ infrun_thread_ptid_changed () target1.mock_inferior.pid = old_ptid.pid (); target1.mock_thread.ptid = old_ptid; + target1.mock_inferior.ptid_thread_map.clear (); + target1.mock_inferior.ptid_thread_map[old_ptid] = &target1.mock_thread; + target2.mock_inferior.pid = old_ptid.pid (); target2.mock_thread.ptid = old_ptid; + target2.mock_inferior.ptid_thread_map.clear (); + target2.mock_inferior.ptid_thread_map[old_ptid] = &target2.mock_thread; auto restore_inferior_ptid = make_scoped_restore (&inferior_ptid, old_ptid); set_current_inferior (&target1.mock_inferior); @@ -9442,8 +9447,13 @@ infrun_thread_ptid_changed () target1.mock_inferior.pid = old_ptid.pid (); target1.mock_thread.ptid = old_ptid; + target1.mock_inferior.ptid_thread_map.clear (); + target1.mock_inferior.ptid_thread_map[old_ptid] = &target1.mock_thread; + target2.mock_inferior.pid = old_ptid.pid (); target2.mock_thread.ptid = old_ptid; + target2.mock_inferior.ptid_thread_map.clear (); + target2.mock_inferior.ptid_thread_map[old_ptid] = &target2.mock_thread; auto restore_inferior_ptid = make_scoped_restore (&inferior_ptid, old_ptid); set_current_inferior (&target2.mock_inferior); diff --git a/gdb/regcache.c b/gdb/regcache.c index 21fa25d3155..ac44d714ddc 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -2044,8 +2044,13 @@ regcache_thread_ptid_changed () target1.mock_inferior.pid = old_ptid.pid (); target1.mock_thread.ptid = old_ptid; + target1.mock_inferior.ptid_thread_map.clear (); + target1.mock_inferior.ptid_thread_map[old_ptid] = &target1.mock_thread; + target2.mock_inferior.pid = old_ptid.pid (); target2.mock_thread.ptid = old_ptid; + target2.mock_inferior.ptid_thread_map.clear (); + target2.mock_inferior.ptid_thread_map[old_ptid] = &target2.mock_thread; gdb_assert (regcaches.empty ()); diff --git a/gdb/scoped-mock-context.h b/gdb/scoped-mock-context.h index ba3b81ed12a..48fdbacbb14 100644 --- a/gdb/scoped-mock-context.h +++ b/gdb/scoped-mock-context.h @@ -51,6 +51,7 @@ struct scoped_mock_context inferior_list.push_back (mock_inferior); mock_inferior.thread_list.push_back (mock_thread); + mock_inferior.ptid_thread_map[mock_ptid] = &mock_thread; mock_inferior.gdbarch = gdbarch; mock_inferior.aspace = mock_pspace.aspace; mock_inferior.pspace = &mock_pspace; diff --git a/gdb/thread.c b/gdb/thread.c index 74d6b90923f..8f0584e5238 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -206,6 +206,14 @@ set_thread_exited (thread_info *tp, bool silent) /* Clear breakpoints, etc. associated with this thread. */ clear_thread_inferior_resources (tp); + + /* Remove from the ptid_t map. We don't want for + find_thread_ptid to find exited threads. Also, the target + may reuse the ptid for a new thread, and there can only be + one value per key; adding a new thread with the same ptid_t + would overwrite the exited thread's ptid entry. */ + size_t nr_deleted = tp->inf->ptid_thread_map.erase (tp->ptid); + gdb_assert (nr_deleted == 1); } } @@ -228,6 +236,11 @@ new_thread (struct inferior *inf, ptid_t ptid) inf->thread_list.push_back (*tp); + /* A thread with this ptid should not exist in the map yet. */ + gdb_assert (inf->ptid_thread_map.find (ptid) == inf->ptid_thread_map.end ()); + + inf->ptid_thread_map[ptid] = tp; + return tp; } @@ -484,11 +497,11 @@ find_thread_ptid (inferior *inf, ptid_t ptid) { gdb_assert (inf != nullptr); - for (thread_info *tp : inf->non_exited_threads ()) - if (tp->ptid == ptid) - return tp; - - return NULL; + auto it = inf->ptid_thread_map.find (ptid); + if (it != inf->ptid_thread_map.end ()) + return it->second; + else + return nullptr; } /* See gdbthread.h. */ @@ -758,7 +771,13 @@ thread_change_ptid (process_stratum_target *targ, inf->pid = new_ptid.pid (); tp = find_thread_ptid (inf, old_ptid); + gdb_assert (tp != nullptr); + + int num_erased = inf->ptid_thread_map.erase (old_ptid); + gdb_assert (num_erased == 1); + tp->ptid = new_ptid; + inf->ptid_thread_map[new_ptid] = tp; gdb::observers::thread_ptid_changed.notify (targ, old_ptid, new_ptid); }