diff --git a/js/public/HashTable.h b/js/public/HashTable.h index 48cb77757448..e505b43d953e 100644 --- a/js/public/HashTable.h +++ b/js/public/HashTable.h @@ -96,7 +96,7 @@ class HashTableEntry { void setCollision(HashNumber collisionBit) { JS_ASSERT(isLive()); keyHash |= collisionBit; } - void unsetCollision() { JS_ASSERT(isLive()); keyHash &= ~sCollisionBit; } + void unsetCollision() { keyHash &= ~sCollisionBit; } bool hasCollision() const { JS_ASSERT(isLive()); return keyHash & sCollisionBit; } bool matchHash(HashNumber hn) { return (keyHash & ~sCollisionBit) == hn; } HashNumber getKeyHash() const { JS_ASSERT(!hasCollision()); return keyHash; } @@ -158,14 +158,9 @@ class HashTable : private AllocPolicy { friend class HashTable; HashNumber keyHash; -#ifdef DEBUG - uint64_t mutationCount; + DebugOnly mutationCount; - AddPtr(Entry &entry, HashNumber hn, uint64_t mutationCount) - : Ptr(entry), keyHash(hn), mutationCount(mutationCount) {} -#else AddPtr(Entry &entry, HashNumber hn) : Ptr(entry), keyHash(hn) {} -#endif public: /* Leaves AddPtr uninitialized. */ AddPtr() {} @@ -222,6 +217,7 @@ class HashTable : private AllocPolicy friend class HashTable; HashTable &table; + bool added; bool removed; /* Not copyable. */ @@ -230,7 +226,7 @@ class HashTable : private AllocPolicy public: template explicit - Enum(Map &map) : Range(map.all()), table(map.impl), removed(false) {} + Enum(Map &map) : Range(map.all()), table(map.impl), added(false), removed(false) {} /* * Removes the |front()| element from the table, leaving |front()| @@ -246,14 +242,35 @@ class HashTable : private AllocPolicy removed = true; } + /* + * Removes the |front()| element and re-inserts it into the table with + * a new key at the new Lookup position. |front()| is invalid after + * this operation until the next call to |popFront()|. + */ + void rekeyFront(Key &k) { + JS_ASSERT(&k != &HashPolicy::getKey(this->cur->t)); + JS_ASSERT(!table.match(*this->cur, k)); + Entry e = *this->cur; + HashPolicy::setKey(e.t, k); + table.remove(*this->cur); + table.add(k, e); + added = true; + } + /* Potentially rehashes the table. */ ~Enum() { + if (added) + table.checkOverloaded(); if (removed) table.checkUnderloaded(); } /* Can be used to end the enumeration before the destructor. */ void endEnumeration() { + if (added) { + table.checkOverloaded(); + added = false; + } if (removed) { table.checkUnderloaded(); removed = false; @@ -290,11 +307,9 @@ class HashTable : private AllocPolicy # define METER(x) #endif -#ifdef DEBUG friend class js::ReentrancyGuard; - mutable bool entered; - uint64_t mutationCount; -#endif + mutable DebugOnly entered; + DebugOnly mutationCount; /* The default initial capacity is 16, but you can ask for as small as 4. */ static const unsigned sMinSizeLog2 = 2; @@ -364,11 +379,9 @@ class HashTable : private AllocPolicy entryCount(0), gen(0), removedCount(0), - table(NULL) -#ifdef DEBUG - , entered(false), + table(NULL), + entered(false), mutationCount(0) -#endif {} bool init(uint32_t length) @@ -424,8 +437,22 @@ class HashTable : private AllocPolicy return hash0 >> shift; } - static HashNumber hash2(HashNumber hash0, uint32_t log2, uint32_t shift) { - return ((hash0 << log2) >> shift) | 1; + struct DoubleHash { + HashNumber h2; + HashNumber sizeMask; + }; + + DoubleHash hash2(HashNumber curKeyHash, uint32_t hashShift) const { + unsigned sizeLog2 = sHashBits - hashShift; + DoubleHash dh = { + ((curKeyHash << sizeLog2) >> hashShift) | 1, + (HashNumber(1) << sizeLog2) - 1 + }; + return dh; + } + + static HashNumber applyDoubleHash(HashNumber h1, const DoubleHash &dh) { + return (h1 - dh.h2) & dh.sizeMask; } bool overloaded() { @@ -467,9 +494,7 @@ class HashTable : private AllocPolicy } /* Collision: double hash. */ - unsigned sizeLog2 = sHashBits - hashShift; - HashNumber h2 = hash2(keyHash, sizeLog2, hashShift); - HashNumber sizeMask = (HashNumber(1) << sizeLog2) - 1; + DoubleHash dh = hash2(keyHash, hashShift); /* Save the first removed entry pointer so we can recycle later. */ Entry *firstRemoved = NULL; @@ -483,8 +508,7 @@ class HashTable : private AllocPolicy } METER(stats.steps++); - h1 -= h2; - h1 &= sizeMask; + h1 = applyDoubleHash(h1, dh); entry = &table[h1]; if (entry->isFree()) { @@ -525,17 +549,14 @@ class HashTable : private AllocPolicy } /* Collision: double hash. */ - unsigned sizeLog2 = sHashBits - hashShift; - HashNumber h2 = hash2(keyHash, sizeLog2, hashShift); - HashNumber sizeMask = (HashNumber(1) << sizeLog2) - 1; + DoubleHash dh = hash2(keyHash, hashShift); while(true) { JS_ASSERT(!entry->isRemoved()); entry->setCollision(); METER(stats.steps++); - h1 -= h2; - h1 &= sizeMask; + h1 = applyDoubleHash(h1, dh); entry = &table[h1]; if (entry->isFree()) { @@ -579,6 +600,44 @@ class HashTable : private AllocPolicy return true; } + void add(const Lookup &l, const Entry &e) + { + HashNumber keyHash = prepareHash(l); + Entry &entry = lookup(l, keyHash, sCollisionBit); + + if (entry.isRemoved()) { + METER(stats.addOverRemoved++); + removedCount--; + keyHash |= sCollisionBit; + } + + entry.t = e.t; + entry.setLive(keyHash); + entryCount++; + mutationCount++; + } + + bool checkOverloaded() + { + if (overloaded()) { + /* Compress if a quarter or more of all entries are removed. */ + int deltaLog2; + if (removedCount >= (capacity() >> 2)) { + METER(stats.compresses++); + deltaLog2 = 0; + } else { + METER(stats.grows++); + deltaLog2 = 1; + } + + (void) changeTableSize(deltaLog2); + + return true; + } + + return false; + } + void remove(Entry &e) { METER(stats.removes++); @@ -590,9 +649,7 @@ class HashTable : private AllocPolicy e.setFree(); } entryCount--; -#ifdef DEBUG mutationCount++; -#endif } void checkUnderloaded() @@ -615,9 +672,7 @@ class HashTable : private AllocPolicy } removedCount = 0; entryCount = 0; -#ifdef DEBUG mutationCount++; -#endif } void finish() @@ -626,15 +681,13 @@ class HashTable : private AllocPolicy if (!table) return; - + destroyTable(*this, table, capacity()); table = NULL; gen++; entryCount = 0; removedCount = 0; -#ifdef DEBUG mutationCount++; -#endif } Range all() const { @@ -675,11 +728,9 @@ class HashTable : private AllocPolicy ReentrancyGuard g(*this); HashNumber keyHash = prepareHash(l); Entry &entry = lookup(l, keyHash, sCollisionBit); -#ifdef DEBUG - return AddPtr(entry, keyHash, mutationCount); -#else - return AddPtr(entry, keyHash); -#endif + AddPtr p(entry, keyHash); + p.mutationCount = mutationCount; + return p; } bool add(AddPtr &p) @@ -699,31 +750,14 @@ class HashTable : private AllocPolicy removedCount--; p.keyHash |= sCollisionBit; } else { - /* If alpha is >= .75, grow or compress the table. */ - if (overloaded()) { - /* Compress if a quarter or more of all entries are removed. */ - int deltaLog2; - if (removedCount >= (capacity() >> 2)) { - METER(stats.compresses++); - deltaLog2 = 0; - } else { - METER(stats.grows++); - deltaLog2 = 1; - } - - if (!changeTableSize(deltaLog2)) - return false; - + if (checkOverloaded()) /* Preserve the validity of |p.entry|. */ p.entry = &findFreeEntry(p.keyHash); - } } p.entry->setLive(p.keyHash); entryCount++; -#ifdef DEBUG mutationCount++; -#endif return true; } @@ -750,9 +784,7 @@ class HashTable : private AllocPolicy bool relookupOrAdd(AddPtr& p, const Lookup &l, const T& t) { -#ifdef DEBUG p.mutationCount = mutationCount; -#endif { ReentrancyGuard g(*this); p.entry = &lookup(l, p.keyHash, sCollisionBit); @@ -767,6 +799,7 @@ class HashTable : private AllocPolicy remove(*p.entry); checkUnderloaded(); } + #undef METER }; @@ -880,7 +913,7 @@ class HashMapEntry template HashMapEntry(const KeyInput &k, const ValueInput &v) : key(k), value(v) {} - HashMapEntry(MoveRef rhs) + HashMapEntry(MoveRef rhs) : key(Move(rhs->key)), value(Move(rhs->value)) { } void operator=(MoveRef rhs) { const_cast(key) = Move(rhs->key); @@ -939,6 +972,7 @@ class HashMap { typedef Key KeyType; static const Key &getKey(Entry &e) { return e.key; } + static void setKey(Entry &e, Key &k) { const_cast(e.key) = k; } }; typedef detail::HashTable Impl; @@ -1068,7 +1102,7 @@ class HashMap return impl.sizeOfExcludingThis(mallocSizeOf); } size_t sizeOfIncludingThis(JSMallocSizeOfFun mallocSizeOf) const { - /* + /* * Don't just call |impl.sizeOfExcludingThis()| because there's no * guarantee that |impl| is the first field in HashMap. */ @@ -1176,6 +1210,7 @@ class HashSet struct SetOps : HashPolicy { typedef T KeyType; static const KeyType &getKey(const T &t) { return t; } + static void setKey(T &t, KeyType &k) { t = k; } }; typedef detail::HashTable Impl; @@ -1278,7 +1313,7 @@ class HashSet return impl.sizeOfExcludingThis(mallocSizeOf); } size_t sizeOfIncludingThis(JSMallocSizeOfFun mallocSizeOf) const { - /* + /* * Don't just call |impl.sizeOfExcludingThis()| because there's no * guarantee that |impl| is the first field in HashSet. */ diff --git a/js/src/jsapi-tests/Makefile.in b/js/src/jsapi-tests/Makefile.in index 9b4d9477e536..aa1840e6aa2e 100644 --- a/js/src/jsapi-tests/Makefile.in +++ b/js/src/jsapi-tests/Makefile.in @@ -69,6 +69,7 @@ CPPSRCS = \ testGCOutOfMemory.cpp \ testOOM.cpp \ testGetPropertyDefault.cpp \ + testHashTable.cpp \ testIndexToString.cpp \ testIntString.cpp \ testIntTypesABI.cpp \ diff --git a/js/src/jsapi-tests/testHashTable.cpp b/js/src/jsapi-tests/testHashTable.cpp new file mode 100644 index 000000000000..56494f8e9351 --- /dev/null +++ b/js/src/jsapi-tests/testHashTable.cpp @@ -0,0 +1,287 @@ +#include "tests.h" + +#include "js/HashTable.h" + +//#define FUZZ + +typedef js::HashMap, js::SystemAllocPolicy> IntMap; +typedef js::HashSet, js::SystemAllocPolicy> IntSet; + +/* + * The rekeying test as conducted by adding only keys masked with 0x0000FFFF + * that are unique. We rekey by shifting left 16 bits. + */ +#ifdef FUZZ +const size_t TestSize = 0x0000FFFF / 2; +const size_t TestIterations = SIZE_MAX; +#else +const size_t TestSize = 10000; +const size_t TestIterations = 10; +#endif + +JS_STATIC_ASSERT(TestSize <= 0x0000FFFF / 2); + +struct LowToHigh +{ + static uint32_t rekey(uint32_t initial) { + if (initial > uint32_t(0x0000FFFF)) + return initial; + return initial << 16; + } + + static bool shouldBeRemoved(uint32_t initial) { + return false; + } +}; + +struct LowToHighWithRemoval +{ + static uint32_t rekey(uint32_t initial) { + if (initial > uint32_t(0x0000FFFF)) + return initial; + return initial << 16; + } + + static bool shouldBeRemoved(uint32_t initial) { + if (initial >= 0x00010000) + return (initial >> 16) % 2 == 0; + return initial % 2 == 0; + } +}; + +static bool +MapsAreEqual(IntMap &am, IntMap &bm) +{ + bool equal = true; + if (am.count() != bm.count()) { + equal = false; + fprintf(stderr, "A.count() == %u and B.count() == %u\n", am.count(), bm.count()); + } + for (IntMap::Range r = am.all(); !r.empty(); r.popFront()) { + if (!bm.has(r.front().key)) { + equal = false; + fprintf(stderr, "B does not have %x which is in A\n", r.front().key); + } + } + for (IntMap::Range r = bm.all(); !r.empty(); r.popFront()) { + if (!am.has(r.front().key)) { + equal = false; + fprintf(stderr, "A does not have %x which is in B\n", r.front().key); + } + } + return equal; +} + +static bool +SetsAreEqual(IntSet &am, IntSet &bm) +{ + bool equal = true; + if (am.count() != bm.count()) { + equal = false; + fprintf(stderr, "A.count() == %u and B.count() == %u\n", am.count(), bm.count()); + } + for (IntSet::Range r = am.all(); !r.empty(); r.popFront()) { + if (!bm.has(r.front())) { + equal = false; + fprintf(stderr, "B does not have %x which is in A\n", r.front()); + } + } + for (IntSet::Range r = bm.all(); !r.empty(); r.popFront()) { + if (!am.has(r.front())) { + equal = false; + fprintf(stderr, "A does not have %x which is in B\n", r.front()); + } + } + return equal; +} + +static bool +AddLowKeys(IntMap *am, IntMap *bm, int seed) +{ + size_t i = 0; + srand(seed); + while (i < TestSize) { + uint32_t n = rand() & 0x0000FFFF; + if (!am->has(n)) { + if (bm->has(n)) + return false; + am->putNew(n, n); + bm->putNew(n, n); + i++; + } + } + return true; +} + +static bool +AddLowKeys(IntSet *as, IntSet *bs, int seed) +{ + size_t i = 0; + srand(seed); + while (i < TestSize) { + uint32_t n = rand() & 0x0000FFFF; + if (!as->has(n)) { + if (bs->has(n)) + return false; + as->putNew(n); + bs->putNew(n); + i++; + } + } + return true; +} + +template +static bool +SlowRekey(IntMap *m) { + IntMap tmp; + tmp.init(); + + for (IntMap::Range r = m->all(); !r.empty(); r.popFront()) { + if (NewKeyFunction::shouldBeRemoved(r.front().key)) + continue; + uint32_t hi = NewKeyFunction::rekey(r.front().key); + if (tmp.has(hi)) + return false; + tmp.putNew(hi, r.front().value); + } + + m->clear(); + for (IntMap::Range r = tmp.all(); !r.empty(); r.popFront()) { + m->putNew(r.front().key, r.front().value); + } + + return true; +} + +template +static bool +SlowRekey(IntSet *s) { + IntSet tmp; + tmp.init(); + + for (IntSet::Range r = s->all(); !r.empty(); r.popFront()) { + if (NewKeyFunction::shouldBeRemoved(r.front())) + continue; + uint32_t hi = NewKeyFunction::rekey(r.front()); + if (tmp.has(hi)) + return false; + tmp.putNew(hi); + } + + s->clear(); + for (IntSet::Range r = tmp.all(); !r.empty(); r.popFront()) { + s->putNew(r.front()); + } + + return true; +} + +BEGIN_TEST(testHashRekeyManual) +{ + IntMap am, bm; + am.init(); + bm.init(); + for (size_t i = 0; i < TestIterations; ++i) { +#ifdef FUZZ + fprintf(stderr, "map1: %lu\n", i); +#endif + CHECK(AddLowKeys(&am, &bm, i)); + CHECK(MapsAreEqual(am, bm)); + + for (IntMap::Enum e(am); !e.empty(); e.popFront()) { + uint32_t tmp = LowToHigh::rekey(e.front().key); + if (tmp != e.front().key) + e.rekeyFront(tmp); + } + CHECK(SlowRekey(&bm)); + + CHECK(MapsAreEqual(am, bm)); + am.clear(); + bm.clear(); + } + + IntSet as, bs; + as.init(); + bs.init(); + for (size_t i = 0; i < TestIterations; ++i) { +#ifdef FUZZ + fprintf(stderr, "set1: %lu\n", i); +#endif + CHECK(AddLowKeys(&as, &bs, i)); + CHECK(SetsAreEqual(as, bs)); + + for (IntSet::Enum e(as); !e.empty(); e.popFront()) { + uint32_t tmp = LowToHigh::rekey(e.front()); + if (tmp != e.front()) + e.rekeyFront(tmp); + } + CHECK(SlowRekey(&bs)); + + CHECK(SetsAreEqual(as, bs)); + as.clear(); + bs.clear(); + } + + return true; +} +END_TEST(testHashRekeyManual) + +BEGIN_TEST(testHashRekeyManualRemoval) +{ + IntMap am, bm; + am.init(); + bm.init(); + for (size_t i = 0; i < TestIterations; ++i) { +#ifdef FUZZ + fprintf(stderr, "map2: %lu\n", i); +#endif + CHECK(AddLowKeys(&am, &bm, i)); + CHECK(MapsAreEqual(am, bm)); + + for (IntMap::Enum e(am); !e.empty(); e.popFront()) { + if (LowToHighWithRemoval::shouldBeRemoved(e.front().key)) { + e.removeFront(); + } else { + uint32_t tmp = LowToHighWithRemoval::rekey(e.front().key); + if (tmp != e.front().key) + e.rekeyFront(tmp); + } + } + CHECK(SlowRekey(&bm)); + + CHECK(MapsAreEqual(am, bm)); + am.clear(); + bm.clear(); + } + + IntSet as, bs; + as.init(); + bs.init(); + for (size_t i = 0; i < TestIterations; ++i) { +#ifdef FUZZ + fprintf(stderr, "set1: %lu\n", i); +#endif + CHECK(AddLowKeys(&as, &bs, i)); + CHECK(SetsAreEqual(as, bs)); + + for (IntSet::Enum e(as); !e.empty(); e.popFront()) { + if (LowToHighWithRemoval::shouldBeRemoved(e.front())) { + e.removeFront(); + } else { + uint32_t tmp = LowToHighWithRemoval::rekey(e.front()); + if (tmp != e.front()) + e.rekeyFront(tmp); + } + } + CHECK(SlowRekey(&bs)); + + CHECK(SetsAreEqual(as, bs)); + as.clear(); + bs.clear(); + } + + return true; +} +END_TEST(testHashRekeyManualRemoval) + diff --git a/js/src/jsopcode.cpp b/js/src/jsopcode.cpp index 3fee6cfd801d..e0cc5317bdeb 100644 --- a/js/src/jsopcode.cpp +++ b/js/src/jsopcode.cpp @@ -2530,15 +2530,6 @@ InitSprintStack(JSContext *cx, SprintStack *ss, JSPrinter *jp, unsigned depth) return JS_TRUE; } -template -void -Swap(T &a, T &b) -{ - T tmp = a; - a = b; - b = tmp; -} - /* * If nb is non-negative, decompile nb bytecodes starting at pc. Otherwise * the decompiler starts at pc and continues until it reaches an opcode for diff --git a/js/src/jsutil.h b/js/src/jsutil.h index c20fbf143332..d5007e50c301 100644 --- a/js/src/jsutil.h +++ b/js/src/jsutil.h @@ -273,6 +273,15 @@ PodEqual(T *one, T *two, size_t len) return !memcmp(one, two, len * sizeof(T)); } +template +JS_ALWAYS_INLINE static void +Swap(T &t, T &u) +{ + T tmp(Move(t)); + t = Move(u); + u = Move(tmp); +} + JS_ALWAYS_INLINE static size_t UnsignedPtrDiff(const void *bigger, const void *smaller) {