From 0d610b39ece952b639e2d66d7a7fdbad045f01f7 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Thu, 4 Jul 2024 19:30:34 -0700 Subject: [PATCH 01/15] Improve type safety and refactor dict entry handling - Replace `struct dictEntry` with `typedef _dictEntryNormal` - Rename `embeddedDictEntry` to `_dictEntryEmbedded` - Rename `dictEntryNoValue` to `_dictEntryNoValue` - Introduce macros for setting and getting values in dict entries - Update functions to utilize new typedef structures and macros - Refactor code for improved readability and maintainability Signed-off-by: Ping Xie --- src/dict.c | 222 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 125 insertions(+), 97 deletions(-) diff --git a/src/dict.c b/src/dict.c index b6a06eb36a..9329fba45e 100644 --- a/src/dict.c +++ b/src/dict.c @@ -70,7 +70,7 @@ static dictResizeEnable dict_can_resize = DICT_RESIZE_ENABLE; static unsigned int dict_force_resize_ratio = 4; /* -------------------------- types ----------------------------------------- */ -struct dictEntry { +typedef struct { void *key; union { void *val; @@ -79,7 +79,7 @@ struct dictEntry { double d; } v; struct dictEntry *next; /* Next entry in the same hash bucket. */ -}; +} _dictEntryNormal; typedef struct { union { @@ -91,27 +91,27 @@ typedef struct { struct dictEntry *next; /* Next entry in the same hash bucket. */ uint8_t key_header_size; /* offset into key_buf where the key is located at. */ unsigned char key_buf[]; /* buffer with embedded key. */ -} embeddedDictEntry; +} _dictEntryEmbedded; -/* Validation and helper for `embeddedDictEntry` */ +/* Validation and helper for `_dictEntryEmbedded` */ -static_assert(offsetof(embeddedDictEntry, v) == 0, "unexpected field offset"); -static_assert(offsetof(embeddedDictEntry, next) == sizeof(double), "unexpected field offset"); -static_assert(offsetof(embeddedDictEntry, key_header_size) == sizeof(double) + sizeof(void *), +static_assert(offsetof(_dictEntryEmbedded, v) == 0, "unexpected field offset"); +static_assert(offsetof(_dictEntryEmbedded, next) == sizeof(double), "unexpected field offset"); +static_assert(offsetof(_dictEntryEmbedded, key_header_size) == sizeof(double) + sizeof(void *), "unexpected field offset"); /* key_buf is located after a union with a double value `v.d`, a pointer `next` and uint8_t field `key_header_size` */ -static_assert(offsetof(embeddedDictEntry, key_buf) == sizeof(double) + sizeof(void *) + sizeof(uint8_t), +static_assert(offsetof(_dictEntryEmbedded, key_buf) == sizeof(double) + sizeof(void *) + sizeof(uint8_t), "unexpected field offset"); /* The minimum amount of bytes required for embedded dict entry. */ static inline size_t compactSizeEmbeddedDictEntry(void) { - return offsetof(embeddedDictEntry, key_buf); + return offsetof(_dictEntryEmbedded, key_buf); } typedef struct { void *key; dictEntry *next; -} dictEntryNoValue; +} _dictEntryNoValue; /* -------------------------- private prototypes ---------------------------- */ @@ -172,28 +172,27 @@ uint64_t dictGenCaseHashFunction(const unsigned char *buf, size_t len) { #define ENTRY_PTR_NORMAL 0 /* 000 */ #define ENTRY_PTR_NO_VALUE 2 /* 010 */ #define ENTRY_PTR_EMBEDDED 4 /* 100 */ -/* ENTRY_PTR_IS_KEY xx1 */ +#define ENTRY_PTR_IS_KEY 1 /* 001 */ /* Returns 1 if the entry pointer is a pointer to a key, rather than to an * allocated entry. Returns 0 otherwise. */ -static inline int entryIsKey(const dictEntry *de) { - return (uintptr_t)(void *)de & 1; +static inline int entryIsKey(const void *de) { + return (uintptr_t)(void *)de & ENTRY_PTR_IS_KEY; } /* Returns 1 if the pointer is actually a pointer to a dictEntry struct. Returns * 0 otherwise. */ -static inline int entryIsNormal(const dictEntry *de) { +static inline int entryIsNormal(const void *de) { return ((uintptr_t)(void *)de & ENTRY_PTR_MASK) == ENTRY_PTR_NORMAL; } /* Returns 1 if the entry is a special entry with key and next, but without * value. Returns 0 otherwise. */ -static inline int entryIsNoValue(const dictEntry *de) { +static inline int entryIsNoValue(const void *de) { return ((uintptr_t)(void *)de & ENTRY_PTR_MASK) == ENTRY_PTR_NO_VALUE; } - -static inline int entryIsEmbedded(const dictEntry *de) { +static inline int entryIsEmbedded(const void *de) { return ((uintptr_t)(void *)de & ENTRY_PTR_MASK) == ENTRY_PTR_EMBEDDED; } @@ -207,9 +206,17 @@ static inline void *decodeMaskedPtr(const dictEntry *de) { return (void *)((uintptr_t)(void *)de & ~ENTRY_PTR_MASK); } +static inline dictEntry *createEntryNormal(void *key, dictEntry *next) { + _dictEntryNormal *entry = zmalloc(sizeof(_dictEntryNormal)); + assert(entryIsNormal(entry)); /* Check alignment of allocation */ + entry->key = key; + entry->next = next; + return encodeMaskedPtr(entry, ENTRY_PTR_NORMAL); + } + /* Creates an entry without a value field. */ static inline dictEntry *createEntryNoValue(void *key, dictEntry *next) { - dictEntryNoValue *entry = zmalloc(sizeof(*entry)); + _dictEntryNoValue *entry = zmalloc(sizeof(*entry)); entry->key = key; entry->next = next; return encodeMaskedPtr(entry, ENTRY_PTR_NO_VALUE); @@ -217,24 +224,28 @@ static inline dictEntry *createEntryNoValue(void *key, dictEntry *next) { static inline dictEntry *createEmbeddedEntry(void *key, dictEntry *next, dictType *dt) { size_t key_len = dt->embedKey(NULL, 0, key, NULL); - embeddedDictEntry *entry = zmalloc(compactSizeEmbeddedDictEntry() + key_len); + _dictEntryEmbedded *entry = zmalloc(compactSizeEmbeddedDictEntry() + key_len); dt->embedKey(entry->key_buf, key_len, key, &entry->key_header_size); entry->next = next; return encodeMaskedPtr(entry, ENTRY_PTR_EMBEDDED); } static inline void *getEmbeddedKey(const dictEntry *de) { - embeddedDictEntry *entry = (embeddedDictEntry *)decodeMaskedPtr(de); + _dictEntryEmbedded *entry = (_dictEntryEmbedded *)decodeMaskedPtr(de); return &entry->key_buf[entry->key_header_size]; } /* Decodes the pointer to an entry without value, when you know it is an entry * without value. Hint: Use entryIsNoValue to check. */ -static inline dictEntryNoValue *decodeEntryNoValue(const dictEntry *de) { +static inline _dictEntryNoValue *decodeEntryNoValue(const dictEntry *de) { + return decodeMaskedPtr(de); +} + +static inline _dictEntryEmbedded *decodeEntryEmbedded(const dictEntry *de) { return decodeMaskedPtr(de); } -static inline embeddedDictEntry *decodeEmbeddedEntry(const dictEntry *de) { +static inline _dictEntryNormal *decodeEntryNormal(const dictEntry *de) { return decodeMaskedPtr(de); } @@ -301,8 +312,9 @@ int _dictResize(dict *d, unsigned long size, int *malloc_failed) { new_ht_table = ztrycalloc(newsize * sizeof(dictEntry *)); *malloc_failed = new_ht_table == NULL; if (*malloc_failed) return DICT_ERR; - } else + } else { new_ht_table = zcalloc(newsize * sizeof(dictEntry *)); + } new_ht_used = 0; @@ -381,12 +393,7 @@ static void rehashEntriesInBucketAtIndex(dict *d, uint64_t idx) { if (d->type->keys_are_odd && !d->ht_table[1][h]) { /* Destination bucket is empty and we can store the key * directly without an allocated entry. Free the old entry - * if it's an allocated entry. - * - * TODO: Add a flag 'keys_are_even' and if set, we can use - * this optimization for these dicts too. We can set the LSB - * bit when stored as a dict entry and clear it again when - * we need the key back. */ + * if it's an allocated entry. */ assert(entryIsKey(key)); if (!entryIsKey(de)) zfree(decodeMaskedPtr(de)); de = key; @@ -570,15 +577,14 @@ dictEntry *dictInsertAtPosition(dict *d, void *key, void *position) { * Assert that the provided bucket is the right table. */ int htidx = dictIsRehashing(d) ? 1 : 0; assert(bucket >= &d->ht_table[htidx][0] && bucket <= &d->ht_table[htidx][DICTHT_SIZE_MASK(d->ht_size_exp[htidx])]); - if (d->type->no_value) { + /* Allocate the memory and store the new entry. + * Insert the element in top, with the assumption that in a database + * system it is more likely that recently added entries are accessed + * more frequently. */ + if (d->type->no_value) { if (d->type->keys_are_odd && !*bucket) { /* We can store the key directly in the destination bucket without the - * allocated entry. - * - * TODO: Add a flag 'keys_are_even' and if set, we can use this - * optimization for these dicts too. We can set the LSB bit when - * stored as a dict entry and clear it again when we need the key - * back. */ + * allocated entry. */ entry = key; assert(entryIsKey(entry)); } else { @@ -588,14 +594,7 @@ dictEntry *dictInsertAtPosition(dict *d, void *key, void *position) { } else if (d->type->embedded_entry) { entry = createEmbeddedEntry(key, *bucket, d->type); } else { - /* Allocate the memory and store the new entry. - * Insert the element in top, with the assumption that in a database - * system it is more likely that recently added entries are accessed - * more frequently. */ - entry = zmalloc(sizeof(*entry)); - assert(entryIsNormal(entry)); /* Check alignment of allocation */ - entry->key = key; - entry->next = *bucket; + entry = createEntryNormal(key, *bucket); } *bucket = entry; d->ht_used[htidx]++; @@ -870,88 +869,114 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictResumeRehashing(d); } +#define DICT_SET_KEY(e, k) \ + { \ + if (entryIsNormal(e)) { \ + _dictEntryNormal *_de = decodeEntryNormal(e); \ + _de->key = k; \ + } else if (entryIsNoValue(e)) { \ + _dictEntryNoValue *_de = decodeEntryNoValue(e); \ + _de->key = k; \ + } else { \ + assert(false); \ + } \ + } + +#define DICT_SET_VALUE(e, f, v) \ + { \ + if (entryIsNormal(e)) { \ + _dictEntryNormal *_de = decodeEntryNormal(e); \ + _de->f = v; \ + } else if (entryIsEmbedded(e)) { \ + _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ + _de->f = v; \ + } else { \ + assert(false); \ + } \ + } + +#define DICT_INCR_VALUE(e, f, v) \ + { \ + if (entryIsNormal(e)) { \ + _dictEntryNormal *_de = decodeEntryNormal(e); \ + _de->f += v; \ + } else if (entryIsEmbedded(e)) { \ + _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ + _de->f += v; \ + } else { \ + assert(false); \ + } \ + } + +#define DICT_GET_VALUE(e, f) \ + (assert(entryHasValue(e)), entryIsNormal(e) ? decodeEntryNormal(e)->f : decodeEntryEmbedded(e)->f) + +#define DICT_GET_VALUE_PTR(e, f) \ + (assert(entryHasValue(e)), entryIsNormal(e) ? &decodeEntryNormal(e)->f : &decodeEntryEmbedded(e)->f) + void dictSetKey(dict *d, dictEntry *de, void *key) { - assert(!d->type->no_value); - if (d->type->keyDup) - de->key = d->type->keyDup(d, key); - else - de->key = key; + void *k = d->type->keyDup ? d->type->keyDup(d, key) : key; + DICT_SET_KEY(de, k); } void dictSetVal(dict *d, dictEntry *de, void *val) { UNUSED(d); - assert(entryHasValue(de)); - if (entryIsEmbedded(de)) { - decodeEmbeddedEntry(de)->v.val = val; - } else { - de->v.val = val; - } + DICT_SET_VALUE(de, v.val, val); } void dictSetSignedIntegerVal(dictEntry *de, int64_t val) { - assert(entryHasValue(de)); - de->v.s64 = val; + DICT_SET_VALUE(de, v.s64, val); } void dictSetUnsignedIntegerVal(dictEntry *de, uint64_t val) { - assert(entryHasValue(de)); - de->v.u64 = val; + DICT_SET_VALUE(de, v.u64, val); } void dictSetDoubleVal(dictEntry *de, double val) { - assert(entryHasValue(de)); - de->v.d = val; + DICT_SET_VALUE(de, v.d, val); } int64_t dictIncrSignedIntegerVal(dictEntry *de, int64_t val) { - assert(entryHasValue(de)); - return de->v.s64 += val; + DICT_INCR_VALUE(de, v.s64, val); + return DICT_GET_VALUE(de, v.s64); } uint64_t dictIncrUnsignedIntegerVal(dictEntry *de, uint64_t val) { - assert(entryHasValue(de)); - return de->v.u64 += val; + DICT_INCR_VALUE(de, v.u64, val); + return DICT_GET_VALUE(de, v.u64); } double dictIncrDoubleVal(dictEntry *de, double val) { - assert(entryHasValue(de)); - return de->v.d += val; + DICT_INCR_VALUE(de, v.d, val); + return DICT_GET_VALUE(de, v.d); } void *dictGetKey(const dictEntry *de) { if (entryIsKey(de)) return (void *)de; if (entryIsNoValue(de)) return decodeEntryNoValue(de)->key; if (entryIsEmbedded(de)) return getEmbeddedKey(de); - return de->key; + return decodeEntryNormal(de)->key; } void *dictGetVal(const dictEntry *de) { - assert(entryHasValue(de)); - if (entryIsEmbedded(de)) { - return decodeEmbeddedEntry(de)->v.val; - } - return de->v.val; + return DICT_GET_VALUE(de, v.val); } int64_t dictGetSignedIntegerVal(const dictEntry *de) { - assert(entryHasValue(de)); - return de->v.s64; + return DICT_GET_VALUE(de, v.s64); } uint64_t dictGetUnsignedIntegerVal(const dictEntry *de) { - assert(entryHasValue(de)); - return de->v.u64; + return DICT_GET_VALUE(de, v.u64); } double dictGetDoubleVal(const dictEntry *de) { - assert(entryHasValue(de)); - return de->v.d; + return DICT_GET_VALUE(de, v.d); } /* Returns a mutable reference to the value as a double within the entry. */ double *dictGetDoubleValPtr(dictEntry *de) { - assert(entryHasValue(de)); - return &de->v.d; + return DICT_GET_VALUE_PTR(de, v.d); } /* Returns the 'next' field of the entry or NULL if the entry doesn't have a @@ -959,8 +984,8 @@ double *dictGetDoubleValPtr(dictEntry *de) { static dictEntry *dictGetNext(const dictEntry *de) { if (entryIsKey(de)) return NULL; /* there's no next */ if (entryIsNoValue(de)) return decodeEntryNoValue(de)->next; - if (entryIsEmbedded(de)) return decodeEmbeddedEntry(de)->next; - return de->next; + if (entryIsEmbedded(de)) return decodeEntryEmbedded(de)->next; + return decodeEntryNormal(de)->next; } /* Returns a pointer to the 'next' field in the entry or NULL if the entry @@ -968,8 +993,8 @@ static dictEntry *dictGetNext(const dictEntry *de) { static dictEntry **dictGetNextRef(dictEntry *de) { if (entryIsKey(de)) return NULL; if (entryIsNoValue(de)) return &decodeEntryNoValue(de)->next; - if (entryIsEmbedded(de)) return &decodeEmbeddedEntry(de)->next; - return &de->next; + if (entryIsEmbedded(de)) return &decodeEntryEmbedded(de)->next; + return &decodeEntryNormal(de)->next; } static void dictSetNext(dictEntry *de, dictEntry *next) { @@ -977,29 +1002,30 @@ static void dictSetNext(dictEntry *de, dictEntry *next) { if (entryIsNoValue(de)) { decodeEntryNoValue(de)->next = next; } else if (entryIsEmbedded(de)) { - decodeEmbeddedEntry(de)->next = next; + decodeEntryEmbedded(de)->next = next; } else { - de->next = next; + assert(entryIsNormal(de)); + decodeEntryNormal(de)->next = next; } } /* Returns the memory usage in bytes of the dict, excluding the size of the keys * and values. */ size_t dictMemUsage(const dict *d) { - return dictSize(d) * sizeof(dictEntry) + dictBuckets(d) * sizeof(dictEntry *); + return dictSize(d) * sizeof(_dictEntryNormal) + dictBuckets(d) * sizeof(dictEntry *); } /* Returns the memory usage in bytes of dictEntry based on the type. if `de` is NULL, return the size of * regular dict entry else return based on the type. */ size_t dictEntryMemUsage(dictEntry *de) { if (de == NULL || entryIsNormal(de)) - return sizeof(dictEntry); + return sizeof(_dictEntryNormal); else if (entryIsKey(de)) return 0; else if (entryIsNoValue(de)) - return sizeof(dictEntryNoValue); + return sizeof(_dictEntryNoValue); else if (entryIsEmbedded(de)) - return zmalloc_size(decodeEmbeddedEntry(de)); + return zmalloc_size(decodeEntryEmbedded(de)); else assert("Entry type not supported"); return 0; @@ -1284,7 +1310,7 @@ static void dictDefragBucket(dictEntry **bucketref, dictDefragFunctions *defragf if (newkey) *bucketref = newkey; assert(entryIsKey(*bucketref)); } else if (entryIsNoValue(de)) { - dictEntryNoValue *entry = decodeEntryNoValue(de), *newentry; + _dictEntryNoValue *entry = decodeEntryNoValue(de), *newentry; if ((newentry = defragalloc(entry))) { newde = encodeMaskedPtr(newentry, ENTRY_PTR_NO_VALUE); entry = newentry; @@ -1292,7 +1318,7 @@ static void dictDefragBucket(dictEntry **bucketref, dictDefragFunctions *defragf if (newkey) entry->key = newkey; } else if (entryIsEmbedded(de)) { defragfns->defragEntryStartCb(privdata, de); - embeddedDictEntry *entry = decodeEmbeddedEntry(de), *newentry; + _dictEntryEmbedded *entry = decodeEntryEmbedded(de), *newentry; if ((newentry = defragalloc(entry))) { newde = encodeMaskedPtr(newentry, ENTRY_PTR_EMBEDDED); entry = newentry; @@ -1303,10 +1329,12 @@ static void dictDefragBucket(dictEntry **bucketref, dictDefragFunctions *defragf if (newval) entry->v.val = newval; } else { assert(entryIsNormal(de)); - newde = defragalloc(de); - if (newde) de = newde; - if (newkey) de->key = newkey; - if (newval) de->v.val = newval; + _dictEntryNormal *entry = decodeEntryNormal(de), *newentry; + newentry = defragalloc(entry); + newde = encodeMaskedPtr(newentry, ENTRY_PTR_NORMAL); + if (newde) entry = newentry; + if (newkey) entry->key = newkey; + if (newval) entry->v.val = newval; } if (newde) { *bucketref = newde; From 36182691ba45bac8352116a769eb33b90ae02515 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Thu, 4 Jul 2024 19:42:53 -0700 Subject: [PATCH 02/15] Fix clang-format errors Signed-off-by: Ping Xie --- src/dict.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dict.c b/src/dict.c index 9329fba45e..495e608836 100644 --- a/src/dict.c +++ b/src/dict.c @@ -212,7 +212,7 @@ static inline dictEntry *createEntryNormal(void *key, dictEntry *next) { entry->key = key; entry->next = next; return encodeMaskedPtr(entry, ENTRY_PTR_NORMAL); - } +} /* Creates an entry without a value field. */ static inline dictEntry *createEntryNoValue(void *key, dictEntry *next) { @@ -581,7 +581,7 @@ dictEntry *dictInsertAtPosition(dict *d, void *key, void *position) { * Insert the element in top, with the assumption that in a database * system it is more likely that recently added entries are accessed * more frequently. */ - if (d->type->no_value) { + if (d->type->no_value) { if (d->type->keys_are_odd && !*bucket) { /* We can store the key directly in the destination bucket without the * allocated entry. */ From 8230cb64859379d9e8d4e18749c0be8d8860b27f Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Thu, 4 Jul 2024 19:52:25 -0700 Subject: [PATCH 03/15] Fix build break Signed-off-by: Ping Xie --- src/dict.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dict.c b/src/dict.c index 495e608836..9709d8ef9a 100644 --- a/src/dict.c +++ b/src/dict.c @@ -878,7 +878,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table _dictEntryNoValue *_de = decodeEntryNoValue(e); \ _de->key = k; \ } else { \ - assert(false); \ + assert(0); \ } \ } @@ -891,7 +891,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ _de->f = v; \ } else { \ - assert(false); \ + assert(0); \ } \ } @@ -904,7 +904,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ _de->f += v; \ } else { \ - assert(false); \ + assert(0); \ } \ } From d042af5a16f39bda877e61b43ca790fd08a69517 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Thu, 4 Jul 2024 19:59:22 -0700 Subject: [PATCH 04/15] Fix clang-format errors Signed-off-by: Ping Xie --- src/dict.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dict.c b/src/dict.c index 9709d8ef9a..30ac664704 100644 --- a/src/dict.c +++ b/src/dict.c @@ -878,7 +878,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table _dictEntryNoValue *_de = decodeEntryNoValue(e); \ _de->key = k; \ } else { \ - assert(0); \ + assert(0); \ } \ } @@ -891,7 +891,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ _de->f = v; \ } else { \ - assert(0); \ + assert(0); \ } \ } @@ -904,7 +904,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ _de->f += v; \ } else { \ - assert(0); \ + assert(0); \ } \ } From ac0ad45fe13c2631ab66d64a16fa723460e4ddd9 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Mon, 19 Aug 2024 21:57:15 -0700 Subject: [PATCH 05/15] Address review feedback Signed-off-by: Ping Xie --- src/dict.c | 239 ++++++++++++++++++++++++++--------------------------- 1 file changed, 115 insertions(+), 124 deletions(-) diff --git a/src/dict.c b/src/dict.c index 5b5e3ea6f9..cf00ab69aa 100644 --- a/src/dict.c +++ b/src/dict.c @@ -79,7 +79,7 @@ typedef struct { double d; } v; struct dictEntry *next; /* Next entry in the same hash bucket. */ -} _dictEntryNormal; +} dictEntryNormal; typedef struct { union { @@ -91,39 +91,68 @@ typedef struct { struct dictEntry *next; /* Next entry in the same hash bucket. */ uint8_t key_header_size; /* offset into key_buf where the key is located at. */ unsigned char key_buf[]; /* buffer with embedded key. */ -} _dictEntryEmbedded; +} dictEntryEmbedded; /* Validation and helper for `_dictEntryEmbedded` */ -static_assert(offsetof(_dictEntryEmbedded, v) == 0, "unexpected field offset"); -static_assert(offsetof(_dictEntryEmbedded, next) == sizeof(double), "unexpected field offset"); -static_assert(offsetof(_dictEntryEmbedded, key_header_size) == sizeof(double) + sizeof(void *), +static_assert(offsetof(dictEntryEmbedded, v) == 0, "unexpected field offset"); +static_assert(offsetof(dictEntryEmbedded, next) == sizeof(double), "unexpected field offset"); +static_assert(offsetof(dictEntryEmbedded, key_header_size) == sizeof(double) + sizeof(void *), "unexpected field offset"); /* key_buf is located after a union with a double value `v.d`, a pointer `next` and uint8_t field `key_header_size` */ -static_assert(offsetof(_dictEntryEmbedded, key_buf) == sizeof(double) + sizeof(void *) + sizeof(uint8_t), +static_assert(offsetof(dictEntryEmbedded, key_buf) == sizeof(double) + sizeof(void *) + sizeof(uint8_t), "unexpected field offset"); /* The minimum amount of bytes required for embedded dict entry. */ static inline size_t compactSizeEmbeddedDictEntry(void) { - return offsetof(_dictEntryEmbedded, key_buf); + return offsetof(dictEntryEmbedded, key_buf); } typedef struct { void *key; dictEntry *next; -} _dictEntryNoValue; +} dictEntryNoValue; /* -------------------------- private prototypes ---------------------------- */ - -static void _dictExpandIfNeeded(dict *d); -static void _dictShrinkIfNeeded(dict *d); -static signed char _dictNextExp(unsigned long size); -static int _dictInit(dict *d, dictType *type); static dictEntry *dictGetNext(const dictEntry *de); static dictEntry **dictGetNextRef(dictEntry *de); static void dictSetNext(dictEntry *de, dictEntry *next); /* -------------------------- Utility functions -------------------------------- */ +static void dictShrinkIfAutoResizeAllowed(dict *d) { + /* Automatic resizing is disallowed. Return */ + if (d->pauseAutoResize > 0) return; + + dictShrinkIfNeeded(d); +} + +/* Expand the hash table if needed */ +static void dictExpandIfAutoResizeAllowed(dict *d) { + /* Automatic resizing is disallowed. Return */ + if (d->pauseAutoResize > 0) return; + + dictExpandIfNeeded(d); +} + +/* Our hash table capability is a power of two */ +static signed char dictNextExp(unsigned long size) { + if (size <= DICT_HT_INITIAL_SIZE) return DICT_HT_INITIAL_EXP; + if (size >= LONG_MAX) return (8 * sizeof(long) - 1); + + return 8 * sizeof(long) - __builtin_clzl(size - 1); +} + +/* This function performs just a step of rehashing, and only if hashing has + * 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 + * some elements can be missed or duplicated. + * + * This function is called by common lookup or update operations in the + * dictionary so that the hash table automatically migrates from H1 to H2 + * while it is actively used. */ +static void dictRehashStep(dict *d) { + if (d->pauserehash == 0) dictRehash(d, 1); +} /* Validates dict type members dependencies. */ static inline void validateDictType(dictType *type) { @@ -172,7 +201,7 @@ uint64_t dictGenCaseHashFunction(const unsigned char *buf, size_t len) { #define ENTRY_PTR_NORMAL 0 /* 000 */ #define ENTRY_PTR_NO_VALUE 2 /* 010 */ #define ENTRY_PTR_EMBEDDED 4 /* 100 */ -#define ENTRY_PTR_IS_KEY 1 /* 001 */ +#define ENTRY_PTR_IS_KEY 1 /* XX1 */ /* Returns 1 if the entry pointer is a pointer to a key, rather than to an * allocated entry. Returns 0 otherwise. */ @@ -197,18 +226,15 @@ static inline int entryIsEmbedded(const void *de) { } static inline dictEntry *encodeMaskedPtr(const void *ptr, unsigned int bits) { - assert(((uintptr_t)ptr & ENTRY_PTR_MASK) == 0); return (dictEntry *)(void *)((uintptr_t)ptr | bits); } static inline void *decodeMaskedPtr(const dictEntry *de) { - assert(!entryIsKey(de)); return (void *)((uintptr_t)(void *)de & ~ENTRY_PTR_MASK); } static inline dictEntry *createEntryNormal(void *key, dictEntry *next) { - _dictEntryNormal *entry = zmalloc(sizeof(_dictEntryNormal)); - assert(entryIsNormal(entry)); /* Check alignment of allocation */ + dictEntryNormal *entry = zmalloc(sizeof(dictEntryNormal)); entry->key = key; entry->next = next; return encodeMaskedPtr(entry, ENTRY_PTR_NORMAL); @@ -216,7 +242,7 @@ static inline dictEntry *createEntryNormal(void *key, dictEntry *next) { /* Creates an entry without a value field. */ static inline dictEntry *createEntryNoValue(void *key, dictEntry *next) { - _dictEntryNoValue *entry = zmalloc(sizeof(*entry)); + dictEntryNoValue *entry = zmalloc(sizeof(*entry)); entry->key = key; entry->next = next; return encodeMaskedPtr(entry, ENTRY_PTR_NO_VALUE); @@ -224,28 +250,28 @@ static inline dictEntry *createEntryNoValue(void *key, dictEntry *next) { static inline dictEntry *createEmbeddedEntry(void *key, dictEntry *next, dictType *dt) { size_t key_len = dt->embedKey(NULL, 0, key, NULL); - _dictEntryEmbedded *entry = zmalloc(compactSizeEmbeddedDictEntry() + key_len); + dictEntryEmbedded *entry = zmalloc(compactSizeEmbeddedDictEntry() + key_len); dt->embedKey(entry->key_buf, key_len, key, &entry->key_header_size); entry->next = next; return encodeMaskedPtr(entry, ENTRY_PTR_EMBEDDED); } static inline void *getEmbeddedKey(const dictEntry *de) { - _dictEntryEmbedded *entry = (_dictEntryEmbedded *)decodeMaskedPtr(de); + dictEntryEmbedded *entry = (dictEntryEmbedded *)decodeMaskedPtr(de); return &entry->key_buf[entry->key_header_size]; } /* Decodes the pointer to an entry without value, when you know it is an entry * without value. Hint: Use entryIsNoValue to check. */ -static inline _dictEntryNoValue *decodeEntryNoValue(const dictEntry *de) { +static inline dictEntryNoValue *decodeEntryNoValue(const dictEntry *de) { return decodeMaskedPtr(de); } -static inline _dictEntryEmbedded *decodeEntryEmbedded(const dictEntry *de) { +static inline dictEntryEmbedded *decodeEntryEmbedded(const dictEntry *de) { return decodeMaskedPtr(de); } -static inline _dictEntryNormal *decodeEntryNormal(const dictEntry *de) { +static inline dictEntryNormal *decodeEntryNormal(const dictEntry *de) { return decodeMaskedPtr(de); } @@ -257,12 +283,23 @@ static inline int entryHasValue(const dictEntry *de) { /* ----------------------------- API implementation ------------------------- */ /* Reset hash table parameters already initialized with _dictInit()*/ -static void _dictReset(dict *d, int htidx) { +static void dictReset(dict *d, int htidx) { d->ht_table[htidx] = NULL; d->ht_size_exp[htidx] = -1; d->ht_used[htidx] = 0; } +/* Initialize the hash table */ +static int dictInit(dict *d, dictType *type) { + dictReset(d, 0); + dictReset(d, 1); + d->type = type; + d->rehashidx = -1; + d->pauserehash = 0; + d->pauseAutoResize = 0; + return DICT_OK; +} + /* Create a new hash table */ dict *dictCreate(dictType *type) { validateDictType(type); @@ -271,25 +308,14 @@ dict *dictCreate(dictType *type) { if (metasize > 0) { memset(dictMetadata(d), 0, metasize); } - _dictInit(d, type); + dictInit(d, type); return d; } -/* Initialize the hash table */ -int _dictInit(dict *d, dictType *type) { - _dictReset(d, 0); - _dictReset(d, 1); - d->type = type; - d->rehashidx = -1; - d->pauserehash = 0; - d->pauseAutoResize = 0; - return DICT_OK; -} - /* Resize or create the hash table, * when malloc_failed is non-NULL, it'll avoid panic if malloc fails (in which case it'll be set to 1). * Returns DICT_OK if resize was performed, and DICT_ERR if skipped. */ -int _dictResize(dict *d, unsigned long size, int *malloc_failed) { +static int dictResizeWithOptionalCheck(dict *d, unsigned long size, int *malloc_failed) { if (malloc_failed) *malloc_failed = 0; /* We can't rehash twice if rehashing is ongoing. */ @@ -298,7 +324,7 @@ int _dictResize(dict *d, unsigned long size, int *malloc_failed) { /* the new hash table */ dictEntry **new_ht_table; unsigned long new_ht_used; - signed char new_ht_size_exp = _dictNextExp(size); + signed char new_ht_size_exp = dictNextExp(size); /* Detect overflows */ size_t newsize = DICTHT_SIZE(new_ht_size_exp); @@ -336,7 +362,7 @@ int _dictResize(dict *d, unsigned long size, int *malloc_failed) { d->ht_size_exp[0] = new_ht_size_exp; d->ht_used[0] = new_ht_used; d->ht_table[0] = new_ht_table; - _dictReset(d, 1); + dictReset(d, 1); d->rehashidx = -1; return DICT_OK; } @@ -350,22 +376,22 @@ int _dictResize(dict *d, unsigned long size, int *malloc_failed) { return DICT_OK; } -int _dictExpand(dict *d, unsigned long size, int *malloc_failed) { +static int dictExpandWithOptionalCheck(dict *d, unsigned long size, int *malloc_failed) { /* the size is invalid if it is smaller than the size of the hash table * or smaller than the number of elements already inside the hash table */ if (dictIsRehashing(d) || d->ht_used[0] > size || DICTHT_SIZE(d->ht_size_exp[0]) >= size) return DICT_ERR; - return _dictResize(d, size, malloc_failed); + return dictResizeWithOptionalCheck(d, size, malloc_failed); } /* return DICT_ERR if expand was not performed */ int dictExpand(dict *d, unsigned long size) { - return _dictExpand(d, size, NULL); + return dictExpandWithOptionalCheck(d, size, NULL); } /* return DICT_ERR if expand failed due to memory allocation failure */ int dictTryExpand(dict *d, unsigned long size) { int malloc_failed = 0; - _dictExpand(d, size, &malloc_failed); + dictExpandWithOptionalCheck(d, size, &malloc_failed); return malloc_failed ? DICT_ERR : DICT_OK; } @@ -374,7 +400,7 @@ int dictShrink(dict *d, unsigned long size) { /* the size is invalid if it is bigger than the size of the hash table * or smaller than the number of elements already inside the hash table */ if (dictIsRehashing(d) || d->ht_used[0] > size || DICTHT_SIZE(d->ht_size_exp[0]) <= size) return DICT_ERR; - return _dictResize(d, size, NULL); + return dictResizeWithOptionalCheck(d, size, NULL); } /* Helper function for `dictRehash` and `dictBucketRehash` which rehashes all the keys @@ -433,7 +459,7 @@ static int dictCheckRehashingCompleted(dict *d) { d->ht_table[0] = d->ht_table[1]; d->ht_used[0] = d->ht_used[1]; d->ht_size_exp[0] = d->ht_size_exp[1]; - _dictReset(d, 1); + dictReset(d, 1); d->rehashidx = -1; return 1; } @@ -500,20 +526,8 @@ int dictRehashMicroseconds(dict *d, uint64_t us) { return rehashes; } -/* This function performs just a step of rehashing, and only if hashing has - * 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 - * some elements can be missed or duplicated. - * - * This function is called by common lookup or update operations in the - * dictionary so that the hash table automatically migrates from H1 to H2 - * while it is actively used. */ -static void _dictRehashStep(dict *d) { - if (d->pauserehash == 0) dictRehash(d, 1); -} - /* Performs rehashing on a single bucket. */ -int _dictBucketRehash(dict *d, uint64_t idx) { +static int dictBucketRehash(dict *d, uint64_t idx) { if (d->pauserehash != 0) return 0; unsigned long s0 = DICTHT_SIZE(d->ht_size_exp[0]); unsigned long s1 = DICTHT_SIZE(d->ht_size_exp[1]); @@ -666,11 +680,11 @@ static dictEntry *dictGenericDelete(dict *d, const void *key, int nofree) { if ((long)idx >= d->rehashidx && d->ht_table[0][idx]) { /* If we have a valid hash entry at `idx` in ht0, we perform * rehash on the bucket at `idx` (being more CPU cache friendly) */ - _dictBucketRehash(d, idx); + dictBucketRehash(d, idx); } else { /* If the hash entry is not in ht0, we rehash the buckets based * on the rehashidx (not CPU cache friendly). */ - _dictRehashStep(d); + dictRehashStep(d); } } @@ -691,7 +705,7 @@ static dictEntry *dictGenericDelete(dict *d, const void *key, int nofree) { dictFreeUnlinkedEntry(d, he); } d->ht_used[table]--; - _dictShrinkIfNeeded(d); + dictShrinkIfAutoResizeAllowed(d); return he; } prevHe = he; @@ -744,7 +758,7 @@ void dictFreeUnlinkedEntry(dict *d, dictEntry *he) { } /* Destroy an entire dictionary */ -int _dictClear(dict *d, int htidx, void(callback)(dict *)) { +static int dictClear(dict *d, int htidx, void(callback)(dict *)) { unsigned long i; /* Free all the elements */ @@ -766,7 +780,7 @@ int _dictClear(dict *d, int htidx, void(callback)(dict *)) { /* Free the table and the allocated cache structure */ zfree(d->ht_table[htidx]); /* Re-initialize the table */ - _dictReset(d, htidx); + dictReset(d, htidx); return DICT_OK; /* never fails */ } @@ -775,8 +789,8 @@ void dictRelease(dict *d) { /* Someone may be monitoring a dict that started rehashing, before * destroying the dict fake completion. */ if (dictIsRehashing(d) && d->type->rehashingCompleted) d->type->rehashingCompleted(d); - _dictClear(d, 0, NULL); - _dictClear(d, 1, NULL); + dictClear(d, 0, NULL); + dictClear(d, 1, NULL); zfree(d); } @@ -793,11 +807,11 @@ dictEntry *dictFind(dict *d, const void *key) { if ((long)idx >= d->rehashidx && d->ht_table[0][idx]) { /* If we have a valid hash entry at `idx` in ht0, we perform * rehash on the bucket at `idx` (being more CPU cache friendly) */ - _dictBucketRehash(d, idx); + dictBucketRehash(d, idx); } else { /* If the hash entry is not in ht0, we rehash the buckets based * on the rehashidx (not CPU cache friendly). */ - _dictRehashStep(d); + dictRehashStep(d); } } @@ -842,7 +856,7 @@ dictEntry *dictTwoPhaseUnlinkFind(dict *d, const void *key, dictEntry ***plink, uint64_t h, idx, table; if (dictSize(d) == 0) return NULL; /* dict is empty */ - if (dictIsRehashing(d)) _dictRehashStep(d); + if (dictIsRehashing(d)) dictRehashStep(d); h = dictHashKey(d, key); for (table = 0; table <= 1; table++) { @@ -871,54 +885,55 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictFreeKey(d, he); dictFreeVal(d, he); if (!entryIsKey(he)) zfree(decodeMaskedPtr(he)); - _dictShrinkIfNeeded(d); + dictShrinkIfAutoResizeAllowed(d); dictResumeRehashing(d); } +/* In the macros below, `e` stands for entry, `k` for key, and `v` for value. */ #define DICT_SET_KEY(e, k) \ { \ if (entryIsNormal(e)) { \ - _dictEntryNormal *_de = decodeEntryNormal(e); \ + dictEntryNormal *_de = decodeEntryNormal(e); \ _de->key = k; \ } else if (entryIsNoValue(e)) { \ - _dictEntryNoValue *_de = decodeEntryNoValue(e); \ + dictEntryNoValue *_de = decodeEntryNoValue(e); \ _de->key = k; \ } else { \ - assert(0); \ + assert(!"Entry type not supported"); \ } \ } - #define DICT_SET_VALUE(e, f, v) \ { \ if (entryIsNormal(e)) { \ - _dictEntryNormal *_de = decodeEntryNormal(e); \ + dictEntryNormal *_de = decodeEntryNormal(e); \ _de->f = v; \ } else if (entryIsEmbedded(e)) { \ - _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ + dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ _de->f = v; \ } else { \ - assert(0); \ + assert(!"Entry type not supported"); \ } \ } - #define DICT_INCR_VALUE(e, f, v) \ { \ if (entryIsNormal(e)) { \ - _dictEntryNormal *_de = decodeEntryNormal(e); \ + dictEntryNormal *_de = decodeEntryNormal(e); \ _de->f += v; \ } else if (entryIsEmbedded(e)) { \ - _dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ + dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ _de->f += v; \ } else { \ - assert(0); \ + assert(!"Entry type not supported"); \ } \ } - #define DICT_GET_VALUE(e, f) \ - (assert(entryHasValue(e)), entryIsNormal(e) ? decodeEntryNormal(e)->f : decodeEntryEmbedded(e)->f) - + (entryIsNormal(e) ? decodeEntryNormal(e)->f \ + : (entryIsEmbedded(e) ? decodeEntryEmbedded(e)->f \ + : (assert(!"Entry type not supported"), ((dictEntryNormal *)e)->f))) #define DICT_GET_VALUE_PTR(e, f) \ - (assert(entryHasValue(e)), entryIsNormal(e) ? &decodeEntryNormal(e)->f : &decodeEntryEmbedded(e)->f) + (entryIsNormal(e) \ + ? &decodeEntryNormal(e)->f \ + : (entryIsEmbedded(e) ? &decodeEntryEmbedded(e)->f : (assert(!"Entry type not supported"), NULL))) void dictSetKey(dict *d, dictEntry *de, void *key) { void *k = d->type->keyDup ? d->type->keyDup(d, key) : key; @@ -1004,7 +1019,6 @@ static dictEntry **dictGetNextRef(dictEntry *de) { } static void dictSetNext(dictEntry *de, dictEntry *next) { - assert(!entryIsKey(de)); if (entryIsNoValue(de)) { decodeEntryNoValue(de)->next = next; } else if (entryIsEmbedded(de)) { @@ -1018,22 +1032,22 @@ static void dictSetNext(dictEntry *de, dictEntry *next) { /* Returns the memory usage in bytes of the dict, excluding the size of the keys * and values. */ size_t dictMemUsage(const dict *d) { - return dictSize(d) * sizeof(_dictEntryNormal) + dictBuckets(d) * sizeof(dictEntry *); + return dictSize(d) * sizeof(dictEntryNormal) + dictBuckets(d) * sizeof(dictEntry *); } /* Returns the memory usage in bytes of dictEntry based on the type. if `de` is NULL, return the size of * regular dict entry else return based on the type. */ size_t dictEntryMemUsage(dictEntry *de) { if (de == NULL || entryIsNormal(de)) - return sizeof(_dictEntryNormal); + return sizeof(dictEntryNormal); else if (entryIsKey(de)) return 0; else if (entryIsNoValue(de)) - return sizeof(_dictEntryNoValue); + return sizeof(dictEntryNoValue); else if (entryIsEmbedded(de)) return zmalloc_size(decodeEntryEmbedded(de)); else - assert("Entry type not supported"); + assert(!"Entry type not supported"); return 0; } @@ -1166,7 +1180,7 @@ dictEntry *dictGetRandomKey(dict *d) { int listlen, listele; if (dictSize(d) == 0) return NULL; - if (dictIsRehashing(d)) _dictRehashStep(d); + if (dictIsRehashing(d)) dictRehashStep(d); if (dictIsRehashing(d)) { unsigned long s0 = DICTHT_SIZE(d->ht_size_exp[0]); do { @@ -1233,7 +1247,7 @@ unsigned int dictGetSomeKeys(dict *d, dictEntry **des, unsigned int count) { /* Try to do a rehashing work proportional to 'count'. */ for (j = 0; j < count; j++) { if (dictIsRehashing(d)) - _dictRehashStep(d); + dictRehashStep(d); else break; } @@ -1316,7 +1330,7 @@ static void dictDefragBucket(dictEntry **bucketref, dictDefragFunctions *defragf if (newkey) *bucketref = newkey; assert(entryIsKey(*bucketref)); } else if (entryIsNoValue(de)) { - _dictEntryNoValue *entry = decodeEntryNoValue(de), *newentry; + dictEntryNoValue *entry = decodeEntryNoValue(de), *newentry; if ((newentry = defragalloc(entry))) { newde = encodeMaskedPtr(newentry, ENTRY_PTR_NO_VALUE); entry = newentry; @@ -1324,7 +1338,7 @@ static void dictDefragBucket(dictEntry **bucketref, dictDefragFunctions *defragf if (newkey) entry->key = newkey; } else if (entryIsEmbedded(de)) { defragfns->defragEntryStartCb(privdata, de); - _dictEntryEmbedded *entry = decodeEntryEmbedded(de), *newentry; + dictEntryEmbedded *entry = decodeEntryEmbedded(de), *newentry; if ((newentry = defragalloc(entry))) { newde = encodeMaskedPtr(newentry, ENTRY_PTR_EMBEDDED); entry = newentry; @@ -1335,7 +1349,7 @@ static void dictDefragBucket(dictEntry **bucketref, dictDefragFunctions *defragf if (newval) entry->v.val = newval; } else { assert(entryIsNormal(de)); - _dictEntryNormal *entry = decodeEntryNormal(de), *newentry; + dictEntryNormal *entry = decodeEntryNormal(de), *newentry; newentry = defragalloc(entry); newde = encodeMaskedPtr(newentry, ENTRY_PTR_NORMAL); if (newde) entry = newentry; @@ -1576,7 +1590,7 @@ dictScanDefrag(dict *d, unsigned long v, dictScanFunction *fn, dictDefragFunctio * type has resizeAllowed member function. */ static int dictTypeResizeAllowed(dict *d, size_t size) { if (d->type->resizeAllowed == NULL) return 1; - return d->type->resizeAllowed(DICTHT_SIZE(_dictNextExp(size)) * sizeof(dictEntry *), + return d->type->resizeAllowed(DICTHT_SIZE(dictNextExp(size)) * sizeof(dictEntry *), (double)d->ht_used[0] / DICTHT_SIZE(d->ht_size_exp[0])); } @@ -1606,14 +1620,6 @@ int dictExpandIfNeeded(dict *d) { return DICT_ERR; } -/* Expand the hash table if needed */ -static void _dictExpandIfNeeded(dict *d) { - /* Automatic resizing is disallowed. Return */ - if (d->pauseAutoResize > 0) return; - - dictExpandIfNeeded(d); -} - /* Returning DICT_OK indicates a successful shrinking or the dictionary is undergoing rehashing, * and there is nothing else we need to do about this dictionary currently. While DICT_ERR indicates * that shrinking has not been triggered (may be try expanding?)*/ @@ -1637,21 +1643,6 @@ int dictShrinkIfNeeded(dict *d) { return DICT_ERR; } -static void _dictShrinkIfNeeded(dict *d) { - /* Automatic resizing is disallowed. Return */ - if (d->pauseAutoResize > 0) return; - - dictShrinkIfNeeded(d); -} - -/* Our hash table capability is a power of two */ -static signed char _dictNextExp(unsigned long size) { - if (size <= DICT_HT_INITIAL_SIZE) return DICT_HT_INITIAL_EXP; - if (size >= LONG_MAX) return (8 * sizeof(long) - 1); - - return 8 * sizeof(long) - __builtin_clzl(size - 1); -} - /* Finds and returns the position within the dict where the provided key should * be inserted using dictInsertAtPosition if the key does not already exist in * the dict. If the key exists in the dict, NULL is returned and the optional @@ -1667,16 +1658,16 @@ void *dictFindPositionForInsert(dict *d, const void *key, dictEntry **existing) if ((long)idx >= d->rehashidx && d->ht_table[0][idx]) { /* If we have a valid hash entry at `idx` in ht0, we perform * rehash on the bucket at `idx` (being more CPU cache friendly) */ - _dictBucketRehash(d, idx); + dictBucketRehash(d, idx); } else { /* If the hash entry is not in ht0, we rehash the buckets based * on the rehashidx (not CPU cache friendly). */ - _dictRehashStep(d); + dictRehashStep(d); } } /* Expand the hash table if needed */ - _dictExpandIfNeeded(d); + dictExpandIfAutoResizeAllowed(d); for (table = 0; table <= 1; table++) { if (table == 0 && (long)idx < d->rehashidx) continue; idx = hash & DICTHT_SIZE_MASK(d->ht_size_exp[table]); @@ -1703,8 +1694,8 @@ void dictEmpty(dict *d, void(callback)(dict *)) { /* Someone may be monitoring a dict that started rehashing, before * destroying the dict fake completion. */ if (dictIsRehashing(d) && d->type->rehashingCompleted) d->type->rehashingCompleted(d); - _dictClear(d, 0, callback); - _dictClear(d, 1, callback); + dictClear(d, 0, callback); + dictClear(d, 1, callback); d->rehashidx = -1; d->pauserehash = 0; d->pauseAutoResize = 0; From cd072e216dad8fcf941c8f052715c7d7ea768faa Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Mon, 19 Aug 2024 22:12:26 -0700 Subject: [PATCH 06/15] Remove unused function Signed-off-by: Ping Xie --- src/dict.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/dict.c b/src/dict.c index cf00ab69aa..31b065b2ab 100644 --- a/src/dict.c +++ b/src/dict.c @@ -275,11 +275,6 @@ static inline dictEntryNormal *decodeEntryNormal(const dictEntry *de) { return decodeMaskedPtr(de); } -/* Returns 1 if the entry has a value field and 0 otherwise. */ -static inline int entryHasValue(const dictEntry *de) { - return entryIsNormal(de) || entryIsEmbedded(de); -} - /* ----------------------------- API implementation ------------------------- */ /* Reset hash table parameters already initialized with _dictInit()*/ From cd75c359e205e0bac8c7573a10d55bfc1a6b4ce9 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Sun, 25 Aug 2024 16:33:04 -0700 Subject: [PATCH 07/15] Use more descriptive variable names Signed-off-by: Ping Xie --- src/dict.c | 57 +++++++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/src/dict.c b/src/dict.c index 31b065b2ab..c29259260c 100644 --- a/src/dict.c +++ b/src/dict.c @@ -884,51 +884,52 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictResumeRehashing(d); } -/* In the macros below, `e` stands for entry, `k` for key, and `v` for value. */ -#define DICT_SET_KEY(e, k) \ +/* In the macros below, `de` stands for dict entry, and `k` for key. */ +#define DICT_SET_KEY(de, k) \ { \ - if (entryIsNormal(e)) { \ - dictEntryNormal *_de = decodeEntryNormal(e); \ + if (entryIsNormal(de)) { \ + dictEntryNormal *_de = decodeEntryNormal(de); \ _de->key = k; \ - } else if (entryIsNoValue(e)) { \ - dictEntryNoValue *_de = decodeEntryNoValue(e); \ + } else if (entryIsNoValue(de)) { \ + dictEntryNoValue *_de = decodeEntryNoValue(de); \ _de->key = k; \ } else { \ assert(!"Entry type not supported"); \ } \ } -#define DICT_SET_VALUE(e, f, v) \ +#define DICT_SET_VALUE(de, field, val) \ { \ - if (entryIsNormal(e)) { \ - dictEntryNormal *_de = decodeEntryNormal(e); \ - _de->f = v; \ - } else if (entryIsEmbedded(e)) { \ - dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ - _de->f = v; \ + if (entryIsNormal(de)) { \ + dictEntryNormal *_de = decodeEntryNormal(de); \ + _de->field = val; \ + } else if (entryIsEmbedded(de)) { \ + dictEntryEmbedded *_de = decodeEntryEmbedded(de); \ + _de->field = val; \ } else { \ assert(!"Entry type not supported"); \ } \ } -#define DICT_INCR_VALUE(e, f, v) \ +#define DICT_INCR_VALUE(de, field, val) \ { \ - if (entryIsNormal(e)) { \ - dictEntryNormal *_de = decodeEntryNormal(e); \ - _de->f += v; \ - } else if (entryIsEmbedded(e)) { \ - dictEntryEmbedded *_de = decodeEntryEmbedded(e); \ - _de->f += v; \ + if (entryIsNormal(de)) { \ + dictEntryNormal *_de = decodeEntryNormal(de); \ + _de->field += val; \ + } else if (entryIsEmbedded(de)) { \ + dictEntryEmbedded *_de = decodeEntryEmbedded(de); \ + _de->field += val; \ } else { \ assert(!"Entry type not supported"); \ } \ } -#define DICT_GET_VALUE(e, f) \ - (entryIsNormal(e) ? decodeEntryNormal(e)->f \ - : (entryIsEmbedded(e) ? decodeEntryEmbedded(e)->f \ - : (assert(!"Entry type not supported"), ((dictEntryNormal *)e)->f))) -#define DICT_GET_VALUE_PTR(e, f) \ - (entryIsNormal(e) \ - ? &decodeEntryNormal(e)->f \ - : (entryIsEmbedded(e) ? &decodeEntryEmbedded(e)->f : (assert(!"Entry type not supported"), NULL))) +#define DICT_GET_VALUE(de, field) \ + (entryIsNormal(de) \ + ? decodeEntryNormal(de)->field \ + : (entryIsEmbedded(de) ? decodeEntryEmbedded(de)->field \ + : (assert(!"Entry type not supported"), ((dictEntryNormal *)de)->field))) +#define DICT_GET_VALUE_PTR(de, field) \ + (entryIsNormal(de) \ + ? &decodeEntryNormal(de)->field \ + : (entryIsEmbedded(de) ? &decodeEntryEmbedded(de)->field : (assert(!"Entry type not supported"), NULL))) void dictSetKey(dict *d, dictEntry *de, void *key) { void *k = d->type->keyDup ? d->type->keyDup(d, key) : key; From 550e83297c3f3d02d3f4da75dcd68553871d0ff2 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Sun, 25 Aug 2024 17:44:39 -0700 Subject: [PATCH 08/15] Keep the core changes and move refactoring changes into a different PR Signed-off-by: Ping Xie --- src/dict.c | 165 ++++++++++++++++++++++++++++------------------------- 1 file changed, 88 insertions(+), 77 deletions(-) diff --git a/src/dict.c b/src/dict.c index 31b065b2ab..49ff2078da 100644 --- a/src/dict.c +++ b/src/dict.c @@ -114,45 +114,16 @@ typedef struct { } dictEntryNoValue; /* -------------------------- private prototypes ---------------------------- */ + +static void _dictExpandIfNeeded(dict *d); +static void _dictShrinkIfNeeded(dict *d); +static signed char _dictNextExp(unsigned long size); +static int _dictInit(dict *d, dictType *type); static dictEntry *dictGetNext(const dictEntry *de); static dictEntry **dictGetNextRef(dictEntry *de); static void dictSetNext(dictEntry *de, dictEntry *next); /* -------------------------- Utility functions -------------------------------- */ -static void dictShrinkIfAutoResizeAllowed(dict *d) { - /* Automatic resizing is disallowed. Return */ - if (d->pauseAutoResize > 0) return; - - dictShrinkIfNeeded(d); -} - -/* Expand the hash table if needed */ -static void dictExpandIfAutoResizeAllowed(dict *d) { - /* Automatic resizing is disallowed. Return */ - if (d->pauseAutoResize > 0) return; - - dictExpandIfNeeded(d); -} - -/* Our hash table capability is a power of two */ -static signed char dictNextExp(unsigned long size) { - if (size <= DICT_HT_INITIAL_SIZE) return DICT_HT_INITIAL_EXP; - if (size >= LONG_MAX) return (8 * sizeof(long) - 1); - - return 8 * sizeof(long) - __builtin_clzl(size - 1); -} - -/* This function performs just a step of rehashing, and only if hashing has - * 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 - * some elements can be missed or duplicated. - * - * This function is called by common lookup or update operations in the - * dictionary so that the hash table automatically migrates from H1 to H2 - * while it is actively used. */ -static void dictRehashStep(dict *d) { - if (d->pauserehash == 0) dictRehash(d, 1); -} /* Validates dict type members dependencies. */ static inline void validateDictType(dictType *type) { @@ -278,23 +249,12 @@ static inline dictEntryNormal *decodeEntryNormal(const dictEntry *de) { /* ----------------------------- API implementation ------------------------- */ /* Reset hash table parameters already initialized with _dictInit()*/ -static void dictReset(dict *d, int htidx) { +static void _dictReset(dict *d, int htidx) { d->ht_table[htidx] = NULL; d->ht_size_exp[htidx] = -1; d->ht_used[htidx] = 0; } -/* Initialize the hash table */ -static int dictInit(dict *d, dictType *type) { - dictReset(d, 0); - dictReset(d, 1); - d->type = type; - d->rehashidx = -1; - d->pauserehash = 0; - d->pauseAutoResize = 0; - return DICT_OK; -} - /* Create a new hash table */ dict *dictCreate(dictType *type) { validateDictType(type); @@ -303,14 +263,25 @@ dict *dictCreate(dictType *type) { if (metasize > 0) { memset(dictMetadata(d), 0, metasize); } - dictInit(d, type); + _dictInit(d, type); return d; } +/* Initialize the hash table */ +int _dictInit(dict *d, dictType *type) { + _dictReset(d, 0); + _dictReset(d, 1); + d->type = type; + d->rehashidx = -1; + d->pauserehash = 0; + d->pauseAutoResize = 0; + return DICT_OK; +} + /* Resize or create the hash table, * when malloc_failed is non-NULL, it'll avoid panic if malloc fails (in which case it'll be set to 1). * Returns DICT_OK if resize was performed, and DICT_ERR if skipped. */ -static int dictResizeWithOptionalCheck(dict *d, unsigned long size, int *malloc_failed) { +int _dictResize(dict *d, unsigned long size, int *malloc_failed) { if (malloc_failed) *malloc_failed = 0; /* We can't rehash twice if rehashing is ongoing. */ @@ -319,7 +290,7 @@ static int dictResizeWithOptionalCheck(dict *d, unsigned long size, int *malloc_ /* the new hash table */ dictEntry **new_ht_table; unsigned long new_ht_used; - signed char new_ht_size_exp = dictNextExp(size); + signed char new_ht_size_exp = _dictNextExp(size); /* Detect overflows */ size_t newsize = DICTHT_SIZE(new_ht_size_exp); @@ -357,7 +328,7 @@ static int dictResizeWithOptionalCheck(dict *d, unsigned long size, int *malloc_ d->ht_size_exp[0] = new_ht_size_exp; d->ht_used[0] = new_ht_used; d->ht_table[0] = new_ht_table; - dictReset(d, 1); + _dictReset(d, 1); d->rehashidx = -1; return DICT_OK; } @@ -371,22 +342,22 @@ static int dictResizeWithOptionalCheck(dict *d, unsigned long size, int *malloc_ return DICT_OK; } -static int dictExpandWithOptionalCheck(dict *d, unsigned long size, int *malloc_failed) { +int _dictExpand(dict *d, unsigned long size, int *malloc_failed) { /* the size is invalid if it is smaller than the size of the hash table * or smaller than the number of elements already inside the hash table */ if (dictIsRehashing(d) || d->ht_used[0] > size || DICTHT_SIZE(d->ht_size_exp[0]) >= size) return DICT_ERR; - return dictResizeWithOptionalCheck(d, size, malloc_failed); + return _dictResize(d, size, malloc_failed); } /* return DICT_ERR if expand was not performed */ int dictExpand(dict *d, unsigned long size) { - return dictExpandWithOptionalCheck(d, size, NULL); + return _dictExpand(d, size, NULL); } /* return DICT_ERR if expand failed due to memory allocation failure */ int dictTryExpand(dict *d, unsigned long size) { int malloc_failed = 0; - dictExpandWithOptionalCheck(d, size, &malloc_failed); + _dictExpand(d, size, &malloc_failed); return malloc_failed ? DICT_ERR : DICT_OK; } @@ -395,7 +366,7 @@ int dictShrink(dict *d, unsigned long size) { /* the size is invalid if it is bigger than the size of the hash table * or smaller than the number of elements already inside the hash table */ if (dictIsRehashing(d) || d->ht_used[0] > size || DICTHT_SIZE(d->ht_size_exp[0]) <= size) return DICT_ERR; - return dictResizeWithOptionalCheck(d, size, NULL); + return _dictResize(d, size, NULL); } /* Helper function for `dictRehash` and `dictBucketRehash` which rehashes all the keys @@ -420,7 +391,12 @@ static void rehashEntriesInBucketAtIndex(dict *d, uint64_t idx) { if (d->type->keys_are_odd && !d->ht_table[1][h]) { /* Destination bucket is empty and we can store the key * directly without an allocated entry. Free the old entry - * if it's an allocated entry. */ + * if it's an allocated entry. + * + * TODO: Add a flag 'keys_are_even' and if set, we can use + * this optimization for these dicts too. We can set the LSB + * bit when stored as a dict entry and clear it again when + * we need the key back. */ assert(entryIsKey(key)); if (!entryIsKey(de)) zfree(decodeMaskedPtr(de)); de = key; @@ -454,7 +430,7 @@ static int dictCheckRehashingCompleted(dict *d) { d->ht_table[0] = d->ht_table[1]; d->ht_used[0] = d->ht_used[1]; d->ht_size_exp[0] = d->ht_size_exp[1]; - dictReset(d, 1); + _dictReset(d, 1); d->rehashidx = -1; return 1; } @@ -521,8 +497,20 @@ int dictRehashMicroseconds(dict *d, uint64_t us) { return rehashes; } +/* This function performs just a step of rehashing, and only if hashing has + * 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 + * some elements can be missed or duplicated. + * + * This function is called by common lookup or update operations in the + * dictionary so that the hash table automatically migrates from H1 to H2 + * while it is actively used. */ +static void _dictRehashStep(dict *d) { + if (d->pauserehash == 0) dictRehash(d, 1); +} + /* Performs rehashing on a single bucket. */ -static int dictBucketRehash(dict *d, uint64_t idx) { +int _dictBucketRehash(dict *d, uint64_t idx) { if (d->pauserehash != 0) return 0; unsigned long s0 = DICTHT_SIZE(d->ht_size_exp[0]); unsigned long s1 = DICTHT_SIZE(d->ht_size_exp[1]); @@ -675,11 +663,11 @@ static dictEntry *dictGenericDelete(dict *d, const void *key, int nofree) { if ((long)idx >= d->rehashidx && d->ht_table[0][idx]) { /* If we have a valid hash entry at `idx` in ht0, we perform * rehash on the bucket at `idx` (being more CPU cache friendly) */ - dictBucketRehash(d, idx); + _dictBucketRehash(d, idx); } else { /* If the hash entry is not in ht0, we rehash the buckets based * on the rehashidx (not CPU cache friendly). */ - dictRehashStep(d); + _dictRehashStep(d); } } @@ -700,7 +688,7 @@ static dictEntry *dictGenericDelete(dict *d, const void *key, int nofree) { dictFreeUnlinkedEntry(d, he); } d->ht_used[table]--; - dictShrinkIfAutoResizeAllowed(d); + _dictShrinkIfNeeded(d); return he; } prevHe = he; @@ -753,7 +741,7 @@ void dictFreeUnlinkedEntry(dict *d, dictEntry *he) { } /* Destroy an entire dictionary */ -static int dictClear(dict *d, int htidx, void(callback)(dict *)) { +int _dictClear(dict *d, int htidx, void(callback)(dict *)) { unsigned long i; /* Free all the elements */ @@ -775,7 +763,7 @@ static int dictClear(dict *d, int htidx, void(callback)(dict *)) { /* Free the table and the allocated cache structure */ zfree(d->ht_table[htidx]); /* Re-initialize the table */ - dictReset(d, htidx); + _dictReset(d, htidx); return DICT_OK; /* never fails */ } @@ -784,8 +772,8 @@ void dictRelease(dict *d) { /* Someone may be monitoring a dict that started rehashing, before * destroying the dict fake completion. */ if (dictIsRehashing(d) && d->type->rehashingCompleted) d->type->rehashingCompleted(d); - dictClear(d, 0, NULL); - dictClear(d, 1, NULL); + _dictClear(d, 0, NULL); + _dictClear(d, 1, NULL); zfree(d); } @@ -802,11 +790,11 @@ dictEntry *dictFind(dict *d, const void *key) { if ((long)idx >= d->rehashidx && d->ht_table[0][idx]) { /* If we have a valid hash entry at `idx` in ht0, we perform * rehash on the bucket at `idx` (being more CPU cache friendly) */ - dictBucketRehash(d, idx); + _dictBucketRehash(d, idx); } else { /* If the hash entry is not in ht0, we rehash the buckets based * on the rehashidx (not CPU cache friendly). */ - dictRehashStep(d); + _dictRehashStep(d); } } @@ -851,7 +839,7 @@ dictEntry *dictTwoPhaseUnlinkFind(dict *d, const void *key, dictEntry ***plink, uint64_t h, idx, table; if (dictSize(d) == 0) return NULL; /* dict is empty */ - if (dictIsRehashing(d)) dictRehashStep(d); + if (dictIsRehashing(d)) _dictRehashStep(d); h = dictHashKey(d, key); for (table = 0; table <= 1; table++) { @@ -880,7 +868,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictFreeKey(d, he); dictFreeVal(d, he); if (!entryIsKey(he)) zfree(decodeMaskedPtr(he)); - dictShrinkIfAutoResizeAllowed(d); + _dictShrinkIfNeeded(d); dictResumeRehashing(d); } @@ -1175,7 +1163,7 @@ dictEntry *dictGetRandomKey(dict *d) { int listlen, listele; if (dictSize(d) == 0) return NULL; - if (dictIsRehashing(d)) dictRehashStep(d); + if (dictIsRehashing(d)) _dictRehashStep(d); if (dictIsRehashing(d)) { unsigned long s0 = DICTHT_SIZE(d->ht_size_exp[0]); do { @@ -1242,7 +1230,7 @@ unsigned int dictGetSomeKeys(dict *d, dictEntry **des, unsigned int count) { /* Try to do a rehashing work proportional to 'count'. */ for (j = 0; j < count; j++) { if (dictIsRehashing(d)) - dictRehashStep(d); + _dictRehashStep(d); else break; } @@ -1585,7 +1573,7 @@ dictScanDefrag(dict *d, unsigned long v, dictScanFunction *fn, dictDefragFunctio * type has resizeAllowed member function. */ static int dictTypeResizeAllowed(dict *d, size_t size) { if (d->type->resizeAllowed == NULL) return 1; - return d->type->resizeAllowed(DICTHT_SIZE(dictNextExp(size)) * sizeof(dictEntry *), + return d->type->resizeAllowed(DICTHT_SIZE(_dictNextExp(size)) * sizeof(dictEntry *), (double)d->ht_used[0] / DICTHT_SIZE(d->ht_size_exp[0])); } @@ -1615,6 +1603,14 @@ int dictExpandIfNeeded(dict *d) { return DICT_ERR; } +/* Expand the hash table if needed */ +static void _dictExpandIfNeeded(dict *d) { + /* Automatic resizing is disallowed. Return */ + if (d->pauseAutoResize > 0) return; + + dictExpandIfNeeded(d); +} + /* Returning DICT_OK indicates a successful shrinking or the dictionary is undergoing rehashing, * and there is nothing else we need to do about this dictionary currently. While DICT_ERR indicates * that shrinking has not been triggered (may be try expanding?)*/ @@ -1638,6 +1634,21 @@ int dictShrinkIfNeeded(dict *d) { return DICT_ERR; } +static void _dictShrinkIfNeeded(dict *d) { + /* Automatic resizing is disallowed. Return */ + if (d->pauseAutoResize > 0) return; + + dictShrinkIfNeeded(d); +} + +/* Our hash table capability is a power of two */ +static signed char _dictNextExp(unsigned long size) { + if (size <= DICT_HT_INITIAL_SIZE) return DICT_HT_INITIAL_EXP; + if (size >= LONG_MAX) return (8 * sizeof(long) - 1); + + return 8 * sizeof(long) - __builtin_clzl(size - 1); +} + /* Finds and returns the position within the dict where the provided key should * be inserted using dictInsertAtPosition if the key does not already exist in * the dict. If the key exists in the dict, NULL is returned and the optional @@ -1653,16 +1664,16 @@ void *dictFindPositionForInsert(dict *d, const void *key, dictEntry **existing) if ((long)idx >= d->rehashidx && d->ht_table[0][idx]) { /* If we have a valid hash entry at `idx` in ht0, we perform * rehash on the bucket at `idx` (being more CPU cache friendly) */ - dictBucketRehash(d, idx); + _dictBucketRehash(d, idx); } else { /* If the hash entry is not in ht0, we rehash the buckets based * on the rehashidx (not CPU cache friendly). */ - dictRehashStep(d); + _dictRehashStep(d); } } /* Expand the hash table if needed */ - dictExpandIfAutoResizeAllowed(d); + _dictExpandIfNeeded(d); for (table = 0; table <= 1; table++) { if (table == 0 && (long)idx < d->rehashidx) continue; idx = hash & DICTHT_SIZE_MASK(d->ht_size_exp[table]); @@ -1689,8 +1700,8 @@ void dictEmpty(dict *d, void(callback)(dict *)) { /* Someone may be monitoring a dict that started rehashing, before * destroying the dict fake completion. */ if (dictIsRehashing(d) && d->type->rehashingCompleted) d->type->rehashingCompleted(d); - dictClear(d, 0, callback); - dictClear(d, 1, callback); + _dictClear(d, 0, callback); + _dictClear(d, 1, callback); d->rehashidx = -1; d->pauserehash = 0; d->pauseAutoResize = 0; From 8a48ec8e5ee4af95b014a88db3d61338eba88aa2 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Sun, 25 Aug 2024 20:42:14 -0700 Subject: [PATCH 09/15] Use `panic` in lieu of unconditional assert Signed-off-by: Ping Xie --- src/dict.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dict.c b/src/dict.c index bc8163088e..faa4c05aba 100644 --- a/src/dict.c +++ b/src/dict.c @@ -882,7 +882,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictEntryNoValue *_de = decodeEntryNoValue(de); \ _de->key = k; \ } else { \ - assert(!"Entry type not supported"); \ + panic("Entry type not supported"); \ } \ } #define DICT_SET_VALUE(de, field, val) \ @@ -894,7 +894,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictEntryEmbedded *_de = decodeEntryEmbedded(de); \ _de->field = val; \ } else { \ - assert(!"Entry type not supported"); \ + panic("Entry type not supported"); \ } \ } #define DICT_INCR_VALUE(de, field, val) \ @@ -906,18 +906,18 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictEntryEmbedded *_de = decodeEntryEmbedded(de); \ _de->field += val; \ } else { \ - assert(!"Entry type not supported"); \ + panic("Entry type not supported"); \ } \ } #define DICT_GET_VALUE(de, field) \ (entryIsNormal(de) \ ? decodeEntryNormal(de)->field \ : (entryIsEmbedded(de) ? decodeEntryEmbedded(de)->field \ - : (assert(!"Entry type not supported"), ((dictEntryNormal *)de)->field))) + : (panic("Entry type not supported"), ((dictEntryNormal *)de)->field))) #define DICT_GET_VALUE_PTR(de, field) \ (entryIsNormal(de) \ ? &decodeEntryNormal(de)->field \ - : (entryIsEmbedded(de) ? &decodeEntryEmbedded(de)->field : (assert(!"Entry type not supported"), NULL))) + : (entryIsEmbedded(de) ? &decodeEntryEmbedded(de)->field : (panic("Entry type not supported"), NULL))) void dictSetKey(dict *d, dictEntry *de, void *key) { void *k = d->type->keyDup ? d->type->keyDup(d, key) : key; @@ -1031,7 +1031,7 @@ size_t dictEntryMemUsage(dictEntry *de) { else if (entryIsEmbedded(de)) return zmalloc_size(decodeEntryEmbedded(de)); else - assert(!"Entry type not supported"); + panic("Entry type not supported"); return 0; } From e827090110d9c35c4d1cd59779908161615fa8cd Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Sun, 25 Aug 2024 20:51:23 -0700 Subject: [PATCH 10/15] Fix clang-format breaks Signed-off-by: Ping Xie --- src/dict.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/dict.c b/src/dict.c index faa4c05aba..2575a2d763 100644 --- a/src/dict.c +++ b/src/dict.c @@ -882,7 +882,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictEntryNoValue *_de = decodeEntryNoValue(de); \ _de->key = k; \ } else { \ - panic("Entry type not supported"); \ + panic("Entry type not supported"); \ } \ } #define DICT_SET_VALUE(de, field, val) \ @@ -894,7 +894,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictEntryEmbedded *_de = decodeEntryEmbedded(de); \ _de->field = val; \ } else { \ - panic("Entry type not supported"); \ + panic("Entry type not supported"); \ } \ } #define DICT_INCR_VALUE(de, field, val) \ @@ -906,14 +906,13 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictEntryEmbedded *_de = decodeEntryEmbedded(de); \ _de->field += val; \ } else { \ - panic("Entry type not supported"); \ + panic("Entry type not supported"); \ } \ } #define DICT_GET_VALUE(de, field) \ - (entryIsNormal(de) \ - ? decodeEntryNormal(de)->field \ - : (entryIsEmbedded(de) ? decodeEntryEmbedded(de)->field \ - : (panic("Entry type not supported"), ((dictEntryNormal *)de)->field))) + (entryIsNormal(de) ? decodeEntryNormal(de)->field \ + : (entryIsEmbedded(de) ? decodeEntryEmbedded(de)->field \ + : (panic("Entry type not supported"), ((dictEntryNormal *)de)->field))) #define DICT_GET_VALUE_PTR(de, field) \ (entryIsNormal(de) \ ? &decodeEntryNormal(de)->field \ From ac8af533431ddb4e3dd8961fa789928e91b5cc37 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Mon, 2 Sep 2024 08:54:53 -0700 Subject: [PATCH 11/15] Convert DICT_SET_KEY to a function Signed-off-by: Ping Xie --- src/dict.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/dict.c b/src/dict.c index 2575a2d763..e98a075e4a 100644 --- a/src/dict.c +++ b/src/dict.c @@ -872,19 +872,19 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictResumeRehashing(d); } -/* In the macros below, `de` stands for dict entry, and `k` for key. */ -#define DICT_SET_KEY(de, k) \ - { \ - if (entryIsNormal(de)) { \ - dictEntryNormal *_de = decodeEntryNormal(de); \ - _de->key = k; \ - } else if (entryIsNoValue(de)) { \ - dictEntryNoValue *_de = decodeEntryNoValue(de); \ - _de->key = k; \ - } else { \ - panic("Entry type not supported"); \ - } \ +static inline void dictSetEntryKey(void *de, char *k) { + if (entryIsNormal(de)) { + dictEntryNormal *_de = decodeEntryNormal(de); + _de->key = k; + } else if (entryIsNoValue(de)) { + dictEntryNoValue *_de = decodeEntryNoValue(de); + _de->key = k; + } else { + panic("Entry type not supported"); } +} + +/* In the macros below, `de` stands for dict entry, and `k` for key. */ #define DICT_SET_VALUE(de, field, val) \ { \ if (entryIsNormal(de)) { \ @@ -920,7 +920,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table void dictSetKey(dict *d, dictEntry *de, void *key) { void *k = d->type->keyDup ? d->type->keyDup(d, key) : key; - DICT_SET_KEY(de, k); + dictSetEntryKey(de, k); } void dictSetVal(dict *d, dictEntry *de, void *val) { From 872b5eed42c935014517a9c0948e1850aaa3c137 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Mon, 2 Sep 2024 09:13:42 -0700 Subject: [PATCH 12/15] minor name change Signed-off-by: Ping Xie --- src/dict.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dict.c b/src/dict.c index e98a075e4a..0d541e2c57 100644 --- a/src/dict.c +++ b/src/dict.c @@ -872,13 +872,13 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictResumeRehashing(d); } -static inline void dictSetEntryKey(void *de, char *k) { +static inline void dictSetEntryKey(void *de, char *key) { if (entryIsNormal(de)) { dictEntryNormal *_de = decodeEntryNormal(de); - _de->key = k; + _de->key = key; } else if (entryIsNoValue(de)) { dictEntryNoValue *_de = decodeEntryNoValue(de); - _de->key = k; + _de->key = key; } else { panic("Entry type not supported"); } From ab988ff61b115e4350bc1a3547a064dd3a0ae6c4 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Mon, 2 Sep 2024 11:28:58 -0700 Subject: [PATCH 13/15] Update dict.c Co-authored-by: Madelyn Olson Signed-off-by: Ping Xie --- src/dict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dict.c b/src/dict.c index 0d541e2c57..ad9ac13c45 100644 --- a/src/dict.c +++ b/src/dict.c @@ -93,7 +93,7 @@ typedef struct { unsigned char key_buf[]; /* buffer with embedded key. */ } dictEntryEmbedded; -/* Validation and helper for `_dictEntryEmbedded` */ +/* Validation and helper for `dictEntryEmbedded` */ static_assert(offsetof(dictEntryEmbedded, v) == 0, "unexpected field offset"); static_assert(offsetof(dictEntryEmbedded, next) == sizeof(double), "unexpected field offset"); From 457aead40675674582d89549c8357165ecdd9ac8 Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Mon, 2 Sep 2024 11:29:10 -0700 Subject: [PATCH 14/15] Update dict.c Co-authored-by: Madelyn Olson Signed-off-by: Ping Xie --- src/dict.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dict.c b/src/dict.c index ad9ac13c45..16ef74ae40 100644 --- a/src/dict.c +++ b/src/dict.c @@ -884,7 +884,7 @@ static inline void dictSetEntryKey(void *de, char *key) { } } -/* In the macros below, `de` stands for dict entry, and `k` for key. */ +/* In the macros below, `de` stands for dict entry. */ #define DICT_SET_VALUE(de, field, val) \ { \ if (entryIsNormal(de)) { \ From e4bb137af27ce5b1fd0055c8fe8012a615c7a5da Mon Sep 17 00:00:00 2001 From: Ping Xie Date: Mon, 2 Sep 2024 11:31:31 -0700 Subject: [PATCH 15/15] Address review feedback Signed-off-by: Ping Xie --- src/dict.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/dict.c b/src/dict.c index 16ef74ae40..37aca8887e 100644 --- a/src/dict.c +++ b/src/dict.c @@ -872,17 +872,6 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table dictResumeRehashing(d); } -static inline void dictSetEntryKey(void *de, char *key) { - if (entryIsNormal(de)) { - dictEntryNormal *_de = decodeEntryNormal(de); - _de->key = key; - } else if (entryIsNoValue(de)) { - dictEntryNoValue *_de = decodeEntryNoValue(de); - _de->key = key; - } else { - panic("Entry type not supported"); - } -} /* In the macros below, `de` stands for dict entry. */ #define DICT_SET_VALUE(de, field, val) \ @@ -920,7 +909,15 @@ static inline void dictSetEntryKey(void *de, char *key) { void dictSetKey(dict *d, dictEntry *de, void *key) { void *k = d->type->keyDup ? d->type->keyDup(d, key) : key; - dictSetEntryKey(de, k); + if (entryIsNormal(de)) { + dictEntryNormal *_de = decodeEntryNormal(de); + _de->key = k; + } else if (entryIsNoValue(de)) { + dictEntryNoValue *_de = decodeEntryNoValue(de); + _de->key = k; + } else { + panic("Entry type not supported"); + } } void dictSetVal(dict *d, dictEntry *de, void *val) {