diff --git a/js/public/HashTable.h b/js/public/HashTable.h index 11b83df83ac..b427240357f 100644 --- a/js/public/HashTable.h +++ b/js/public/HashTable.h @@ -220,10 +220,10 @@ class HashTable : private AllocPolicy JS_ASSERT(&k != &HashPolicy::getKey(this->cur->t)); if (match(*this->cur, l)) return; - typename HashTableEntry::NonConstT t = this->cur->t; - HashPolicy::setKey(t, const_cast(k)); + Entry e = *this->cur; + HashPolicy::setKey(e.t, const_cast(k)); table.remove(*this->cur); - table.putNewInfallible(l, t); + table.add(l, e); added = true; this->validEntry = false; } @@ -234,27 +234,22 @@ class HashTable : private AllocPolicy /* Potentially rehashes the table. */ ~Enum() { - JS_ASSERT(!added); + if (added) + table.checkOverloaded(); if (removed) table.checkUnderloaded(); } - /* - * Can be used to end the enumeration before the destructor. Unlike - * |~Enum()|, this can report OOM on resize, so must be called if - * |rekeyFront()| is used during enumeration. - */ - bool endEnumeration() { + /* Can be used to end the enumeration before the destructor. */ + void endEnumeration() { if (added) { + table.checkOverloaded(); added = false; - if (table.checkOverloaded() == RehashFailed) - return false; } if (removed) { - removed = false; table.checkUnderloaded(); + removed = false; } - return true; } }; @@ -524,7 +519,7 @@ class HashTable : private AllocPolicy Entry *entry = &table[h1]; /* Miss: return space for a new entry. */ - if (!entry->isLive()) { + if (entry->isFree()) { METER(stats.misses++); return *entry; } @@ -540,16 +535,14 @@ class HashTable : private AllocPolicy h1 = applyDoubleHash(h1, dh); entry = &table[h1]; - if (!entry->isLive()) { + if (entry->isFree()) { METER(stats.misses++); return *entry; } } } - enum RebuildStatus { NotOverloaded, Rehashed, RehashFailed }; - - RebuildStatus changeTableSize(int deltaLog2) + bool changeTableSize(int deltaLog2) { /* Look, but don't touch, until we succeed in getting new entry store. */ Entry *oldTable = table; @@ -558,12 +551,12 @@ class HashTable : private AllocPolicy uint32_t newCapacity = JS_BIT(newLog2); if (newCapacity > sMaxCapacity) { this->reportAllocOverflow(); - return RehashFailed; + return false; } Entry *newTable = createTable(*this, newCapacity); if (!newTable) - return RehashFailed; + return false; /* We can't fail from here on, so update table parameters. */ setTableSizeLog2(newLog2); @@ -580,13 +573,32 @@ class HashTable : private AllocPolicy } destroyTable(*this, oldTable, oldCap); - return Rehashed; + return true; } - RebuildStatus checkOverloaded() + void add(const Lookup &l, const Entry &e) + { + JS_ASSERT(table); + + 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()) - return NotOverloaded; + return false; /* Compress if a quarter or more of all entries are removed. */ int deltaLog2; @@ -720,11 +732,8 @@ class HashTable : private AllocPolicy removedCount--; p.keyHash |= sCollisionBit; } else { - /* Preserve the validity of |p.entry|. */ - RebuildStatus status = checkOverloaded(); - if (status == RehashFailed) - return false; - if (status == Rehashed) + if (checkOverloaded()) + /* Preserve the validity of |p.entry|. */ p.entry = &findFreeEntry(p.keyHash); } @@ -755,34 +764,6 @@ class HashTable : private AllocPolicy return true; } - void putNewInfallible(const Lookup &l, const T &t) - { - JS_ASSERT(table); - - HashNumber keyHash = prepareHash(l); - Entry *entry = &findFreeEntry(keyHash); - - if (entry->isRemoved()) { - METER(stats.addOverRemoved++); - removedCount--; - keyHash |= sCollisionBit; - } - - entry->t = t; - entry->setLive(keyHash); - entryCount++; - mutationCount++; - } - - bool putNew(const Lookup &l, const T &t) - { - if (checkOverloaded() == RehashFailed) - return false; - - putNewInfallible(l, t); - return true; - } - bool relookupOrAdd(AddPtr& p, const Lookup &l, const T& t) { p.mutationCount = mutationCount; @@ -1168,7 +1149,9 @@ class HashMap /* Like put, but assert that the given key is not already present. */ bool putNew(const Key &k, const Value &v) { - return impl.putNew(k, Entry(k, v)); + AddPtr p = lookupForAdd(k); + JS_ASSERT(!p); + return add(p, k, v); } /* Add (k,defaultValue) if k no found. Return false-y Ptr on oom. */ @@ -1374,11 +1357,15 @@ class HashSet /* Like put, but assert that the given key is not already present. */ bool putNew(const T &t) { - return impl.putNew(t, t); + AddPtr p = lookupForAdd(t); + JS_ASSERT(!p); + return add(p, t); } bool putNew(const Lookup &l, const T &t) { - return impl.putNew(l, t); + AddPtr p = lookupForAdd(l); + JS_ASSERT(!p); + return add(p, t); } void remove(const Lookup &l) { diff --git a/js/src/jsapi-tests/testHashTable.cpp b/js/src/jsapi-tests/testHashTable.cpp index 58210f86b9b..c3c062cfa01 100644 --- a/js/src/jsapi-tests/testHashTable.cpp +++ b/js/src/jsapi-tests/testHashTable.cpp @@ -193,13 +193,11 @@ BEGIN_TEST(testHashRekeyManual) CHECK(AddLowKeys(&am, &bm, i)); CHECK(MapsAreEqual(am, bm)); - IntMap::Enum e(am); - for (; !e.empty(); e.popFront()) { + 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(e.endEnumeration()); CHECK(SlowRekey(&bm)); CHECK(MapsAreEqual(am, bm)); @@ -217,13 +215,11 @@ BEGIN_TEST(testHashRekeyManual) CHECK(AddLowKeys(&as, &bs, i)); CHECK(SetsAreEqual(as, bs)); - IntSet::Enum e(as); - for (; !e.empty(); e.popFront()) { + for (IntSet::Enum e(as); !e.empty(); e.popFront()) { uint32_t tmp = LowToHigh::rekey(e.front()); if (tmp != e.front()) e.rekeyFront(tmp); } - CHECK(e.endEnumeration()); CHECK(SlowRekey(&bs)); CHECK(SetsAreEqual(as, bs)); @@ -247,8 +243,7 @@ BEGIN_TEST(testHashRekeyManualRemoval) CHECK(AddLowKeys(&am, &bm, i)); CHECK(MapsAreEqual(am, bm)); - IntMap::Enum e(am); - for (; !e.empty(); e.popFront()) { + for (IntMap::Enum e(am); !e.empty(); e.popFront()) { if (LowToHighWithRemoval::shouldBeRemoved(e.front().key)) { e.removeFront(); } else { @@ -257,7 +252,6 @@ BEGIN_TEST(testHashRekeyManualRemoval) e.rekeyFront(tmp); } } - CHECK(e.endEnumeration()); CHECK(SlowRekey(&bm)); CHECK(MapsAreEqual(am, bm)); @@ -275,8 +269,7 @@ BEGIN_TEST(testHashRekeyManualRemoval) CHECK(AddLowKeys(&as, &bs, i)); CHECK(SetsAreEqual(as, bs)); - IntSet::Enum e(as); - for (; !e.empty(); e.popFront()) { + for (IntSet::Enum e(as); !e.empty(); e.popFront()) { if (LowToHighWithRemoval::shouldBeRemoved(e.front())) { e.removeFront(); } else { @@ -285,7 +278,6 @@ BEGIN_TEST(testHashRekeyManualRemoval) e.rekeyFront(tmp); } } - CHECK(e.endEnumeration()); CHECK(SlowRekey(&bs)); CHECK(SetsAreEqual(as, bs));