Change regcache list to be an hash map

The regcache objects created for threads are stored in a linked list,
regcache::current_regcache.

This may result in bad performance when debugging a lot of threads, as GDB
needs to walk the linked list to fetch a regcache given a ptid.

This patch replaces the linked list with an std::unordered_map, indexed by
(ptid, arch).  The lookups, in get_thread_arch_aspace_regcache, should become
faster when the number of threads grow.  With a small number of threads, it
will probably be a bit slower to do an hash map lookup than to walk a few
linked list nodes,  but it should not be perceptible.

The function registers_changed_ptid deletes all regcaches related to a given
ptid (there could be multiple, with different arches).  Since the hash map is
indexed by (ptid, arch), we can't do a simple lookup, so we fall back on
iterating on all the hash map values.  It should be the same complexity as what
we have today.  This function is called when resuming execution, so it would be
nice to find a more efficient way to do this.

Similarly, the function regcache_thread_ptid_changed is called when a thread
changes ptid, so it needs to look up all regcaches for a given ptid.  This one
also iterates on all the hash map values.  However, it is not called often
(mostly when creating an inferior, on some specific platforms), so it shouldn't
be a problem.

diff --git a/gdb/gdbsupport/ptid.h b/gdb/gdbsupport/ptid.h
index f5625a61387b5bfd44b88f464571b5ace1de921f..d00a0964bb992ad6ac2349b4056cc482ecc11633 100644 (file)
--- a/gdb/gdbsupport/ptid.h
+++ b/gdb/gdbsupport/ptid.h
@@ -32,6 +32,8 @@
    thread_stratum target that might want to sit on top.
 */
 
+#include <functional>
+
 class ptid_t
 {
 public:
@@ -143,6 +145,17 @@ private:
   long m_tid;
 };
 
+struct hash_ptid
+{
+  size_t operator() (const ptid_t &ptid) const
+  {
+    std::hash<long> long_hash;
+    return (long_hash (ptid.pid ())
+           + long_hash (ptid.lwp ())
+           + long_hash (ptid.tid ()));
+  }
+};
+
 /* The null or zero ptid, often used to indicate no process. */
 
 extern const ptid_t null_ptid;

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 6912b14d06e85834c14e55897dd71afdb956f839..adffa415e5bda2c2c5af9327f8a13ce844561957 100644 (file)
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -317,21 +317,58 @@ reg_buffer::assert_regnum (int regnum) const
    recording if the register values have been changed (eg. by the
    user).  Therefore all registers must be written back to the
    target when appropriate.  */
-std::forward_list<regcache *> regcache::current_regcache;
+
+/* Key for the hash map keeping the regcaches.  */
+
+struct ptid_arch
+{
+  ptid_arch (ptid_t ptid, gdbarch *arch)
+    : ptid (ptid), arch (arch)
+  {}
+
+  ptid_t ptid;
+  gdbarch *arch;
+
+  bool operator== (const ptid_arch &other) const
+  {
+    return this->ptid == other.ptid && this->arch == other.arch;
+  }
+};
+
+/* Hash function for ptid_arch.  */
+
+struct hash_ptid_arch
+{
+  size_t operator() (const ptid_arch &val) const
+  {
+    hash_ptid h_ptid;
+    std::hash<long> h_long;
+    return h_ptid (val.ptid) + h_long ((long) val.arch);
+  }
+};
+
+using ptid_arch_regcache_map = std::unordered_map<ptid_arch, regcache *, hash_ptid_arch>;
+
+/* Hash map containing the regcaches.  */
+
+static ptid_arch_regcache_map the_regcaches;
 
 struct regcache *
-get_thread_arch_aspace_regcache (ptid_t ptid, struct gdbarch *gdbarch,
+get_thread_arch_aspace_regcache (ptid_t ptid, gdbarch *arch,
                                 struct address_space *aspace)
 {
-  for (const auto &regcache : regcache::current_regcache)
-    if (regcache->ptid () == ptid && regcache->arch () == gdbarch)
-      return regcache;
-
-  regcache *new_regcache = new regcache (gdbarch, aspace);
+  /* Look up a regcache for this (ptid, arch).  */
+  ptid_arch key (ptid, arch);
+  auto it = the_regcaches.find (key);
+  if (it != the_regcaches.end ())
+    return it->second;
 
-  regcache::current_regcache.push_front (new_regcache);
+  /* It does not exist, create it.  */
+  regcache *new_regcache = new regcache (arch, aspace);
   new_regcache->set_ptid (ptid);
 
+  the_regcaches[key] = new_regcache;
+
   return new_regcache;
 }
 
@@ -390,13 +427,32 @@ regcache_observer_target_changed (struct target_ops *target)
 
 /* Update global variables old ptids to hold NEW_PTID if they were
    holding OLD_PTID.  */
-void
-regcache::regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
+static void
+regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
 {
-  for (auto &regcache : regcache::current_regcache)
+  std::vector<ptid_arch> keys_to_update;
+
+  /* Find all the regcaches to updates.  */
+  for (auto &pair : the_regcaches)
     {
-      if (regcache->ptid () == old_ptid)
-       regcache->set_ptid (new_ptid);
+      regcache *rc = pair.second;
+      if (rc->ptid () == old_ptid)
+       keys_to_update.push_back (pair.first);
+    }
+
+  for (const ptid_arch &old_key : keys_to_update)
+    {
+      /* Get the regcache, delete the hash map entry.  */
+      auto it = the_regcaches.find (old_key);
+      gdb_assert (it != the_regcaches.end ());
+      regcache *rc = it->second;
+
+      the_regcaches.erase (it);
+
+      /* Insert the regcache back, with an updated key.  */
+      ptid_arch new_key (new_ptid, rc->arch ());
+      rc->set_ptid (new_ptid);
+      the_regcaches[new_key] = rc;
     }
 }
 
@@ -414,18 +470,17 @@ regcache::regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
 void
 registers_changed_ptid (ptid_t ptid)
 {
-  for (auto oit = regcache::current_regcache.before_begin (),
-        it = std::next (oit);
-       it != regcache::current_regcache.end ();
-       )
+  for (auto iter = the_regcaches.begin (); iter != the_regcaches.end (); )
     {
-      if ((*it)->ptid ().matches (ptid))
+      regcache *rc = iter->second;
+
+      if (rc->ptid ().matches (ptid))
        {
-         delete *it;
-         it = regcache::current_regcache.erase_after (oit);
+         delete iter->second;
+         iter = the_regcaches.erase (iter);
        }
       else
-       oit = it++;
+       ++iter;
     }
 
   if (current_thread_ptid.matches (ptid))
@@ -1395,8 +1450,7 @@ public:
   static size_t
   current_regcache_size ()
   {
-    return std::distance (regcache::current_regcache.begin (),
-                         regcache::current_regcache.end ());
+    return the_regcaches.size ();
   }
 };
 
@@ -1803,8 +1857,7 @@ _initialize_regcache (void)
     = gdbarch_data_register_post_init (init_regcache_descr);
 
   gdb::observers::target_changed.attach (regcache_observer_target_changed);
-  gdb::observers::thread_ptid_changed.attach
-    (regcache::regcache_thread_ptid_changed);
+  gdb::observers::thread_ptid_changed.attach (regcache_thread_ptid_changed);
 
   add_com ("flushregs", class_maintenance, reg_flush_command,
           _("Force gdb to flush its register cache (maintainer command)."));

diff --git a/gdb/regcache.h b/gdb/regcache.h
index f7d4bc60f65133ff0c735b847f1b26cb15c31a62..f8af38cf97fd86de7e267e222aeb02331b4a8d42 100644 (file)
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -389,12 +389,9 @@ public:
    debug.  */
   void debug_print_register (const char *func, int regno);
 
-  static void regcache_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid);
 protected:
   regcache (gdbarch *gdbarch, const address_space *aspace_);
 
-  static std::forward_list<regcache *> current_regcache;
-
 private:
 
   /* Helper function for transfer_regset.  Copies across a single register.  */

diff --git a/gdb/thread-map.h b/gdb/thread-map.h
index 2b41b7361ce62b741fd416069b1ec50bf8882346..44cccc16c4c5dc560c9e66919b8231cf26d1fc05 100644 (file)
--- a/gdb/thread-map.h
+++ b/gdb/thread-map.h
@@ -25,15 +25,6 @@
 
 struct thread_info;
 
-struct hash_ptid
-{
-  size_t operator() (const ptid_t &ptid) const
-  {
-    std::hash<long> long_hash;
-    return long_hash(ptid.pid()) + long_hash(ptid.lwp()) + long_hash(ptid.tid());
-  }
-};
-
 using ptid_thread_map = std::unordered_map<ptid_t, thread_info *, hash_ptid>;
 
 struct all_thread_map_range_iterator