diff --git a/src/dict.c b/src/dict.c index e37b659e..2d9ac35a 100644 --- a/src/dict.c +++ b/src/dict.c @@ -66,23 +66,11 @@ static unsigned int dict_force_resize_ratio = 5; static int _dictExpandIfNeeded(dict *ht); static unsigned long _dictNextPower(unsigned long size); -static int _dictKeyIndex(dict *ht, const void *key); +static int _dictKeyIndex(dict *ht, const void *key, unsigned int hash, dictEntry **existing); static int _dictInit(dict *ht, dictType *type, void *privDataPtr); /* -------------------------- hash functions -------------------------------- */ -/* Thomas Wang's 32 bit Mix Function */ -unsigned int dictIntHashFunction(unsigned int key) -{ - key += ~(key << 15); - key ^= (key >> 10); - key += (key << 3); - key ^= (key >> 6); - key += ~(key << 11); - key ^= (key >> 16); - return key; -} - static uint32_t dict_hash_function_seed = 5381; void dictSetHashFunctionSeed(uint32_t seed) { @@ -325,29 +313,32 @@ static void _dictRehashStep(dict *d) { /* Add an element to the target hash table */ int dictAdd(dict *d, void *key, void *val) { - dictEntry *entry = dictAddRaw(d,key); + dictEntry *entry = dictAddRaw(d,key,NULL); if (!entry) return DICT_ERR; dictSetVal(d, entry, val); return DICT_OK; } -/* Low level add. This function adds the entry but instead of setting - * a value returns the dictEntry structure to the user, that will make - * sure to fill the value field as he wishes. +/* Low level add or find: + * This function adds the entry but instead of setting a value returns the + * dictEntry structure to the user, that will make sure to fill the value + * field as he wishes. * * This function is also directly exposed to the user API to be called * mainly in order to store non-pointers inside the hash value, example: * - * entry = dictAddRaw(dict,mykey); + * entry = dictAddRaw(dict,mykey,NULL); * if (entry != NULL) dictSetSignedIntegerVal(entry,1000); * * Return values: * - * If key already exists NULL is returned. + * If key already exists NULL is returned, and "*existing" is populated + * with the existing entry if existing is not NULL. + * * If key was added, the hash entry is returned to be manipulated by the caller. */ -dictEntry *dictAddRaw(dict *d, void *key) +dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing) { int index; dictEntry *entry; @@ -357,7 +348,7 @@ dictEntry *dictAddRaw(dict *d, void *key) /* Get the index of the new element, or -1 if * the element already exists. */ - if ((index = _dictKeyIndex(d, key)) == -1) + if ((index = _dictKeyIndex(d, key, dictHashKey(d,key), existing)) == -1) return NULL; /* Allocate the memory and store the new entry. @@ -375,41 +366,45 @@ dictEntry *dictAddRaw(dict *d, void *key) return entry; } -/* Add an element, discarding the old if the key already exists. +/* Add or Overwrite: + * Add an element, discarding the old value if the key already exists. * Return 1 if the key was added from scratch, 0 if there was already an * element with such key and dictReplace() just performed a value update * operation. */ int dictReplace(dict *d, void *key, void *val) { - dictEntry *entry, auxentry; + dictEntry *entry, *existing, auxentry; /* Try to add the element. If the key * does not exists dictAdd will suceed. */ - if (dictAdd(d, key, val) == DICT_OK) + entry = dictAddRaw(d,key,&existing); + if (entry) { + dictSetVal(d, entry, val); return 1; - /* It already exists, get the entry */ - entry = dictFind(d, key); + } + /* Set the new value and free the old one. Note that it is important * to do that in this order, as the value may just be exactly the same * as the previous one. In this context, think to reference counting, * you want to increment (set), and then decrement (free), and not the * reverse. */ - auxentry = *entry; - dictSetVal(d, entry, val); + auxentry = *existing; + dictSetVal(d, existing, val); dictFreeVal(d, &auxentry); return 0; } -/* dictReplaceRaw() is simply a version of dictAddRaw() that always +/* Add or Find: + * dictReplaceRaw() is simply a version of dictAddRaw() that always * returns the hash entry of the specified key, even if the key already * exists and can't be added (in that case the entry of the already * existing key is returned.) * * See dictAddRaw() for more information. */ dictEntry *dictReplaceRaw(dict *d, void *key) { - dictEntry *entry = dictFind(d,key); - - return entry ? entry : dictAddRaw(d,key); + dictEntry *entry, *existing; + entry = dictAddRaw(d,key,&existing); + return entry ? entry : existing; } /* Search and remove an element */ @@ -966,27 +961,29 @@ static unsigned long _dictNextPower(unsigned long size) /* Returns the index of a free slot that can be populated with * a hash entry for the given 'key'. - * If the key already exists, -1 is returned. + * If the key already exists, -1 is returned + * and the optional output parameter may be filled. * * Note that if we are in the process of rehashing the hash table, the * index is always returned in the context of the second (new) hash table. */ -static int _dictKeyIndex(dict *d, const void *key) +static int _dictKeyIndex(dict *d, const void *key, unsigned int hash, dictEntry **existing) { - unsigned int h, idx, table; + unsigned int idx, table; dictEntry *he; + if (existing) *existing = NULL; /* Expand the hash table if needed */ if (_dictExpandIfNeeded(d) == DICT_ERR) return -1; - /* Compute the key hash value */ - h = dictHashKey(d, key); for (table = 0; table <= 1; table++) { - idx = h & d->ht[table].sizemask; + idx = hash & d->ht[table].sizemask; /* Search if this slot does not already contain the given key */ he = d->ht[table].table[idx]; while(he) { - if (key==he->key || dictCompareKeys(d, key, he->key)) + if (key==he->key || dictCompareKeys(d, key, he->key)) { + if (existing) *existing = he; return -1; + } he = he->next; } if (!dictIsRehashing(d)) break; diff --git a/src/dict.h b/src/dict.h index 967a238b..739de68e 100644 --- a/src/dict.h +++ b/src/dict.h @@ -106,19 +106,19 @@ typedef void (dictScanFunction)(void *privdata, const dictEntry *de); #define dictSetVal(d, entry, _val_) do { \ if ((d)->type->valDup) \ - entry->v.val = (d)->type->valDup((d)->privdata, _val_); \ + (entry)->v.val = (d)->type->valDup((d)->privdata, _val_); \ else \ - entry->v.val = (_val_); \ + (entry)->v.val = (_val_); \ } while(0) #define dictSetSignedIntegerVal(entry, _val_) \ - do { entry->v.s64 = _val_; } while(0) + do { (entry)->v.s64 = _val_; } while(0) #define dictSetUnsignedIntegerVal(entry, _val_) \ - do { entry->v.u64 = _val_; } while(0) + do { (entry)->v.u64 = _val_; } while(0) #define dictSetDoubleVal(entry, _val_) \ - do { entry->v.d = _val_; } while(0) + do { (entry)->v.d = _val_; } while(0) #define dictFreeKey(d, entry) \ if ((d)->type->keyDestructor) \ @@ -126,9 +126,9 @@ typedef void (dictScanFunction)(void *privdata, const dictEntry *de); #define dictSetKey(d, entry, _key_) do { \ if ((d)->type->keyDup) \ - entry->key = (d)->type->keyDup((d)->privdata, _key_); \ + (entry)->key = (d)->type->keyDup((d)->privdata, _key_); \ else \ - entry->key = (_key_); \ + (entry)->key = (_key_); \ } while(0) #define dictCompareKeys(d, key1, key2) \ @@ -150,7 +150,7 @@ typedef void (dictScanFunction)(void *privdata, const dictEntry *de); dict *dictCreate(dictType *type, void *privDataPtr); int dictExpand(dict *d, unsigned long size); int dictAdd(dict *d, void *key, void *val); -dictEntry *dictAddRaw(dict *d, void *key); +dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing); int dictReplace(dict *d, void *key, void *val); dictEntry *dictReplaceRaw(dict *d, void *key); int dictDelete(dict *d, const void *key); diff --git a/src/sentinel.c b/src/sentinel.c index 0168aa63..23561154 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -3640,15 +3640,15 @@ struct sentinelLeader { /* Helper function for sentinelGetLeader, increment the counter * relative to the specified runid. */ int sentinelLeaderIncr(dict *counters, char *runid) { - dictEntry *de = dictFind(counters,runid); + dictEntry *existing, *de; uint64_t oldval; - if (de) { - oldval = dictGetUnsignedIntegerVal(de); - dictSetUnsignedIntegerVal(de,oldval+1); + de = dictAddRaw(counters,runid,&existing); + if (existing) { + oldval = dictGetUnsignedIntegerVal(existing); + dictSetUnsignedIntegerVal(existing,oldval+1); return oldval+1; } else { - de = dictAddRaw(counters,runid); serverAssert(de != NULL); dictSetUnsignedIntegerVal(de,1); return 1; diff --git a/src/t_set.c b/src/t_set.c index ddd82b8b..d5a801e1 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -53,7 +53,7 @@ int setTypeAdd(robj *subject, sds value) { long long llval; if (subject->encoding == OBJ_ENCODING_HT) { dict *ht = subject->ptr; - dictEntry *de = dictAddRaw(ht,value); + dictEntry *de = dictAddRaw(ht,value,NULL); if (de) { dictSetKey(ht,de,sdsdup(value)); dictSetVal(ht,de,NULL); diff --git a/src/t_zset.c b/src/t_zset.c index c61ba808..81e6e57c 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -2262,7 +2262,7 @@ void zunionInterGenericCommand(client *c, robj *dstkey, int op) { } else if (op == SET_OP_UNION) { dict *accumulator = dictCreate(&setAccumulatorDictType,NULL); dictIterator *di; - dictEntry *de; + dictEntry *de, *existing; double score; if (setnum) { @@ -2283,16 +2283,16 @@ void zunionInterGenericCommand(client *c, robj *dstkey, int op) { if (isnan(score)) score = 0; /* Search for this element in the accumulating dictionary. */ - de = dictFind(accumulator,zuiSdsFromValue(&zval)); + de = dictAddRaw(accumulator,zuiSdsFromValue(&zval),&existing); /* If we don't have it, we need to create a new entry. */ - if (de == NULL) { + if (!existing) { tmp = zuiNewSdsFromValue(&zval); /* Remember the longest single element encountered, * to understand if it's possible to convert to ziplist * at the end. */ if (sdslen(tmp) > maxelelen) maxelelen = sdslen(tmp); - /* Add the element with its initial score. */ - de = dictAddRaw(accumulator,tmp); + /* Update the element with its initial score. */ + dictSetKey(accumulator, de, tmp); dictSetDoubleVal(de,score); } else { /* Update the score with the score of the new instance @@ -2301,7 +2301,7 @@ void zunionInterGenericCommand(client *c, robj *dstkey, int op) { * Here we access directly the dictEntry double * value inside the union as it is a big speedup * compared to using the getDouble/setDouble API. */ - zunionInterAggregate(&de->v.d,score,aggregate); + zunionInterAggregate(&existing->v.d,score,aggregate); } } zuiClearIterator(&src[i]);