Bug 1303174 - Correct gdb nsTHashtable pretty printer's understanding of table capacity to not be the entry count. r=nfroyd
authorAndrew Sutherland <asutherland@asutherland.org>
Fri, 16 Sep 2016 10:18:29 -0400
changeset 340042 f0283d52d9caa7e58aa3b7c5a3dc25da3e80ca58
parent 340041 caa8a9c7d1223eda20bb5fa91d57750d5e1ba6e3
child 340043 fb3c27c05f398f8be9f6b4989613317ce00a5f51
push id10033
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:50:26 +0000
treeherdermozilla-aurora@5dddbefdf759 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs1303174
milestone51.0a1
Bug 1303174 - Correct gdb nsTHashtable pretty printer's understanding of table capacity to not be the entry count. r=nfroyd It turns out the entry count is not the same as the capacity. This turns out badly when your hashtable is a hashtable and not an array. Which is most of them. If you were using this pretty printer prior to this fix, you may have seen anywhere between 0% and 100% of the actual entries in the table. Apologies! Note that this change has no bearing on any in-tree tests at this point. We still want to add unit tests so this is not the case. (The JS pretty printers have them.)
python/gdbpp/gdbpp/thashtable.py
--- a/python/gdbpp/gdbpp/thashtable.py
+++ b/python/gdbpp/gdbpp/thashtable.py
@@ -91,36 +91,52 @@ class thashtable_printer(object):
             if f.name == 'mKeyHash' or f.name == 'mData':
                 continue
             # ...and assume the first one we find is the key.
             self.key_field_name = f.name
             break
 
     def children(self):
         table = self.value['mTable']
-        # Number of entries
+
+        # mEntryCount is the number of occupied slots/entries in the table.
+        # We can use this to avoid doing wasted memory reads.
         entryCount = table['mEntryCount']
+        if entryCount == 0:
+            return
+
+        # The table capacity is tracked "cleverly" in terms of how many bits
+        # the hash needs to be shifted.  CapacityFromHashShift calculates the
+        # actual entry capacity via ((uint32_t)1 << (kHashBits - mHashShift));
+        capacity = 1 << (table['kHashBits'] - table['mHashShift'])
+
         # Pierce generation-tracking EntryStore class to get at buffer.  The
         # class instance always exists, but this char* may be null.
         store = table['mEntryStore']['mEntryStore']
 
         key_field_name = self.key_field_name
 
+        seenCount = 0
         pEntry = store.cast(self.entry_type.pointer())
-        for i in range(0, int(entryCount)):
+        for i in range(0, int(capacity)):
             entry = (pEntry + i).dereference()
             # An mKeyHash of 0 means empty, 1 means deleted sentinel, so skip
             # if that's the case.
             if entry['mKeyHash'] <= 1:
                 continue
 
             yield ('%d' % i, entry[key_field_name])
             if self.is_table:
                 yield ('%d' % i, entry['mData'])
 
+            # Stop iterating if we know there are no more occupied slots.
+            seenCount += 1
+            if seenCount >= entryCount:
+                break
+
     def to_string(self):
         # The most specific template type is the most interesting.
         return str(self.outermost_type)
 
     def display_hint(self):
         if self.is_table:
             return 'map'
         else: