Bug 979293 - Don't write collision bits in HashTable unnecessarily. r=luke.

This avoids no-op writes to the keyHash of entries when doing a no-add-lookup.
This removes a genuine data race in JSRuntime::permanentAtoms, which receives
frequent no-add-lookups from multiple threads after JSRuntime initialization
without any kind of locking.
This commit is contained in:
Nicholas Nethercote 2015-02-25 10:39:46 -08:00
parent 6ea96662a5
commit 3543c6b95e

View File

@ -732,7 +732,6 @@ class HashTableEntry
void removeLive() { MOZ_ASSERT(isLive()); keyHash = sRemovedKey; mem.addr()->~T(); }
bool isLive() const { return isLiveHash(keyHash); }
void setCollision() { MOZ_ASSERT(isLive()); keyHash |= sCollisionBit; }
void setCollision(HashNumber bit) { MOZ_ASSERT(isLive()); keyHash |= bit; }
void unsetCollision() { keyHash &= ~sCollisionBit; }
bool hasCollision() const { return keyHash & sCollisionBit; }
bool matchHash(HashNumber hn) { return (keyHash & ~sCollisionBit) == hn; }
@ -1026,6 +1025,8 @@ class HashTable : private AllocPolicy
#ifdef JS_DEBUG
uint64_t mutationCount;
mutable bool mEntered;
// Note that some updates to these stats are not thread-safe. See the
// comment on the three-argument overloading of HashTable::lookup().
mutable struct Stats
{
uint32_t searches; // total number of table searches
@ -1223,6 +1224,11 @@ class HashTable : private AllocPolicy
return HashPolicy::match(HashPolicy::getKey(e.get()), l);
}
// Warning: in order for readonlyThreadsafeLookup() to be safe this
// function must not modify the table in any way when |collisionBit| is 0.
// (The use of the METER() macro to increment stats violates this
// restriction but we will live with that for now because it's enabled so
// rarely.)
Entry &lookup(const Lookup &l, HashNumber keyHash, unsigned collisionBit) const
{
MOZ_ASSERT(isLiveHash(keyHash));
@ -1253,12 +1259,13 @@ class HashTable : private AllocPolicy
// Save the first removed entry pointer so we can recycle later.
Entry *firstRemoved = nullptr;
while(true) {
while (true) {
if (MOZ_UNLIKELY(entry->isRemoved())) {
if (!firstRemoved)
firstRemoved = entry;
} else {
entry->setCollision(collisionBit);
if (collisionBit == sCollisionBit)
entry->setCollision();
}
METER(stats.steps++);
@ -1304,7 +1311,7 @@ class HashTable : private AllocPolicy
// Collision: double hash.
DoubleHash dh = hash2(keyHash);
while(true) {
while (true) {
MOZ_ASSERT(!entry->isRemoved());
entry->setCollision();