dict: pause rehash, minor readability refactor (#8515)

The `dict` field `iterators` is misleading and incorrect. 
This variable is used for 1 purpose - to pause rehashing.

The current `iterators` field doesn't actually count "iterators".
It counts "safe iterators".  But - it doesn't actually count safe iterators
either.  For one, it's only incremented once the safe iterator begins to
iterate, not when it's created.  It's also incremented in `dictScan` to
prevent rehashing (and commented to make it clear why `iterators` is
being incremented during a scan).

This update renames the field as `pauserehash` and creates 2 helper
macros `dictPauseRehashing(d)` and `dictResumeRehashing(d)`
This commit is contained in:
Jim Brunner 2021-02-20 02:56:30 -08:00 committed by GitHub
parent 46346e9e3a
commit 06966d2a0e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 14 deletions

View File

@ -126,7 +126,7 @@ int _dictInit(dict *d, dictType *type,
d->type = type; d->type = type;
d->privdata = privDataPtr; d->privdata = privDataPtr;
d->rehashidx = -1; d->rehashidx = -1;
d->iterators = 0; d->pauserehash = 0;
return DICT_OK; return DICT_OK;
} }
@ -264,7 +264,7 @@ long long timeInMilliseconds(void) {
* than 0, and is smaller than 1 in most cases. The exact upper bound * than 0, and is smaller than 1 in most cases. The exact upper bound
* depends on the running time of dictRehash(d,100).*/ * depends on the running time of dictRehash(d,100).*/
int dictRehashMilliseconds(dict *d, int ms) { int dictRehashMilliseconds(dict *d, int ms) {
if (d->iterators > 0) return 0; if (d->pauserehash > 0) return 0;
long long start = timeInMilliseconds(); long long start = timeInMilliseconds();
int rehashes = 0; int rehashes = 0;
@ -276,8 +276,8 @@ int dictRehashMilliseconds(dict *d, int ms) {
return rehashes; return rehashes;
} }
/* This function performs just a step of rehashing, and only if there are /* This function performs just a step of rehashing, and only if hashing has
* no safe iterators bound to our hash table. When we have iterators in the * not been paused for our hash table. When we have iterators in the
* middle of a rehashing we can't mess with the two hash tables otherwise * middle of a rehashing we can't mess with the two hash tables otherwise
* some element can be missed or duplicated. * some element can be missed or duplicated.
* *
@ -285,7 +285,7 @@ int dictRehashMilliseconds(dict *d, int ms) {
* dictionary so that the hash table automatically migrates from H1 to H2 * dictionary so that the hash table automatically migrates from H1 to H2
* while it is actively used. */ * while it is actively used. */
static void _dictRehashStep(dict *d) { static void _dictRehashStep(dict *d) {
if (d->iterators == 0) dictRehash(d,1); if (d->pauserehash == 0) dictRehash(d,1);
} }
/* Add an element to the target hash table */ /* Add an element to the target hash table */
@ -593,7 +593,7 @@ dictEntry *dictNext(dictIterator *iter)
dictht *ht = &iter->d->ht[iter->table]; dictht *ht = &iter->d->ht[iter->table];
if (iter->index == -1 && iter->table == 0) { if (iter->index == -1 && iter->table == 0) {
if (iter->safe) if (iter->safe)
iter->d->iterators++; dictPauseRehashing(iter->d);
else else
iter->fingerprint = dictFingerprint(iter->d); iter->fingerprint = dictFingerprint(iter->d);
} }
@ -625,7 +625,7 @@ void dictReleaseIterator(dictIterator *iter)
{ {
if (!(iter->index == -1 && iter->table == 0)) { if (!(iter->index == -1 && iter->table == 0)) {
if (iter->safe) if (iter->safe)
iter->d->iterators--; dictResumeRehashing(iter->d);
else else
assert(iter->fingerprint == dictFingerprint(iter->d)); assert(iter->fingerprint == dictFingerprint(iter->d));
} }
@ -896,9 +896,8 @@ unsigned long dictScan(dict *d,
if (dictSize(d) == 0) return 0; if (dictSize(d) == 0) return 0;
/* Having a safe iterator means no rehashing can happen, see _dictRehashStep. /* This is needed in case the scan callback tries to do dictFind or alike. */
* This is needed in case the scan callback tries to do dictFind or alike. */ dictPauseRehashing(d);
d->iterators++;
if (!dictIsRehashing(d)) { if (!dictIsRehashing(d)) {
t0 = &(d->ht[0]); t0 = &(d->ht[0]);
@ -966,8 +965,7 @@ unsigned long dictScan(dict *d,
} while (v & (m0 ^ m1)); } while (v & (m0 ^ m1));
} }
/* undo the ++ at the top */ dictResumeRehashing(d);
d->iterators--;
return v; return v;
} }
@ -1056,7 +1054,7 @@ void dictEmpty(dict *d, void(callback)(void*)) {
_dictClear(d,&d->ht[0],callback); _dictClear(d,&d->ht[0],callback);
_dictClear(d,&d->ht[1],callback); _dictClear(d,&d->ht[1],callback);
d->rehashidx = -1; d->rehashidx = -1;
d->iterators = 0; d->pauserehash = 0;
} }
void dictEnableResize(void) { void dictEnableResize(void) {

View File

@ -82,7 +82,7 @@ typedef struct dict {
void *privdata; void *privdata;
dictht ht[2]; dictht ht[2];
long rehashidx; /* rehashing not in progress if rehashidx == -1 */ long rehashidx; /* rehashing not in progress if rehashidx == -1 */
unsigned long iterators; /* number of iterators currently running */ int16_t pauserehash; /* If >0 rehashing is paused (<0 indicates coding error) */
} dict; } dict;
/* If safe is set to 1 this is a safe iterator, that means, you can call /* If safe is set to 1 this is a safe iterator, that means, you can call
@ -150,6 +150,8 @@ typedef void (dictScanBucketFunction)(void *privdata, dictEntry **bucketref);
#define dictSlots(d) ((d)->ht[0].size+(d)->ht[1].size) #define dictSlots(d) ((d)->ht[0].size+(d)->ht[1].size)
#define dictSize(d) ((d)->ht[0].used+(d)->ht[1].used) #define dictSize(d) ((d)->ht[0].used+(d)->ht[1].used)
#define dictIsRehashing(d) ((d)->rehashidx != -1) #define dictIsRehashing(d) ((d)->rehashidx != -1)
#define dictPauseRehashing(d) (d)->pauserehash++
#define dictResumeRehashing(d) (d)->pauserehash--
/* If our unsigned long type can store a 64 bit number, use a 64 bit PRNG. */ /* If our unsigned long type can store a 64 bit number, use a 64 bit PRNG. */
#if ULONG_MAX >= 0xffffffffffffffff #if ULONG_MAX >= 0xffffffffffffffff