From 450eed08c5eb7de7928e185feaf79f0b74b932e5 Mon Sep 17 00:00:00 2001 From: trav90 Date: Sat, 18 Aug 2018 15:18:52 -0500 Subject: 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). --- js/public/HashTable.h | 46 +++++++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 13 deletions(-) (limited to 'js/public') diff --git a/js/public/HashTable.h b/js/public/HashTable.h index 5d4c0665d..8a2493b55 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::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 -- cgit v1.2.3