From b6a6d26147534b428eb5c5c547315520656a7bb0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 18 Jun 2015 22:19:10 -0700 Subject: [PATCH] Bug 1180072 - Remove PL_DHashTableEnumerate(). r=froydnj. It's no longer used, and the Iterator classes are much nicer. Yay. --- xpcom/glue/nsTHashtable.h | 15 ++++++++- xpcom/glue/pldhash.cpp | 70 --------------------------------------- xpcom/glue/pldhash.h | 60 --------------------------------- 3 files changed, 14 insertions(+), 131 deletions(-) diff --git a/xpcom/glue/nsTHashtable.h b/xpcom/glue/nsTHashtable.h index 10eac4c7cb0e..e23b1be09809 100644 --- a/xpcom/glue/nsTHashtable.h +++ b/xpcom/glue/nsTHashtable.h @@ -71,6 +71,16 @@ * @author "Benjamin Smedberg " */ +// These are the codes returned by |Enumerator| functions, which control +// EnumerateEntry()'s behavior. The PLD/PL_D prefix is because they originated +// in PLDHashTable, but that class no longer uses them. +enum PLDHashOperator +{ + PL_DHASH_NEXT = 0, // enumerator says continue + PL_DHASH_STOP = 1, // enumerator says stop + PL_DHASH_REMOVE = 2 // enumerator says remove +}; + template class nsTHashtable { @@ -191,7 +201,10 @@ public: typedef PLDHashOperator (*Enumerator)(EntryType* aEntry, void* userArg); /** - * Enumerate all the entries of the function. + * Enumerate all the entries of the function. If any entries are removed via + * a PL_DHASH_REMOVE return value from |aEnumFunc|, the table may be shrunk + * at the end. Use RawRemoveEntry() instead if you wish to remove an entry + * without possibly shrinking the table. * @param enumFunc the Enumerator function to call * @param userArg a pointer to pass to the * Enumerator function diff --git a/xpcom/glue/pldhash.cpp b/xpcom/glue/pldhash.cpp index e0d5f1592070..24f0c2ada708 100644 --- a/xpcom/glue/pldhash.cpp +++ b/xpcom/glue/pldhash.cpp @@ -728,7 +728,6 @@ PLDHashTable::ShrinkIfAppropriate() uint32_t capacity = Capacity(); if (mRemovedCount >= capacity >> 2 || (capacity > PL_DHASH_MIN_CAPACITY && mEntryCount <= MinLoad(capacity))) { - uint32_t log2; BestCapacity(mEntryCount, &capacity, &log2); @@ -739,75 +738,6 @@ PLDHashTable::ShrinkIfAppropriate() } } -MOZ_ALWAYS_INLINE uint32_t -PLDHashTable::Enumerate(PLDHashEnumerator aEtor, void* aArg) -{ -#ifdef DEBUG - // Enumerate() can be a read operation or a write operation, depending on - // whether PL_DHASH_REMOVE is used. Assume for now it's a read; once the - // enumeration is done we'll know if that is false, and if so, - // retrospectively check it as a write. - bool wasIdleAndWritableAtStart = mChecker.IsIdle() && mChecker.IsWritable(); - AutoReadOp op(mChecker); -#endif - - if (!mEntryStore) { - return 0; - } - - char* entryAddr = mEntryStore; - uint32_t capacity = Capacity(); - uint32_t tableSize = capacity * mEntrySize; - char* entryLimit = mEntryStore + tableSize; - uint32_t i = 0; - bool didRemove = false; - - if (ChaosMode::isActive(ChaosMode::HashTableIteration)) { - // Start iterating at a random point in the hashtable. It would be - // even more chaotic to iterate in fully random order, but that's a lot - // more work. - entryAddr += ChaosMode::randomUint32LessThan(capacity) * mEntrySize; - } - - for (uint32_t e = 0; e < capacity; ++e) { - PLDHashEntryHdr* entry = (PLDHashEntryHdr*)entryAddr; - if (ENTRY_IS_LIVE(entry)) { - PLDHashOperator op = aEtor(this, entry, i++, aArg); - if (op & PL_DHASH_REMOVE) { - PL_DHashTableRawRemove(this, entry); - didRemove = true; - } - if (op & PL_DHASH_STOP) { - break; - } - } - entryAddr += mEntrySize; - if (entryAddr >= entryLimit) { - entryAddr -= tableSize; - } - } - - // This is the retrospective check mentioned above. If we removed anything, - // then the table should have been idle and writable before we did so. - MOZ_ASSERT_IF(didRemove, wasIdleAndWritableAtStart); - - // Shrink the table if appropriate. Do this only if we removed above, so - // non-removing enumerations can count on stable |mEntryStore| until the next - // Add, Remove, or removing-Enumerate. - if (didRemove) { - ShrinkIfAppropriate(); - } - - return i; -} - -uint32_t -PL_DHashTableEnumerate(PLDHashTable* aTable, PLDHashEnumerator aEtor, - void* aArg) -{ - return aTable->Enumerate(aEtor, aArg); -} - MOZ_ALWAYS_INLINE size_t PLDHashTable::SizeOfExcludingThis( PLDHashSizeOfEntryExcludingThisFun aSizeOfEntryExcludingThis, diff --git a/xpcom/glue/pldhash.h b/xpcom/glue/pldhash.h index 14d634f4673e..03cc15f60077 100644 --- a/xpcom/glue/pldhash.h +++ b/xpcom/glue/pldhash.h @@ -83,60 +83,6 @@ private: PLDHashNumber mKeyHash; }; -/* - * These are the codes returned by PLDHashEnumerator functions, which control - * PL_DHashTableEnumerate's behavior. - */ -enum PLDHashOperator -{ - PL_DHASH_NEXT = 0, /* enumerator says continue */ - PL_DHASH_STOP = 1, /* enumerator says stop */ - PL_DHASH_REMOVE = 2 /* enumerator says remove */ -}; - -/* - * Enumerate entries in table using etor: - * - * count = PL_DHashTableEnumerate(table, etor, arg); - * - * PL_DHashTableEnumerate calls etor like so: - * - * op = etor(table, entry, number, arg); - * - * where number is a zero-based ordinal assigned to live entries according to - * their order in aTable->mEntryStore. - * - * The return value, op, is treated as a set of flags. If op is PL_DHASH_NEXT, - * then continue enumerating. If op contains PL_DHASH_REMOVE, then clear (via - * aTable->mOps->clearEntry) and free entry. Then we check whether op contains - * PL_DHASH_STOP; if so, stop enumerating and return the number of live entries - * that were enumerated so far. Return the total number of live entries when - * enumeration completes normally. - * - * If etor calls PL_DHashTableAdd or PL_DHashTableRemove on table, it must - * return PL_DHASH_STOP; otherwise undefined behavior results. - * - * If any enumerator returns PL_DHASH_REMOVE, aTable->mEntryStore may be shrunk - * or compressed after enumeration, but before PL_DHashTableEnumerate returns. - * Such an enumerator therefore can't safely set aside entry pointers, but an - * enumerator that never returns PL_DHASH_REMOVE can set pointers to entries - * aside, e.g., to avoid copying live entries into an array of the entry type. - * Copying entry pointers is cheaper, and safe so long as the caller of such a - * "stable" Enumerate doesn't use the set-aside pointers after any call either - * to PL_DHashTableAdd or PL_DHashTableRemove, or to an "unstable" form of - * Enumerate, which might grow or shrink mEntryStore. - * - * If your enumerator wants to remove certain entries, but set aside pointers - * to other entries that it retains, it can use PL_DHashTableRawRemove on the - * entries to be removed, returning PL_DHASH_NEXT to skip them. Likewise, if - * you want to remove entries, but for some reason you do not want mEntryStore - * to be shrunk or compressed, you can call PL_DHashTableRawRemove safely on - * the entry being enumerated, rather than returning PL_DHASH_REMOVE. - */ -typedef PLDHashOperator (*PLDHashEnumerator)(PLDHashTable* aTable, - PLDHashEntryHdr* aHdr, - uint32_t aNumber, void* aArg); - typedef size_t (*PLDHashSizeOfEntryExcludingThisFun)( PLDHashEntryHdr* aHdr, mozilla::MallocSizeOf aMallocSizeOf, void* aArg); @@ -372,8 +318,6 @@ public: void RawRemove(PLDHashEntryHdr* aEntry); - uint32_t Enumerate(PLDHashEnumerator aEtor, void* aArg); - // This function is equivalent to // ClearAndPrepareForLength(PL_DHASH_DEFAULT_INITIAL_LENGTH). void Clear(); @@ -676,10 +620,6 @@ PL_DHashTableRemove(PLDHashTable* aTable, const void* aKey); */ void PL_DHashTableRawRemove(PLDHashTable* aTable, PLDHashEntryHdr* aEntry); -uint32_t -PL_DHashTableEnumerate(PLDHashTable* aTable, PLDHashEnumerator aEtor, - void* aArg); - /** * Measure the size of the table's entry storage, and if * |aSizeOfEntryExcludingThis| is non-nullptr, measure the size of things