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).
This commit is contained in:
trav90 2018-08-18 15:18:52 -05:00 committed by Roy Tam
commit 6b97ee8cab

View file

@ -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