Avoid doing a memset on a non-POD structure

|entryCount| tracks -- in fast-to-check manner -- the number of entries in the hashtable.  But to actually enumerate entries, we have to loop through all of |table|, checking for entries that are actually live.  A live entry is indicated by a zero |hash| in the entry.  The |memset| would properly zero that; removing the memset will not.

It's not entirely clear whether a memset that overwrites a lot of stuff but is maybe simpler, is faster than compiler-generated likely-SIMD code that zeroes out *just* |hash| fields in all the entries.  But I am going to guess that SIMD is good enough.  For now, we should just do the simple and thing: don't distinguish POD and non-POD, and know that the compiler is going to recognize that |mem.addr()->~T()| is a no-op when T is trivial.  So with POD, the loop should degenerate to just zeroing |hash| at consistent offset, and SIMD will eat that up, and it can't be *that* different from the memset in performance (if it is at all).

1 files changed, 33 insertions, 13 deletions

diff --git a/js/public/HashTable.h b/js/public/HashTable.h
index 5d4c0665d7..8a2493b559 100644
--- a/js/public/HashTable.h
+++ b/js/public/HashTable.h
@@ -12,6 +12,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/Casting.h"
 #include "mozilla/HashFunctions.h"
+#include "mozilla/MemoryChecking.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
 #include "mozilla/Opaque.h"
@@ -805,17 +806,22 @@ class HashTableEntry
     void operator=(const HashTableEntry&) = delete;
     ~HashTableEntry() = delete;
 
+    void destroyStoredT() {
+        mem.addr()->~T();
+        MOZ_MAKE_MEM_UNDEFINED(mem.addr(), sizeof(*mem.addr()));
+    }
+
   public:
     // NB: HashTableEntry is treated as a POD: no constructor or destructor calls.
 
     void destroyIfLive() {
         if (isLive())
-            mem.addr()->~T();
+            destroyStoredT();
     }
 
     void destroy() {
         MOZ_ASSERT(isLive());
-        mem.addr()->~T();
+        destroyStoredT();
     }
 
     void swap(HashTableEntry* other) {
@@ -835,10 +841,28 @@ class HashTableEntry
     NonConstT& getMutable() { MOZ_ASSERT(isLive()); return *mem.addr(); }
 
     bool isFree() const    { return keyHash == sFreeKey; }
-    void clearLive()       { MOZ_ASSERT(isLive()); keyHash = sFreeKey; mem.addr()->~T(); }
-    void clear()           { if (isLive()) mem.addr()->~T(); keyHash = sFreeKey; }
+    void clearLive() {
+        MOZ_ASSERT(isLive());
+        keyHash = sFreeKey;
+        destroyStoredT();
+    }
+
+    void clear() {
+        if (isLive())
+            destroyStoredT();
+
+        MOZ_MAKE_MEM_UNDEFINED(this, sizeof(*this));
+        keyHash = sFreeKey;
+    }
+
     bool isRemoved() const { return keyHash == sRemovedKey; }
-    void removeLive()      { MOZ_ASSERT(isLive()); keyHash = sRemovedKey; mem.addr()->~T(); }
+
+    void removeLive() {
+        MOZ_ASSERT(isLive());
+        keyHash = sRemovedKey;
+        destroyStoredT();
+    }
+
     bool isLive() const    { return isLiveHash(keyHash); }
     void setCollision()               { MOZ_ASSERT(isLive()); keyHash |= sCollisionBit; }
     void unsetCollision()             { keyHash &= ~sCollisionBit; }
@@ -1654,14 +1678,10 @@ class HashTable : private AllocPolicy
   public:
     void clear()
     {
-        if (mozilla::IsPod<Entry>::value) {
-            memset(table, 0, sizeof(*table) * capacity());
-        } else {
-            uint32_t tableCapacity = capacity();
-            Entry* end = table + tableCapacity;
-            for (Entry* e = table; e < end; ++e)
-                e->clear();
-        }
+        Entry* end = table + capacity();
+        for (Entry* e = table; e < end; ++e)
+            e->clear();
+
         removedCount = 0;
         entryCount = 0;
 #ifdef JS_DEBUG