Simplify dictType callbacks and move some macros from dict.h to dict.c (

#1281) Remove the dict pointer argument to the `dictType` callbacks `keyDup`, `keyCompare`, `keyDestructor` and `valDestructor`. This argument was unused in all of the callback implementations. The macros `dictFreeKey()` and `dictFreeVal()` are made internal to dict and moved from dict.h to dict.c. They're also changed from macros to static inline functions. Signed-off-by: Qu Chen <[email protected]>
valkey-io · Nov 14, 2024 · 32f7541 · 32f7541
1 parent 863d312
commit 32f7541
Show file tree

Hide file tree

Showing 14 changed files with 74 additions and 98 deletions.
diff --git a/src/config.c b/src/config.c
@@ -1013,15 +1013,14 @@ void configGetCommand(client *c) {
 
 #define CONFIG_REWRITE_SIGNATURE "# Generated by CONFIG REWRITE"
 
-/* We use the following dictionary type to store where a configuration
- * option is mentioned in the old configuration file, so it's
- * like "maxmemory" -> list of line numbers (first line is zero). */
-void dictListDestructor(dict *d, void *val);
-
 /* Sentinel config rewriting is implemented inside sentinel.c by
  * rewriteConfigSentinelOption(). */
 void rewriteConfigSentinelOption(struct rewriteConfigState *state);
 
+/* We use the following dictionary type to store where a configuration
+ * option is mentioned in the old configuration file, so it's
+ * like "maxmemory" -> list of line numbers (first line is zero).
+ */
 dictType optionToLineDictType = {
     dictSdsCaseHash,       /* hash function */
     NULL,                  /* key dup */

diff --git a/src/dict.c b/src/dict.c
@@ -576,7 +576,7 @@ dictEntry *dictAddRaw(dict *d, void *key, dictEntry **existing) {
     if (!position) return NULL;
 
     /* Dup the key if necessary. */
-    if (d->type->keyDup) key = d->type->keyDup(d, key);
+    if (d->type->keyDup) key = d->type->keyDup(key);
 
     return dictInsertAtPosition(d, key, position);
 }
@@ -640,7 +640,7 @@ int dictReplace(dict *d, void *key, void *val) {
      * reverse. */
     void *oldval = dictGetVal(existing);
     dictSetVal(d, existing, val);
-    if (d->type->valDestructor) d->type->valDestructor(d, oldval);
+    if (d->type->valDestructor) d->type->valDestructor(oldval);
     return 0;
 }
 
@@ -742,6 +742,18 @@ dictEntry *dictUnlink(dict *d, const void *key) {
     return dictGenericDelete(d, key, 1);
 }
 
+inline static void dictFreeKey(dict *d, dictEntry *entry) {
+    if (d->type->keyDestructor) {
+        d->type->keyDestructor(dictGetKey(entry));
+    }
+}
+
+inline static void dictFreeVal(dict *d, dictEntry *entry) {
+    if (d->type->valDestructor) {
+        d->type->valDestructor(dictGetVal(entry));
+    }
+}
+
 /* You need to call this function to really free the entry after a call
  * to dictUnlink(). It's safe to call this function with 'he' = NULL. */
 void dictFreeUnlinkedEntry(dict *d, dictEntry *he) {
@@ -919,7 +931,7 @@ void dictTwoPhaseUnlinkFree(dict *d, dictEntry *he, dictEntry **plink, int table
          : (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;
+    void *k = d->type->keyDup ? d->type->keyDup(key) : key;
     if (entryIsNormal(de)) {
         dictEntryNormal *_de = decodeEntryNormal(de);
         _de->key = k;

diff --git a/src/dict.h b/src/dict.h
@@ -53,10 +53,10 @@ typedef struct dict dict;
 typedef struct dictType {
     /* Callbacks */
     uint64_t (*hashFunction)(const void *key);
-    void *(*keyDup)(dict *d, const void *key);
-    int (*keyCompare)(dict *d, const void *key1, const void *key2);
-    void (*keyDestructor)(dict *d, void *key);
-    void (*valDestructor)(dict *d, void *obj);
+    void *(*keyDup)(const void *key);
+    int (*keyCompare)(const void *key1, const void *key2);
+    void (*keyDestructor)(void *key);
+    void (*valDestructor)(void *obj);
     int (*resizeAllowed)(size_t moreMem, double usedRatio);
     /* Invoked at the start of dict initialization/rehashing (old and new ht are already created) */
     void (*rehashingStarted)(dict *d);
@@ -144,16 +144,13 @@ typedef struct {
 #define DICT_HT_INITIAL_SIZE (1 << (DICT_HT_INITIAL_EXP))
 
 /* ------------------------------- Macros ------------------------------------*/
-#define dictFreeVal(d, entry)                                                           \
-    do {                                                                                \
-        if ((d)->type->valDestructor) (d)->type->valDestructor((d), dictGetVal(entry)); \
-    } while (0)
-
-#define dictFreeKey(d, entry) \
-    if ((d)->type->keyDestructor) (d)->type->keyDestructor((d), dictGetKey(entry))
-
-#define dictCompareKeys(d, key1, key2) \
-    (((d)->type->keyCompare) ? (d)->type->keyCompare((d), key1, key2) : (key1) == (key2))
+static inline int dictCompareKeys(dict *d, const void *key1, const void *key2) {
+    if (d->type->keyCompare) {
+        return d->type->keyCompare(key1, key2);
+    } else {
+        return (key1 == key2);
+    }
+}
 
 #define dictMetadata(d) (&(d)->metadata)
 #define dictMetadataSize(d) ((d)->type->dictMetadataBytes ? (d)->type->dictMetadataBytes(d) : 0)

diff --git a/src/eval.c b/src/eval.c
@@ -57,8 +57,7 @@ void evalGenericCommandWithDebugging(client *c, int evalsha);
 sds ldbCatStackValue(sds s, lua_State *lua, int idx);
 listNode *luaScriptsLRUAdd(client *c, sds sha, int evalsha);
 
-static void dictLuaScriptDestructor(dict *d, void *val) {
-    UNUSED(d);
+static void dictLuaScriptDestructor(void *val) {
     if (val == NULL) return; /* Lazy freeing will set value to NULL. */
     decrRefCount(((luaScript *)val)->body);
     zfree(val);

diff --git a/src/functions.c b/src/functions.c
@@ -43,9 +43,9 @@ typedef enum {
 static size_t engine_cache_memory = 0;
 
 /* Forward declaration */
-static void engineFunctionDispose(dict *d, void *obj);
-static void engineStatsDispose(dict *d, void *obj);
-static void engineLibraryDispose(dict *d, void *obj);
+static void engineFunctionDispose(void *obj);
+static void engineStatsDispose(void *obj);
+static void engineLibraryDispose(void *obj);
 static int functionsVerifyName(sds name);
 
 typedef struct functionsLibEngineStats {
@@ -126,15 +126,13 @@ static size_t libraryMallocSize(functionLibInfo *li) {
     return zmalloc_size(li) + sdsAllocSize(li->name) + sdsAllocSize(li->code);
 }
 
-static void engineStatsDispose(dict *d, void *obj) {
-    UNUSED(d);
+static void engineStatsDispose(void *obj) {
     functionsLibEngineStats *stats = obj;
     zfree(stats);
 }
 
 /* Dispose function memory */
-static void engineFunctionDispose(dict *d, void *obj) {
-    UNUSED(d);
+static void engineFunctionDispose(void *obj) {
     if (!obj) {
         return;
     }
@@ -158,8 +156,7 @@ static void engineLibraryFree(functionLibInfo *li) {
     zfree(li);
 }
 
-static void engineLibraryDispose(dict *d, void *obj) {
-    UNUSED(d);
+static void engineLibraryDispose(void *obj) {
     engineLibraryFree(obj);
 }
 

diff --git a/src/latency.c b/src/latency.c
@@ -37,8 +37,7 @@
 #include "hdr_histogram.h"
 
 /* Dictionary type for latency events. */
-int dictStringKeyCompare(dict *d, const void *key1, const void *key2) {
-    UNUSED(d);
+int dictStringKeyCompare(const void *key1, const void *key2) {
     return strcmp(key1, key2) == 0;
 }
 

diff --git a/src/module.c b/src/module.c
@@ -11814,8 +11814,7 @@ uint64_t dictCStringKeyHash(const void *key) {
     return dictGenHashFunction((unsigned char *)key, strlen((char *)key));
 }
 
-int dictCStringKeyCompare(dict *d, const void *key1, const void *key2) {
-    UNUSED(d);
+int dictCStringKeyCompare(const void *key1, const void *key2) {
     return strcmp(key1, key2) == 0;
 }
 

diff --git a/src/sentinel.c b/src/sentinel.c
@@ -416,8 +416,7 @@ void sentinelSimFailureCrash(void);
 
 void releaseSentinelValkeyInstance(sentinelValkeyInstance *ri);
 
-void dictInstancesValDestructor(dict *d, void *obj) {
-    UNUSED(d);
+void dictInstancesValDestructor(void *obj) {
     releaseSentinelValkeyInstance(obj);
 }
 
@@ -4259,7 +4258,7 @@ void sentinelSetCommand(client *c) {
 
             /* If the target name is the same as the source name there
              * is no need to add an entry mapping to itself. */
-            if (!dictSdsKeyCaseCompare(ri->renamed_commands, oldname, newname)) {
+            if (!dictSdsKeyCaseCompare(oldname, newname)) {
                 oldname = sdsdup(oldname);
                 newname = sdsdup(newname);
                 dictAdd(ri->renamed_commands, oldname, newname);

diff --git a/src/server.c b/src/server.c
@@ -360,25 +360,20 @@ void exitFromChild(int retcode) {
  * keys and Objects as values (Objects can hold SDS strings,
  * lists, sets). */
 
-void dictVanillaFree(dict *d, void *val) {
-    UNUSED(d);
+void dictVanillaFree(void *val) {
     zfree(val);
 }
 
-void dictListDestructor(dict *d, void *val) {
-    UNUSED(d);
+void dictListDestructor(void *val) {
     listRelease((list *)val);
 }
 
-void dictDictDestructor(dict *d, void *val) {
-    UNUSED(d);
+void dictDictDestructor(void *val) {
     dictRelease((dict *)val);
 }
 
-int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
+int dictSdsKeyCompare(const void *key1, const void *key2) {
     int l1, l2;
-    UNUSED(d);
-
     l1 = sdslen((sds)key1);
     l2 = sdslen((sds)key2);
     if (l1 != l2) return 0;
@@ -391,30 +386,26 @@ size_t dictSdsEmbedKey(unsigned char *buf, size_t buf_len, const void *key, uint
 
 /* A case insensitive version used for the command lookup table and other
  * places where case insensitive non binary-safe comparison is needed. */
-int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2) {
-    UNUSED(d);
+int dictSdsKeyCaseCompare(const void *key1, const void *key2) {
     return strcasecmp(key1, key2) == 0;
 }
 
-void dictObjectDestructor(dict *d, void *val) {
-    UNUSED(d);
+void dictObjectDestructor(void *val) {
     if (val == NULL) return; /* Lazy freeing will set value to NULL. */
     decrRefCount(val);
 }
 
-void dictSdsDestructor(dict *d, void *val) {
-    UNUSED(d);
+void dictSdsDestructor(void *val) {
     sdsfree(val);
 }
 
-void *dictSdsDup(dict *d, const void *key) {
-    UNUSED(d);
+void *dictSdsDup(const void *key) {
     return sdsdup((const sds)key);
 }
 
-int dictObjKeyCompare(dict *d, const void *key1, const void *key2) {
+int dictObjKeyCompare(const void *key1, const void *key2) {
     const robj *o1 = key1, *o2 = key2;
-    return dictSdsKeyCompare(d, o1->ptr, o2->ptr);
+    return dictSdsKeyCompare(o1->ptr, o2->ptr);
 }
 
 uint64_t dictObjHash(const void *key) {
@@ -446,29 +437,25 @@ uint64_t dictClientHash(const void *key) {
 }
 
 /* Dict compare function for client */
-int dictClientKeyCompare(dict *d, const void *key1, const void *key2) {
-    UNUSED(d);
+int dictClientKeyCompare(const void *key1, const void *key2) {
     return ((client *)key1)->id == ((client *)key2)->id;
 }
 
 /* Dict compare function for null terminated string */
-int dictCStrKeyCompare(dict *d, const void *key1, const void *key2) {
+int dictCStrKeyCompare(const void *key1, const void *key2) {
     int l1, l2;
-    UNUSED(d);
-
     l1 = strlen((char *)key1);
     l2 = strlen((char *)key2);
     if (l1 != l2) return 0;
     return memcmp(key1, key2, l1) == 0;
 }
 
 /* Dict case insensitive compare function for null terminated string */
-int dictCStrKeyCaseCompare(dict *d, const void *key1, const void *key2) {
-    UNUSED(d);
+int dictCStrKeyCaseCompare(const void *key1, const void *key2) {
     return strcasecmp(key1, key2) == 0;
 }
 
-int dictEncObjKeyCompare(dict *d, const void *key1, const void *key2) {
+int dictEncObjKeyCompare(const void *key1, const void *key2) {
     robj *o1 = (robj *)key1, *o2 = (robj *)key2;
     int cmp;
 
@@ -480,7 +467,7 @@ int dictEncObjKeyCompare(dict *d, const void *key1, const void *key2) {
      * objects as well. */
     if (o1->refcount != OBJ_STATIC_REFCOUNT) o1 = getDecodedObject(o1);
     if (o2->refcount != OBJ_STATIC_REFCOUNT) o2 = getDecodedObject(o2);
-    cmp = dictSdsKeyCompare(d, o1->ptr, o2->ptr);
+    cmp = dictSdsKeyCompare(o1->ptr, o2->ptr);
     if (o1->refcount != OBJ_STATIC_REFCOUNT) decrRefCount(o1);
     if (o2->refcount != OBJ_STATIC_REFCOUNT) decrRefCount(o2);
     return cmp;

diff --git a/src/server.h b/src/server.h
@@ -2730,7 +2730,7 @@ int serverSetProcTitle(char *title);
 int validateProcTitleTemplate(const char *template);
 int serverCommunicateSystemd(const char *sd_notify_msg);
 void serverSetCpuAffinity(const char *cpulist);
-void dictVanillaFree(dict *d, void *val);
+void dictVanillaFree(void *val);
 
 /* ERROR STATS constants */
 
@@ -3717,11 +3717,11 @@ void startEvictionTimeProc(void);
 /* Keys hashing / comparison functions for dict.c hash tables. */
 uint64_t dictSdsHash(const void *key);
 uint64_t dictSdsCaseHash(const void *key);
-int dictSdsKeyCompare(dict *d, const void *key1, const void *key2);
-int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2);
-void dictSdsDestructor(dict *d, void *val);
-void dictListDestructor(dict *d, void *val);
-void *dictSdsDup(dict *d, const void *key);
+int dictSdsKeyCompare(const void *key1, const void *key2);
+int dictSdsKeyCaseCompare(const void *key1, const void *key2);
+void dictSdsDestructor(void *val);
+void dictListDestructor(void *val);
+void *dictSdsDup(const void *key);
 
 /* Git SHA1 */
 char *serverGitSHA1(void);

diff --git a/src/unit/test_dict.c b/src/unit/test_dict.c
@@ -5,19 +5,15 @@ uint64_t hashCallback(const void *key) {
     return dictGenHashFunction((unsigned char *)key, strlen((char *)key));
 }
 
-int compareCallback(dict *d, const void *key1, const void *key2) {
+int compareCallback(const void *key1, const void *key2) {
     int l1, l2;
-    UNUSED(d);
-
     l1 = strlen((char *)key1);
     l2 = strlen((char *)key2);
     if (l1 != l2) return 0;
     return memcmp(key1, key2, l1) == 0;
 }
 
-void freeCallback(dict *d, void *val) {
-    UNUSED(d);
-
+void freeCallback(void *val) {
     zfree(val);
 }
 

diff --git a/src/unit/test_kvstore.c b/src/unit/test_kvstore.c
@@ -5,8 +5,7 @@ uint64_t hashTestCallback(const void *key) {
     return dictGenHashFunction((unsigned char *)key, strlen((char *)key));
 }
 
-void freeTestCallback(dict *d, void *val) {
-    UNUSED(d);
+void freeTestCallback(void *val) {
     zfree(val);
 }
 

diff --git a/src/valkey-benchmark.c b/src/valkey-benchmark.c
@@ -199,7 +199,7 @@ static long long showThroughput(struct aeEventLoop *eventLoop, long long id, voi
 
 /* Dict callbacks */
 static uint64_t dictSdsHash(const void *key);
-static int dictSdsKeyCompare(dict *d, const void *key1, const void *key2);
+static int dictSdsKeyCompare(const void *key1, const void *key2);
 
 /* Implementation */
 static long long ustime(void) {
@@ -220,10 +220,8 @@ static uint64_t dictSdsHash(const void *key) {
     return dictGenHashFunction((unsigned char *)key, sdslen((char *)key));
 }
 
-static int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
+static int dictSdsKeyCompare(const void *key1, const void *key2) {
     int l1, l2;
-    UNUSED(d);
-
     l1 = sdslen((sds)key1);
     l2 = sdslen((sds)key2);
     if (l1 != l2) return 0;