Replace dict with thin wrapper around hashtable

cmake/Modules/SourceFiles.cmake

@@ -10,7 +10,6 @@ set(VALKEY_SERVER_SRCS
     ${CMAKE_SOURCE_DIR}/src/quicklist.c
     ${CMAKE_SOURCE_DIR}/src/ae.c
     ${CMAKE_SOURCE_DIR}/src/anet.c
-    ${CMAKE_SOURCE_DIR}/src/dict.c
     ${CMAKE_SOURCE_DIR}/src/hashtable.c
     ${CMAKE_SOURCE_DIR}/src/kvstore.c
     ${CMAKE_SOURCE_DIR}/src/sds.c
@@ -128,7 +127,7 @@ set(VALKEY_SERVER_SRCS
 set(VALKEY_CLI_SRCS
     ${CMAKE_SOURCE_DIR}/src/anet.c
     ${CMAKE_SOURCE_DIR}/src/adlist.c
-    ${CMAKE_SOURCE_DIR}/src/dict.c
+    ${CMAKE_SOURCE_DIR}/src/hashtable.c
     ${CMAKE_SOURCE_DIR}/src/sds.c
     ${CMAKE_SOURCE_DIR}/src/sha256.c
     ${CMAKE_SOURCE_DIR}/src/util.c
@@ -159,7 +158,7 @@ set(VALKEY_BENCHMARK_SRCS
     ${CMAKE_SOURCE_DIR}/src/valkey-benchmark.c
     ${CMAKE_SOURCE_DIR}/src/valkey_strtod.c
     ${CMAKE_SOURCE_DIR}/src/adlist.c
-    ${CMAKE_SOURCE_DIR}/src/dict.c
+    ${CMAKE_SOURCE_DIR}/src/hashtable.c
     ${CMAKE_SOURCE_DIR}/src/zmalloc.c
     ${CMAKE_SOURCE_DIR}/src/serverassert.c
     ${CMAKE_SOURCE_DIR}/src/release.c

deps/CMakeLists.txt

@@ -25,8 +25,7 @@ if (USE_RDMA) # Module or no module
         set(ENABLE_DLOPEN_RDMA ON CACHE BOOL "Build valkey_rdma with dynamic loading")
     endif()
 endif ()
-# Let libvalkey use sds and dict provided by valkey.
-set(DICT_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/src)
+# Let libvalkey use sds provided by valkey.
 set(SDS_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/src)
 
 add_subdirectory(libvalkey)

deps/Makefile

@@ -46,7 +46,7 @@ distclean:
 
 .PHONY: distclean
 
-LIBVALKEY_MAKE_FLAGS = SDS_INCLUDE_DIR=../../src/ DICT_INCLUDE_DIR=../../src/
+LIBVALKEY_MAKE_FLAGS = SDS_INCLUDE_DIR=../../src/
 ifneq (,$(filter $(BUILD_TLS),yes module))
     LIBVALKEY_MAKE_FLAGS += USE_TLS=1
 endif

deps/libvalkey/CMakeLists.txt

@@ -46,19 +46,16 @@ set(valkey_sources
     src/command.c
     src/conn.c
     src/crc16.c
+    src/dict.c
     src/net.c
     src/read.c
     src/sockcompat.c
     src/valkey.c
     src/vkutil.c)
 
-# Allow the libvalkey provided sds and dict types to be replaced by
+# Allow the libvalkey provided sds type to be replaced by
 # compatible implementations (like Valkey's).
 # A replaced type is not included in a built archive or shared library.
-if(NOT DICT_INCLUDE_DIR)
-  set(valkey_sources ${valkey_sources} src/dict.c)
-  set(DICT_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src)
-endif()
 if(NOT SDS_INCLUDE_DIR)
   set(valkey_sources ${valkey_sources} src/sds.c)
   set(SDS_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src)
@@ -107,7 +104,6 @@ TARGET_INCLUDE_DIRECTORIES(valkey
         $<INSTALL_INTERFACE:include>
         $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/valkey>
     PRIVATE
-        $<BUILD_INTERFACE:${DICT_INCLUDE_DIR}>
         $<BUILD_INTERFACE:${SDS_INCLUDE_DIR}>
 )
 
@@ -212,7 +208,6 @@ IF(ENABLE_TLS)
         PRIVATE
             $<INSTALL_INTERFACE:include>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/valkey>
-            $<BUILD_INTERFACE:${DICT_INCLUDE_DIR}>
             $<BUILD_INTERFACE:${SDS_INCLUDE_DIR}>
     )
 
@@ -289,7 +284,6 @@ if(ENABLE_RDMA)
         PRIVATE
             $<INSTALL_INTERFACE:include>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/valkey>
-            $<BUILD_INTERFACE:${DICT_INCLUDE_DIR}>
             $<BUILD_INTERFACE:${SDS_INCLUDE_DIR}>
     )
 

deps/libvalkey/Makefile

@@ -22,13 +22,9 @@ HEADERS = $(filter-out $(INCLUDE_DIR)/tls.h $(INCLUDE_DIR)/rdma.h, $(wildcard $(
 # compatible implementations (like Valkey's).
 # A replaced type is not included in a built archive or shared library.
 SDS_INCLUDE_DIR ?= $(SRC_DIR)
-DICT_INCLUDE_DIR ?= $(SRC_DIR)
 ifneq ($(SDS_INCLUDE_DIR),$(SRC_DIR))
   SOURCES := $(filter-out $(SRC_DIR)/sds.c, $(SOURCES))
 endif
-ifneq ($(DICT_INCLUDE_DIR),$(SRC_DIR))
-  SOURCES := $(filter-out $(SRC_DIR)/dict.c, $(SOURCES))
-endif
 
 OBJS = $(patsubst $(SRC_DIR)/%.c,$(OBJ_DIR)/%.o,$(SOURCES))
 
@@ -268,10 +264,10 @@ $(RDMA_STLIBNAME): $(RDMA_OBJS)
 	$(STLIB_MAKE_CMD) $(RDMA_STLIBNAME) $(RDMA_OBJS)
 
 $(OBJ_DIR)/%.o: $(SRC_DIR)/%.c | $(OBJ_DIR)
-	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -I$(SDS_INCLUDE_DIR) -I$(DICT_INCLUDE_DIR) -MMD -MP -c $< -o $@
+	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -I$(SDS_INCLUDE_DIR) -MMD -MP -c $< -o $@
 
 $(OBJ_DIR)/%.o: $(TEST_DIR)/%.c | $(OBJ_DIR)
-	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -I$(SDS_INCLUDE_DIR) -I$(DICT_INCLUDE_DIR) -I$(SRC_DIR) -MMD -MP -c $< -o $@
+	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -I$(SDS_INCLUDE_DIR) -I$(SRC_DIR) -MMD -MP -c $< -o $@
 
 $(TEST_DIR)/%: $(OBJ_DIR)/%.o $(STLIBNAME) $(TLS_STLIB)
 	$(CC) -o $@ $< $(RDMA_STLIB) $(STLIBNAME) $(TLS_STLIB) $(REAL_LDFLAGS) $(TEST_LDFLAGS)

deps/libvalkey/src/async.c

@@ -47,8 +47,8 @@
 #include "net.h"
 #include "valkey_private.h"
 #include "vkutil.h"
+#include "dict.h"
 
-#include <dict.h>
 #include <sds.h>
 
 #include <assert.h>

deps/libvalkey/src/cluster.c

@@ -39,8 +39,8 @@
 #include "alloc.h"
 #include "command.h"
 #include "vkutil.h"
+#include "dict.h"
 
-#include <dict.h>
 #include <sds.h>
 
 #include <assert.h>

src/Makefile

@@ -478,7 +478,6 @@ ENGINE_SERVER_OBJ = \
     db.o \
     debug.o \
     defrag.o \
-    dict.o \
     entry.o \
     eval.o \
     evict.o \
@@ -574,7 +573,7 @@ ENGINE_CLI_OBJ = \
     crc64.o \
     crccombine.o \
     crcspeed.o \
-    dict.o \
+    hashtable.o \
     monotonic.o \
     mt19937-64.o \
     release.o \
@@ -598,9 +597,9 @@ ENGINE_BENCHMARK_OBJ = \
     crc64.o \
     crccombine.o \
     crcspeed.o \
-    dict.o \
     fuzzer_client.o \
     fuzzer_command_generator.o \
+    hashtable.o \
     monotonic.o \
     mt19937-64.o \
     release.o \

src/cluster_legacy.c

@@ -236,39 +236,33 @@ static_assert(offsetof(clusterMsg, type) + sizeof(uint16_t) == RCVBUF_MIN_READ_L
 /* Cluster nodes hash table, mapping nodes addresses 1.2.3.4:6379 to
  * clusterNode structures. */
 dictType clusterNodesDictType = {
-    dictSdsHash,       /* hash function */
-    NULL,              /* key dup */
-    dictSdsKeyCompare, /* key compare */
-    dictSdsDestructor, /* key destructor */
-    NULL,              /* val destructor */
-    NULL               /* allow to expand */
+    .entryGetKey = dictEntryGetKey,
+    .hashFunction = dictSdsHash,
+    .keyCompare = dictSdsKeyCompare,
+    .entryDestructor = dictEntryDestructorSdsKey,
 };
 
 /* Cluster re-addition blacklist. This maps node IDs to the time
  * we can re-add this node. The goal is to avoid reading a removed
  * node for some time. */
 dictType clusterNodesBlackListDictType = {
-    dictSdsCaseHash,       /* hash function */
-    NULL,                  /* key dup */
-    dictSdsKeyCaseCompare, /* key compare */
-    dictSdsDestructor,     /* key destructor */
-    NULL,                  /* val destructor */
-    NULL                   /* allow to expand */
+    .entryGetKey = dictEntryGetKey,
+    .hashFunction = dictSdsCaseHash,
+    .keyCompare = dictSdsKeyCaseCompare,
+    .entryDestructor = dictEntryDestructorSdsKey,
 };
 
 /* Cluster shards hash table, mapping shard id to list of nodes */
 dictType clusterSdsToListType = {
-    dictSdsHash,        /* hash function */
-    NULL,               /* key dup */
-    dictSdsKeyCompare,  /* key compare */
-    dictSdsDestructor,  /* key destructor */
-    dictListDestructor, /* val destructor */
-    NULL                /* allow to expand */
+    .entryGetKey = dictEntryGetKey,
+    .hashFunction = dictSdsHash,
+    .keyCompare = dictSdsKeyCompare,
+    .entryDestructor = dictEntryDestructorSdsKeyListValue,
 };
 
 static uint64_t dictPtrHash(const void *key) {
     /* We hash the pointer value itself. */
-    return dictGenHashFunction(&key, sizeof(key));
+    return dictGenHashFunction((const char *)&key, sizeof(key));
 }
 
 static int dictPtrCompare(const void *key1, const void *key2) {
@@ -278,12 +272,10 @@ static int dictPtrCompare(const void *key1, const void *key2) {
 /* Dictionary type for mapping hash slots to cluster nodes.
  * Keys are slot numbers encoded directly as pointer values, values are clusterNode pointers. */
 dictType clusterSlotDictType = {
-    dictPtrHash,    /* hash function */
-    NULL,           /* key dup */
-    dictPtrCompare, /* key compare */
-    NULL,           /* key destructor */
-    NULL,           /* val destructor */
-    NULL            /* allow to expand */
+    .entryGetKey = dictEntryGetKey,
+    .hashFunction = dictPtrHash,
+    .keyCompare = dictPtrCompare,
+    .entryDestructor = zfree,
 };
 
 typedef struct {
@@ -4852,7 +4844,7 @@ void clusterSendPing(clusterLink *link, int type) {
     int candidates_wanted = wanted + 2; /* +2 for myself and link->node */
     if (candidates_wanted > (int)dictSize(server.cluster->nodes))
         candidates_wanted = dictSize(server.cluster->nodes);
-    dictEntry **candidates = zmalloc(sizeof(dictEntry *) * candidates_wanted);
+    void **candidates = zmalloc(sizeof(void *) * candidates_wanted);
     unsigned int ncandidates = dictGetSomeKeys(server.cluster->nodes, candidates, candidates_wanted);
 
     for (unsigned int i = 0; i < ncandidates && gossipcount < wanted; i++) {

src/config.c

@@ -1041,21 +1041,17 @@ void rewriteConfigSentinelOption(struct rewriteConfigState *state);
  * like "maxmemory" -> list of line numbers (first line is zero).
  */
 dictType optionToLineDictType = {
-    dictSdsCaseHash,       /* hash function */
-    NULL,                  /* key dup */
-    dictSdsKeyCaseCompare, /* key compare */
-    dictSdsDestructor,     /* key destructor */
-    dictListDestructor,    /* val destructor */
-    NULL                   /* allow to expand */
+    .entryGetKey = dictEntryGetKey,
+    .hashFunction = dictSdsCaseHash,
+    .keyCompare = dictSdsKeyCaseCompare,
+    .entryDestructor = dictEntryDestructorSdsKeyListValue,
 };
 
 dictType optionSetDictType = {
-    dictSdsCaseHash,       /* hash function */
-    NULL,                  /* key dup */
-    dictSdsKeyCaseCompare, /* key compare */
-    dictSdsDestructor,     /* key destructor */
-    NULL,                  /* val destructor */
-    NULL                   /* allow to expand */
+    .entryGetKey = dictEntryGetKey,
+    .hashFunction = dictSdsCaseHash,
+    .keyCompare = dictSdsKeyCaseCompare,
+    .entryDestructor = dictEntryDestructorSdsKey,
 };
 
 /* The config rewrite state. */

src/defrag.c

@@ -299,24 +299,50 @@ static void activeDefragZsetNode(void *privdata, void *entry_ref) {
 #define DEFRAG_SDS_DICT_VAL_VOID_PTR 3
 #define DEFRAG_SDS_DICT_VAL_LUA_SCRIPT 4
 
-static void activeDefragSdsDictCallback(void *privdata, const dictEntry *de) {
-    UNUSED(privdata);
-    UNUSED(de);
+typedef void *(dictDefragAllocFunction)(void *ptr);
+typedef struct {
+    dictDefragAllocFunction *defragKey;
+    dictDefragAllocFunction *defragVal;
+} dictDefragFunctions;
+
+static void activeDefragDictCallback(void *privdata, void *entry_ref) {
+    dictDefragFunctions *defragfns = privdata;
+    dictEntry **de_ref = (dictEntry **)entry_ref;
+    dictEntry *de = *de_ref;
+
+    /* Defrag the entry itself */
+    dictEntry *newentry = activeDefragAlloc(de);
+    if (newentry) {
+        de = newentry;
+        *de_ref = newentry;
+    }
+
+    /* Defrag the key */
+    if (defragfns->defragKey) {
+        void *newkey = defragfns->defragKey(de->key);
+        if (newkey) de->key = newkey;
+    }
+
+    /* Defrag the value */
+    if (defragfns->defragVal) {
+        void *newval = defragfns->defragVal(de->v.val);
+        if (newval) de->v.val = newval;
+    }
 }
 
 /* Defrag a dict with sds key and optional value (either ptr, sds or robj string) */
 static void activeDefragSdsDict(dict *d, int val_type) {
     unsigned long cursor = 0;
     dictDefragFunctions defragfns = {
-        .defragAlloc = activeDefragAlloc,
         .defragKey = (dictDefragAllocFunction *)activeDefragSds,
         .defragVal = (val_type == DEFRAG_SDS_DICT_VAL_IS_SDS       ? (dictDefragAllocFunction *)activeDefragSds
                       : val_type == DEFRAG_SDS_DICT_VAL_IS_STROB   ? (dictDefragAllocFunction *)activeDefragStringOb
                       : val_type == DEFRAG_SDS_DICT_VAL_VOID_PTR   ? (dictDefragAllocFunction *)activeDefragAlloc
                       : val_type == DEFRAG_SDS_DICT_VAL_LUA_SCRIPT ? (dictDefragAllocFunction *)evalActiveDefragScript
                                                                    : NULL)};
     do {
-        cursor = dictScanDefrag(d, cursor, activeDefragSdsDictCallback, &defragfns, NULL);
+        cursor = hashtableScanDefrag(d, cursor, activeDefragDictCallback,
+                                     &defragfns, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF);
     } while (cursor != 0);
 }