diff --git a/js/src/jsdhash.c b/js/src/jsdhash.c index 523d6488486..3e7ba1e5211 100644 --- a/js/src/jsdhash.c +++ b/js/src/jsdhash.c @@ -586,6 +586,7 @@ JS_DHashTableEnumerate(JSDHashTable *table, JSDHashEnumerator etor, void *arg) { char *entryAddr, *entryLimit; uint32 i, capacity, entrySize; + JSBool didRemove; JSDHashEntryHdr *entry; JSDHashOperator op; @@ -594,6 +595,7 @@ JS_DHashTableEnumerate(JSDHashTable *table, JSDHashEnumerator etor, void *arg) capacity = JS_DHASH_TABLE_SIZE(table); entryLimit = entryAddr + capacity * entrySize; i = 0; + didRemove = JS_FALSE; while (entryAddr < entryLimit) { entry = (JSDHashEntryHdr *)entryAddr; if (ENTRY_IS_LIVE(entry)) { @@ -601,6 +603,7 @@ JS_DHashTableEnumerate(JSDHashTable *table, JSDHashEnumerator etor, void *arg) if (op & JS_DHASH_REMOVE) { METER(table->stats.removeEnums++); JS_DHashTableRawRemove(table, entry); + didRemove = JS_TRUE; } if (op & JS_DHASH_STOP) break; @@ -611,11 +614,14 @@ JS_DHashTableEnumerate(JSDHashTable *table, JSDHashEnumerator etor, void *arg) /* * Shrink or compress if a quarter or more of all entries are removed, or * if the table is underloaded according to the configured minimum alpha, - * and is not minimal-size already. + * and is not minimal-size already. Do this only if we removed above, so + * non-removing enumerations can count on stable table->entryStore until + * the next Operate or removing-Enumerate. */ - if (table->removedCount >= capacity >> 2 || - (capacity > JS_DHASH_MIN_SIZE && - table->entryCount <= MIN_LOAD(table, capacity))) { + if (didRemove && + (table->removedCount >= capacity >> 2 || + (capacity > JS_DHASH_MIN_SIZE && + table->entryCount <= MIN_LOAD(table, capacity)))) { METER(table->stats.enumShrinks++); capacity = table->entryCount; capacity += capacity >> 1; diff --git a/js/src/jsdhash.h b/js/src/jsdhash.h index d95d8836d3a..9abb9b83a56 100644 --- a/js/src/jsdhash.h +++ b/js/src/jsdhash.h @@ -323,6 +323,8 @@ typedef void * newly created by the JS_DHASH_ADD call that just succeeded. If placement * new or similar initialization is required, define an initEntry hook. Of * course, the clearEntry hook must zero or null appropriately. + * + * XXX assumes 0 is null for pointer types. */ struct JSDHashTableOps { /* Mandatory hooks. All implementations must provide these. */ @@ -476,7 +478,7 @@ typedef enum JSDHashOperator { * Otherwise, entry->keyHash has been set so that JS_DHASH_ENTRY_IS_BUSY(entry) * is true, and it is up to the caller to initialize the key and value parts * of the entry sub-type, if they have not been set already (i.e. if entry was - * not already in the table). + * not already in the table, and if the optional initEntry hook was not used). * * To remove an entry identified by key from table, call: * @@ -522,6 +524,23 @@ JS_DHashTableRawRemove(JSDHashTable *table, JSDHashEntryHdr *entry); * * If etor calls JS_DHashTableOperate on table, it must return JS_DHASH_STOP; * otherwise undefined behavior results. + * + * If any enumerator returns JS_DHASH_REMOVE, table->entryStore may be shrunk + * or compressed after enumeration, but before JS_DHashTableEnumerate returns. + * Such an enumerator therefore can't safely set aside entry pointers, but an + * enumerator that never returns JS_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_DHashTableOperate, or to an "unstable" form of Enumerate, which might + * grow or shrink entryStore. + * + * If your enumerator wants to remove certain entries, but set aside pointers + * to other entries that it retains, it can use JS_DHashTableRawRemove on the + * entries to be removed, returning JS_DHASH_NEXT to skip them. Likewise, if + * you want to remove entries, but for some reason you do not want entryStore + * to be shrunk or compressed, you can call JS_DHashTableRawRemove safely on + * the entry being enumerated, rather than returning JS_DHASH_REMOVE. */ typedef JSDHashOperator (* JS_DLL_CALLBACK JSDHashEnumerator)(JSDHashTable *table, JSDHashEntryHdr *hdr, diff --git a/xpcom/ds/pldhash.c b/xpcom/ds/pldhash.c index bd7c539e964..fdeb602ffe7 100644 --- a/xpcom/ds/pldhash.c +++ b/xpcom/ds/pldhash.c @@ -587,6 +587,7 @@ PL_DHashTableEnumerate(PLDHashTable *table, PLDHashEnumerator etor, void *arg) { char *entryAddr, *entryLimit; PRUint32 i, capacity, entrySize; + PRBool didRemove; PLDHashEntryHdr *entry; PLDHashOperator op; @@ -595,6 +596,7 @@ PL_DHashTableEnumerate(PLDHashTable *table, PLDHashEnumerator etor, void *arg) capacity = PL_DHASH_TABLE_SIZE(table); entryLimit = entryAddr + capacity * entrySize; i = 0; + didRemove = PR_FALSE; while (entryAddr < entryLimit) { entry = (PLDHashEntryHdr *)entryAddr; if (ENTRY_IS_LIVE(entry)) { @@ -602,6 +604,7 @@ PL_DHashTableEnumerate(PLDHashTable *table, PLDHashEnumerator etor, void *arg) if (op & PL_DHASH_REMOVE) { METER(table->stats.removeEnums++); PL_DHashTableRawRemove(table, entry); + didRemove = PR_TRUE; } if (op & PL_DHASH_STOP) break; @@ -612,11 +615,14 @@ PL_DHashTableEnumerate(PLDHashTable *table, PLDHashEnumerator etor, void *arg) /* * Shrink or compress if a quarter or more of all entries are removed, or * if the table is underloaded according to the configured minimum alpha, - * and is not minimal-size already. + * and is not minimal-size already. Do this only if we removed above, so + * non-removing enumerations can count on stable table->entryStore until + * the next Operate or removing-Enumerate. */ - if (table->removedCount >= capacity >> 2 || - (capacity > PL_DHASH_MIN_SIZE && - table->entryCount <= MIN_LOAD(table, capacity))) { + if (didRemove && + (table->removedCount >= capacity >> 2 || + (capacity > PL_DHASH_MIN_SIZE && + table->entryCount <= MIN_LOAD(table, capacity)))) { METER(table->stats.enumShrinks++); capacity = table->entryCount; capacity += capacity >> 1; diff --git a/xpcom/ds/pldhash.h b/xpcom/ds/pldhash.h index becbbce807b..27a74ef0085 100644 --- a/xpcom/ds/pldhash.h +++ b/xpcom/ds/pldhash.h @@ -324,6 +324,8 @@ typedef void * newly created by the PL_DHASH_ADD call that just succeeded. If placement * new or similar initialization is required, define an initEntry hook. Of * course, the clearEntry hook must zero or null appropriately. + * + * XXX assumes 0 is null for pointer types. */ struct PLDHashTableOps { /* Mandatory hooks. All implementations must provide these. */ @@ -477,7 +479,7 @@ typedef enum PLDHashOperator { * Otherwise, entry->keyHash has been set so that PL_DHASH_ENTRY_IS_BUSY(entry) * is true, and it is up to the caller to initialize the key and value parts * of the entry sub-type, if they have not been set already (i.e. if entry was - * not already in the table). + * not already in the table, and if the optional initEntry hook was not used). * * To remove an entry identified by key from table, call: * @@ -523,6 +525,23 @@ PL_DHashTableRawRemove(PLDHashTable *table, PLDHashEntryHdr *entry); * * If etor calls PL_DHashTableOperate on table, it must return PL_DHASH_STOP; * otherwise undefined behavior results. + * + * If any enumerator returns PL_DHASH_REMOVE, table->entryStore 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_DHashTableOperate, or to an "unstable" form of Enumerate, which might + * grow or shrink entryStore. + * + * 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 entryStore + * to be shrunk or compressed, you can call PL_DHashTableRawRemove safely on + * the entry being enumerated, rather than returning PL_DHASH_REMOVE. */ typedef PLDHashOperator (* PR_CALLBACK PLDHashEnumerator)(PLDHashTable *table, PLDHashEntryHdr *hdr,