From 81d12b47f72e3149164e8ef8b96626ff55a613b0 Mon Sep 17 00:00:00 2001 From: Rain Valentine Date: Mon, 18 Nov 2024 10:36:56 +0100 Subject: [PATCH] Replace dict with hashtable in command tables (#1065) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes the type of command tables from dict to hashtable. Command table lookup takes ~3% of overall CPU time in benchmarks, so it is a good candidate for optimization. My initial SET benchmark comparison suggests that hashtable is about 4.5 times faster than dict and this replacement reduced overall CPU time by 2.79% 🥳 --------- Signed-off-by: Rain Valentine Signed-off-by: Rain Valentine Signed-off-by: Viktor Söderqvist Co-authored-by: Rain Valentine --- src/acl.c | 62 +++++++-------- src/config.c | 12 +-- src/latency.c | 25 +++--- src/module.c | 38 ++++----- src/server.c | 216 +++++++++++++++++++++++++++----------------------- src/server.h | 19 ++--- 6 files changed, 191 insertions(+), 181 deletions(-) diff --git a/src/acl.c b/src/acl.c index 688820fd89..81d8d4ec49 100644 --- a/src/acl.c +++ b/src/acl.c @@ -652,14 +652,14 @@ void ACLChangeSelectorPerm(aclSelector *selector, struct serverCommand *cmd, int unsigned long id = cmd->id; ACLSetSelectorCommandBit(selector, id, allow); ACLResetFirstArgsForCommand(selector, id); - if (cmd->subcommands_dict) { - dictEntry *de; - dictIterator *di = dictGetSafeIterator(cmd->subcommands_dict); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *sub = (struct serverCommand *)dictGetVal(de); + if (cmd->subcommands_ht) { + hashtableIterator iter; + hashtableInitSafeIterator(&iter, cmd->subcommands_ht); + struct serverCommand *sub; + while (hashtableNext(&iter, (void **)&sub)) { ACLSetSelectorCommandBit(selector, sub->id, allow); } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } } @@ -669,19 +669,19 @@ void ACLChangeSelectorPerm(aclSelector *selector, struct serverCommand *cmd, int * value. Since the category passed by the user may be non existing, the * function returns C_ERR if the category was not found, or C_OK if it was * found and the operation was performed. */ -void ACLSetSelectorCommandBitsForCategory(dict *commands, aclSelector *selector, uint64_t cflag, int value) { - dictIterator *di = dictGetIterator(commands); - dictEntry *de; - while ((de = dictNext(di)) != NULL) { - struct serverCommand *cmd = dictGetVal(de); +void ACLSetSelectorCommandBitsForCategory(hashtable *commands, aclSelector *selector, uint64_t cflag, int value) { + hashtableIterator iter; + hashtableInitIterator(&iter, commands); + struct serverCommand *cmd; + while (hashtableNext(&iter, (void **)&cmd)) { if (cmd->acl_categories & cflag) { ACLChangeSelectorPerm(selector, cmd, value); } - if (cmd->subcommands_dict) { - ACLSetSelectorCommandBitsForCategory(cmd->subcommands_dict, selector, cflag, value); + if (cmd->subcommands_ht) { + ACLSetSelectorCommandBitsForCategory(cmd->subcommands_ht, selector, cflag, value); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* This function is responsible for recomputing the command bits for all selectors of the existing users. @@ -732,26 +732,26 @@ int ACLSetSelectorCategory(aclSelector *selector, const char *category, int allo return C_OK; } -void ACLCountCategoryBitsForCommands(dict *commands, +void ACLCountCategoryBitsForCommands(hashtable *commands, aclSelector *selector, unsigned long *on, unsigned long *off, uint64_t cflag) { - dictIterator *di = dictGetIterator(commands); - dictEntry *de; - while ((de = dictNext(di)) != NULL) { - struct serverCommand *cmd = dictGetVal(de); + hashtableIterator iter; + hashtableInitIterator(&iter, commands); + struct serverCommand *cmd; + while (hashtableNext(&iter, (void **)&cmd)) { if (cmd->acl_categories & cflag) { if (ACLGetSelectorCommandBit(selector, cmd->id)) (*on)++; else (*off)++; } - if (cmd->subcommands_dict) { - ACLCountCategoryBitsForCommands(cmd->subcommands_dict, selector, on, off, cflag); + if (cmd->subcommands_ht) { + ACLCountCategoryBitsForCommands(cmd->subcommands_ht, selector, on, off, cflag); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* Return the number of commands allowed (on) and denied (off) for the user 'u' @@ -1163,7 +1163,7 @@ int ACLSetSelector(aclSelector *selector, const char *op, size_t oplen) { return C_ERR; } - if (cmd->subcommands_dict) { + if (cmd->subcommands_ht) { /* If user is trying to allow a valid subcommand we can just add its unique ID */ cmd = ACLLookupCommand(op + 1); if (cmd == NULL) { @@ -2754,22 +2754,22 @@ sds getAclErrorMessage(int acl_res, user *user, struct serverCommand *cmd, sds e * ==========================================================================*/ /* ACL CAT category */ -void aclCatWithFlags(client *c, dict *commands, uint64_t cflag, int *arraylen) { - dictEntry *de; - dictIterator *di = dictGetIterator(commands); +void aclCatWithFlags(client *c, hashtable *commands, uint64_t cflag, int *arraylen) { + hashtableIterator iter; + hashtableInitIterator(&iter, commands); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *cmd = dictGetVal(de); + struct serverCommand *cmd; + while (hashtableNext(&iter, (void **)&cmd)) { if (cmd->acl_categories & cflag) { addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname)); (*arraylen)++; } - if (cmd->subcommands_dict) { - aclCatWithFlags(c, cmd->subcommands_dict, cflag, arraylen); + if (cmd->subcommands_ht) { + aclCatWithFlags(c, cmd->subcommands_ht, cflag, arraylen); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* Add the formatted response from a single selector to the ACL GETUSER diff --git a/src/config.c b/src/config.c index c4009adefa..670168d7f3 100644 --- a/src/config.c +++ b/src/config.c @@ -539,7 +539,6 @@ void loadServerConfigFromString(char *config) { loadServerConfig(argv[1], 0, NULL); } else if (!strcasecmp(argv[0], "rename-command") && argc == 3) { struct serverCommand *cmd = lookupCommandBySds(argv[1]); - int retval; if (!cmd) { err = "No such command in rename-command"; @@ -548,16 +547,13 @@ void loadServerConfigFromString(char *config) { /* If the target command name is the empty string we just * remove it from the command table. */ - retval = dictDelete(server.commands, argv[1]); - serverAssert(retval == DICT_OK); + serverAssert(hashtableDelete(server.commands, argv[1])); /* Otherwise we re-add the command under a different name. */ if (sdslen(argv[2]) != 0) { - sds copy = sdsdup(argv[2]); - - retval = dictAdd(server.commands, copy, cmd); - if (retval != DICT_OK) { - sdsfree(copy); + sdsfree(cmd->fullname); + cmd->fullname = sdsdup(argv[2]); + if (!hashtableAdd(server.commands, cmd)) { err = "Target command name already exists"; goto loaderr; } diff --git a/src/latency.c b/src/latency.c index 783f04b197..249d7c65e4 100644 --- a/src/latency.c +++ b/src/latency.c @@ -526,13 +526,12 @@ void fillCommandCDF(client *c, struct hdr_histogram *histogram) { /* latencyCommand() helper to produce for all commands, * a per command cumulative distribution of latencies. */ -void latencyAllCommandsFillCDF(client *c, dict *commands, int *command_with_data) { - dictIterator *di = dictGetSafeIterator(commands); - dictEntry *de; +void latencyAllCommandsFillCDF(client *c, hashtable *commands, int *command_with_data) { + hashtableIterator iter; + hashtableInitSafeIterator(&iter, commands); struct serverCommand *cmd; - while ((de = dictNext(di)) != NULL) { - cmd = (struct serverCommand *)dictGetVal(de); + while (hashtableNext(&iter, (void **)&cmd)) { if (cmd->latency_histogram) { addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname)); fillCommandCDF(c, cmd->latency_histogram); @@ -540,10 +539,10 @@ void latencyAllCommandsFillCDF(client *c, dict *commands, int *command_with_data } if (cmd->subcommands) { - latencyAllCommandsFillCDF(c, cmd->subcommands_dict, command_with_data); + latencyAllCommandsFillCDF(c, cmd->subcommands_ht, command_with_data); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* latencyCommand() helper to produce for a specific command set, @@ -564,19 +563,19 @@ void latencySpecificCommandsFillCDF(client *c) { command_with_data++; } - if (cmd->subcommands_dict) { - dictEntry *de; - dictIterator *di = dictGetSafeIterator(cmd->subcommands_dict); + if (cmd->subcommands_ht) { + hashtableIterator iter; + hashtableInitSafeIterator(&iter, cmd->subcommands_ht); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *sub = dictGetVal(de); + struct serverCommand *sub; + while (hashtableNext(&iter, (void **)&sub)) { if (sub->latency_histogram) { addReplyBulkCBuffer(c, sub->fullname, sdslen(sub->fullname)); fillCommandCDF(c, sub->latency_histogram); command_with_data++; } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } } setDeferredMapLen(c, replylen, command_with_data); diff --git a/src/module.c b/src/module.c index 1e98b36f30..d1cd55c501 100644 --- a/src/module.c +++ b/src/module.c @@ -1297,8 +1297,8 @@ int VM_CreateCommand(ValkeyModuleCtx *ctx, cp->serverCmd->arity = cmdfunc ? -1 : -2; /* Default value, can be changed later via dedicated API */ /* Drain IO queue before modifying commands dictionary to prevent concurrent access while modifying it. */ drainIOThreadsQueue(); - serverAssert(dictAdd(server.commands, sdsdup(declared_name), cp->serverCmd) == DICT_OK); - serverAssert(dictAdd(server.orig_commands, sdsdup(declared_name), cp->serverCmd) == DICT_OK); + serverAssert(hashtableAdd(server.commands, cp->serverCmd)); + serverAssert(hashtableAdd(server.orig_commands, cp->serverCmd)); cp->serverCmd->id = ACLGetCommandID(declared_name); /* ID used for ACL. */ return VALKEYMODULE_OK; } @@ -1430,7 +1430,7 @@ int VM_CreateSubcommand(ValkeyModuleCommand *parent, /* Check if the command name is busy within the parent command. */ sds declared_name = sdsnew(name); - if (parent_cmd->subcommands_dict && lookupSubcommand(parent_cmd, declared_name) != NULL) { + if (parent_cmd->subcommands_ht && lookupSubcommand(parent_cmd, declared_name) != NULL) { sdsfree(declared_name); return VALKEYMODULE_ERR; } @@ -1440,7 +1440,7 @@ int VM_CreateSubcommand(ValkeyModuleCommand *parent, moduleCreateCommandProxy(parent->module, declared_name, fullname, cmdfunc, flags, firstkey, lastkey, keystep); cp->serverCmd->arity = -2; - commandAddSubcommand(parent_cmd, cp->serverCmd, name); + commandAddSubcommand(parent_cmd, cp->serverCmd); return VALKEYMODULE_OK; } @@ -12058,20 +12058,20 @@ int moduleFreeCommand(struct ValkeyModule *module, struct serverCommand *cmd) { moduleFreeArgs(cmd->args, cmd->num_args); zfree(cp); - if (cmd->subcommands_dict) { - dictEntry *de; - dictIterator *di = dictGetSafeIterator(cmd->subcommands_dict); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *sub = dictGetVal(de); + if (cmd->subcommands_ht) { + hashtableIterator iter; + hashtableInitSafeIterator(&iter, cmd->subcommands_ht); + struct serverCommand *sub; + while (hashtableNext(&iter, (void **)&sub)) { if (moduleFreeCommand(module, sub) != C_OK) continue; - serverAssert(dictDelete(cmd->subcommands_dict, sub->declared_name) == DICT_OK); + serverAssert(hashtableDelete(cmd->subcommands_ht, sub->declared_name)); sdsfree((sds)sub->declared_name); sdsfree(sub->fullname); zfree(sub); } - dictReleaseIterator(di); - dictRelease(cmd->subcommands_dict); + hashtableResetIterator(&iter); + hashtableRelease(cmd->subcommands_ht); } return C_OK; @@ -12081,19 +12081,19 @@ void moduleUnregisterCommands(struct ValkeyModule *module) { /* Drain IO queue before modifying commands dictionary to prevent concurrent access while modifying it. */ drainIOThreadsQueue(); /* Unregister all the commands registered by this module. */ - dictIterator *di = dictGetSafeIterator(server.commands); - dictEntry *de; - while ((de = dictNext(di)) != NULL) { - struct serverCommand *cmd = dictGetVal(de); + hashtableIterator iter; + hashtableInitSafeIterator(&iter, server.commands); + struct serverCommand *cmd; + while (hashtableNext(&iter, (void **)&cmd)) { if (moduleFreeCommand(module, cmd) != C_OK) continue; - serverAssert(dictDelete(server.commands, cmd->fullname) == DICT_OK); - serverAssert(dictDelete(server.orig_commands, cmd->fullname) == DICT_OK); + serverAssert(hashtableDelete(server.commands, cmd->fullname)); + serverAssert(hashtableDelete(server.orig_commands, cmd->fullname)); sdsfree((sds)cmd->declared_name); sdsfree(cmd->fullname); zfree(cmd); } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* We parse argv to add sds "NAME VALUE" pairs to the server.module_configs_queue list of configs. diff --git a/src/server.c b/src/server.c index aebbb57a93..3d670505e8 100644 --- a/src/server.c +++ b/src/server.c @@ -390,6 +390,11 @@ int dictSdsKeyCaseCompare(const void *key1, const void *key2) { return strcasecmp(key1, key2) == 0; } +/* Case insensitive key comparison */ +int hashtableStringKeyCaseCompare(const void *key1, const void *key2) { + return strcasecmp(key1, key2); +} + void dictObjectDestructor(void *val) { if (val == NULL) return; /* Lazy freeing will set value to NULL. */ decrRefCount(val); @@ -506,6 +511,16 @@ int dictResizeAllowed(size_t moreMem, double usedRatio) { } } +const void *hashtableCommandGetKey(const void *element) { + struct serverCommand *command = (struct serverCommand *)element; + return command->fullname; +} + +const void *hashtableSubcommandGetKey(const void *element) { + struct serverCommand *command = (struct serverCommand *)element; + return command->declared_name; +} + /* Generic hash table type where keys are Objects, Values * dummy pointers. */ dictType objectKeyPointerValueDictType = { @@ -578,16 +593,17 @@ dictType kvstoreExpiresDictType = { kvstoreDictMetadataSize, }; -/* Command table. sds string -> command struct pointer. */ -dictType commandTableDictType = { - dictSdsCaseHash, /* hash function */ - NULL, /* key dup */ - dictSdsKeyCaseCompare, /* key compare */ - dictSdsDestructor, /* key destructor */ - NULL, /* val destructor */ - NULL, /* allow to expand */ - .no_incremental_rehash = 1, /* no incremental rehash as the command table may be accessed from IO threads. */ -}; +/* Command set, hashed by sds string, stores serverCommand structs. */ +hashtableType commandSetType = {.entryGetKey = hashtableCommandGetKey, + .hashFunction = dictSdsCaseHash, + .keyCompare = hashtableStringKeyCaseCompare, + .instant_rehashing = 1}; + +/* Sub-command set, hashed by char* string, stores serverCommand structs. */ +hashtableType subcommandSetType = {.entryGetKey = hashtableSubcommandGetKey, + .hashFunction = dictCStrCaseHash, + .keyCompare = hashtableStringKeyCaseCompare, + .instant_rehashing = 1}; /* Hash type hash table (note that small hashes are represented with listpacks) */ dictType hashDictType = { @@ -2192,8 +2208,8 @@ void initServerConfig(void) { /* Command table -- we initialize it here as it is part of the * initial configuration, since command names may be changed via * valkey.conf using the rename-command directive. */ - server.commands = dictCreate(&commandTableDictType); - server.orig_commands = dictCreate(&commandTableDictType); + server.commands = hashtableCreate(&commandSetType); + server.orig_commands = hashtableCreate(&commandSetType); populateCommandTable(); /* Debugging */ @@ -3035,13 +3051,13 @@ sds catSubCommandFullname(const char *parent_name, const char *sub_name) { return sdscatfmt(sdsempty(), "%s|%s", parent_name, sub_name); } -void commandAddSubcommand(struct serverCommand *parent, struct serverCommand *subcommand, const char *declared_name) { - if (!parent->subcommands_dict) parent->subcommands_dict = dictCreate(&commandTableDictType); +void commandAddSubcommand(struct serverCommand *parent, struct serverCommand *subcommand) { + if (!parent->subcommands_ht) parent->subcommands_ht = hashtableCreate(&subcommandSetType); subcommand->parent = parent; /* Assign the parent command */ subcommand->id = ACLGetCommandID(subcommand->fullname); /* Assign the ID used for ACL. */ - serverAssert(dictAdd(parent->subcommands_dict, sdsnew(declared_name), subcommand) == DICT_OK); + serverAssert(hashtableAdd(parent->subcommands_ht, subcommand)); } /* Set implicit ACl categories (see comment above the definition of @@ -3093,7 +3109,7 @@ int populateCommandStructure(struct serverCommand *c) { sub->fullname = catSubCommandFullname(c->declared_name, sub->declared_name); if (populateCommandStructure(sub) == C_ERR) continue; - commandAddSubcommand(c, sub, sub->declared_name); + commandAddSubcommand(c, sub); } } @@ -3117,22 +3133,20 @@ void populateCommandTable(void) { c->fullname = sdsnew(c->declared_name); if (populateCommandStructure(c) == C_ERR) continue; - retval1 = dictAdd(server.commands, sdsdup(c->fullname), c); + retval1 = hashtableAdd(server.commands, c); /* Populate an additional dictionary that will be unaffected * by rename-command statements in valkey.conf. */ - retval2 = dictAdd(server.orig_commands, sdsdup(c->fullname), c); - serverAssert(retval1 == DICT_OK && retval2 == DICT_OK); + retval2 = hashtableAdd(server.orig_commands, c); + serverAssert(retval1 && retval2); } } -void resetCommandTableStats(dict *commands) { +void resetCommandTableStats(hashtable *commands) { struct serverCommand *c; - dictEntry *de; - dictIterator *di; + hashtableIterator iter; - di = dictGetSafeIterator(commands); - while ((de = dictNext(di)) != NULL) { - c = (struct serverCommand *)dictGetVal(de); + hashtableInitSafeIterator(&iter, commands); + while (hashtableNext(&iter, (void **)&c)) { c->microseconds = 0; c->calls = 0; c->rejected_calls = 0; @@ -3141,9 +3155,9 @@ void resetCommandTableStats(dict *commands) { hdr_close(c->latency_histogram); c->latency_histogram = NULL; } - if (c->subcommands_dict) resetCommandTableStats(c->subcommands_dict); + if (c->subcommands_ht) resetCommandTableStats(c->subcommands_ht); } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } void resetErrorTableStats(void) { @@ -3190,13 +3204,16 @@ void serverOpArrayFree(serverOpArray *oa) { /* ====================== Commands lookup and execution ===================== */ int isContainerCommandBySds(sds s) { - struct serverCommand *base_cmd = dictFetchValue(server.commands, s); - int has_subcommands = base_cmd && base_cmd->subcommands_dict; + struct serverCommand *base_cmd; + int found_command = hashtableFind(server.commands, s, (void **)&base_cmd); + int has_subcommands = found_command && base_cmd->subcommands_ht; return has_subcommands; } struct serverCommand *lookupSubcommand(struct serverCommand *container, sds sub_name) { - return dictFetchValue(container->subcommands_dict, sub_name); + struct serverCommand *subcommand = NULL; + hashtableFind(container->subcommands_ht, sub_name, (void **)&subcommand); + return subcommand; } /* Look up a command by argv and argc @@ -3207,9 +3224,10 @@ struct serverCommand *lookupSubcommand(struct serverCommand *container, sds sub_ * name (e.g. in COMMAND INFO) rather than to find the command * a user requested to execute (in processCommand). */ -struct serverCommand *lookupCommandLogic(dict *commands, robj **argv, int argc, int strict) { - struct serverCommand *base_cmd = dictFetchValue(commands, argv[0]->ptr); - int has_subcommands = base_cmd && base_cmd->subcommands_dict; +struct serverCommand *lookupCommandLogic(hashtable *commands, robj **argv, int argc, int strict) { + struct serverCommand *base_cmd = NULL; + int found_command = hashtableFind(commands, argv[0]->ptr, (void **)&base_cmd); + int has_subcommands = found_command && base_cmd->subcommands_ht; if (argc == 1 || !has_subcommands) { if (strict && argc != 1) return NULL; /* Note: It is possible that base_cmd->proc==NULL (e.g. CONFIG) */ @@ -3225,7 +3243,7 @@ struct serverCommand *lookupCommand(robj **argv, int argc) { return lookupCommandLogic(server.commands, argv, argc, 0); } -struct serverCommand *lookupCommandBySdsLogic(dict *commands, sds s) { +struct serverCommand *lookupCommandBySdsLogic(hashtable *commands, sds s) { int argc, j; sds *strings = sdssplitlen(s, sdslen(s), "|", 1, &argc); if (strings == NULL) return NULL; @@ -3252,7 +3270,7 @@ struct serverCommand *lookupCommandBySds(sds s) { return lookupCommandBySdsLogic(server.commands, s); } -struct serverCommand *lookupCommandByCStringLogic(dict *commands, const char *s) { +struct serverCommand *lookupCommandByCStringLogic(hashtable *commands, const char *s) { struct serverCommand *cmd; sds name = sdsnew(s); @@ -4899,23 +4917,24 @@ void addReplyCommandSubCommands(client *c, struct serverCommand *cmd, void (*reply_function)(client *, struct serverCommand *), int use_map) { - if (!cmd->subcommands_dict) { + if (!cmd->subcommands_ht) { addReplySetLen(c, 0); return; } if (use_map) - addReplyMapLen(c, dictSize(cmd->subcommands_dict)); + addReplyMapLen(c, hashtableSize(cmd->subcommands_ht)); else - addReplyArrayLen(c, dictSize(cmd->subcommands_dict)); - dictEntry *de; - dictIterator *di = dictGetSafeIterator(cmd->subcommands_dict); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *sub = (struct serverCommand *)dictGetVal(de); + addReplyArrayLen(c, hashtableSize(cmd->subcommands_ht)); + + hashtableIterator iter; + struct serverCommand *sub; + hashtableInitSafeIterator(&iter, cmd->subcommands_ht); + while (hashtableNext(&iter, (void **)&sub)) { if (use_map) addReplyBulkCBuffer(c, sub->fullname, sdslen(sub->fullname)); reply_function(c, sub); } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* Output the representation of a server command. Used by the COMMAND command and COMMAND INFO. */ @@ -4961,7 +4980,7 @@ void addReplyCommandDocs(client *c, struct serverCommand *cmd) { if (cmd->reply_schema) maplen++; #endif if (cmd->args) maplen++; - if (cmd->subcommands_dict) maplen++; + if (cmd->subcommands_ht) maplen++; addReplyMapLen(c, maplen); if (cmd->summary) { @@ -5011,7 +5030,7 @@ void addReplyCommandDocs(client *c, struct serverCommand *cmd) { addReplyBulkCString(c, "arguments"); addReplyCommandArgList(c, cmd->args, cmd->num_args); } - if (cmd->subcommands_dict) { + if (cmd->subcommands_ht) { addReplyBulkCString(c, "subcommands"); addReplyCommandSubCommands(c, cmd, addReplyCommandDocs, 1); } @@ -5068,20 +5087,20 @@ void getKeysSubcommand(client *c) { /* COMMAND (no args) */ void commandCommand(client *c) { - dictIterator *di; - dictEntry *de; + hashtableIterator iter; + struct serverCommand *cmd; - addReplyArrayLen(c, dictSize(server.commands)); - di = dictGetIterator(server.commands); - while ((de = dictNext(di)) != NULL) { - addReplyCommandInfo(c, dictGetVal(de)); + addReplyArrayLen(c, hashtableSize(server.commands)); + hashtableInitIterator(&iter, server.commands); + while (hashtableNext(&iter, (void **)&cmd)) { + addReplyCommandInfo(c, cmd); } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* COMMAND COUNT */ void commandCountCommand(client *c) { - addReplyLongLong(c, dictSize(server.commands)); + addReplyLongLong(c, hashtableSize(server.commands)); } typedef enum { @@ -5127,39 +5146,39 @@ int shouldFilterFromCommandList(struct serverCommand *cmd, commandListFilter *fi } /* COMMAND LIST FILTERBY (MODULE |ACLCAT |PATTERN ) */ -void commandListWithFilter(client *c, dict *commands, commandListFilter filter, int *numcmds) { - dictEntry *de; - dictIterator *di = dictGetIterator(commands); +void commandListWithFilter(client *c, hashtable *commands, commandListFilter filter, int *numcmds) { + hashtableIterator iter; + hashtableInitIterator(&iter, commands); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *cmd = dictGetVal(de); + struct serverCommand *cmd; + while (hashtableNext(&iter, (void **)&cmd)) { if (!shouldFilterFromCommandList(cmd, &filter)) { addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname)); (*numcmds)++; } - if (cmd->subcommands_dict) { - commandListWithFilter(c, cmd->subcommands_dict, filter, numcmds); + if (cmd->subcommands_ht) { + commandListWithFilter(c, cmd->subcommands_ht, filter, numcmds); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* COMMAND LIST */ -void commandListWithoutFilter(client *c, dict *commands, int *numcmds) { - dictEntry *de; - dictIterator *di = dictGetIterator(commands); +void commandListWithoutFilter(client *c, hashtable *commands, int *numcmds) { + hashtableIterator iter; + struct serverCommand *cmd; + hashtableInitIterator(&iter, commands); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *cmd = dictGetVal(de); + while (hashtableNext(&iter, (void **)&cmd)) { addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname)); (*numcmds)++; - if (cmd->subcommands_dict) { - commandListWithoutFilter(c, cmd->subcommands_dict, numcmds); + if (cmd->subcommands_ht) { + commandListWithoutFilter(c, cmd->subcommands_ht, numcmds); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } /* COMMAND LIST [FILTERBY (MODULE |ACLCAT |PATTERN )] */ @@ -5208,14 +5227,14 @@ void commandInfoCommand(client *c) { int i; if (c->argc == 2) { - dictIterator *di; - dictEntry *de; - addReplyArrayLen(c, dictSize(server.commands)); - di = dictGetIterator(server.commands); - while ((de = dictNext(di)) != NULL) { - addReplyCommandInfo(c, dictGetVal(de)); + hashtableIterator iter; + struct serverCommand *cmd; + addReplyArrayLen(c, hashtableSize(server.commands)); + hashtableInitIterator(&iter, server.commands); + while (hashtableNext(&iter, (void **)&cmd)) { + addReplyCommandInfo(c, cmd); } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } else { addReplyArrayLen(c, c->argc - 2); for (i = 2; i < c->argc; i++) { @@ -5229,16 +5248,15 @@ void commandDocsCommand(client *c) { int i; if (c->argc == 2) { /* Reply with an array of all commands */ - dictIterator *di; - dictEntry *de; - addReplyMapLen(c, dictSize(server.commands)); - di = dictGetIterator(server.commands); - while ((de = dictNext(di)) != NULL) { - struct serverCommand *cmd = dictGetVal(de); + hashtableIterator iter; + struct serverCommand *cmd; + addReplyMapLen(c, hashtableSize(server.commands)); + hashtableInitIterator(&iter, server.commands); + while (hashtableNext(&iter, (void **)&cmd)) { addReplyBulkCBuffer(c, cmd->fullname, sdslen(cmd->fullname)); addReplyCommandDocs(c, cmd); } - dictReleaseIterator(di); + hashtableResetIterator(&iter); } else { /* Reply with an array of the requested commands (if we find them) */ int numcmds = 0; @@ -5358,14 +5376,12 @@ const char *getSafeInfoString(const char *s, size_t len, char **tmp) { return memmapchars(new, len, unsafe_info_chars, unsafe_info_chars_substs, sizeof(unsafe_info_chars) - 1); } -sds genValkeyInfoStringCommandStats(sds info, dict *commands) { +sds genValkeyInfoStringCommandStats(sds info, hashtable *commands) { struct serverCommand *c; - dictEntry *de; - dictIterator *di; - di = dictGetSafeIterator(commands); - while ((de = dictNext(di)) != NULL) { + hashtableIterator iter; + hashtableInitSafeIterator(&iter, commands); + while (hashtableNext(&iter, (void **)&c)) { char *tmpsafe; - c = (struct serverCommand *)dictGetVal(de); if (c->calls || c->failed_calls || c->rejected_calls) { info = sdscatprintf(info, "cmdstat_%s:calls=%lld,usec=%lld,usec_per_call=%.2f" @@ -5375,11 +5391,11 @@ sds genValkeyInfoStringCommandStats(sds info, dict *commands) { c->rejected_calls, c->failed_calls); if (tmpsafe != NULL) zfree(tmpsafe); } - if (c->subcommands_dict) { - info = genValkeyInfoStringCommandStats(info, c->subcommands_dict); + if (c->subcommands_ht) { + info = genValkeyInfoStringCommandStats(info, c->subcommands_ht); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); return info; } @@ -5396,24 +5412,22 @@ sds genValkeyInfoStringACLStats(sds info) { return info; } -sds genValkeyInfoStringLatencyStats(sds info, dict *commands) { +sds genValkeyInfoStringLatencyStats(sds info, hashtable *commands) { struct serverCommand *c; - dictEntry *de; - dictIterator *di; - di = dictGetSafeIterator(commands); - while ((de = dictNext(di)) != NULL) { + hashtableIterator iter; + hashtableInitSafeIterator(&iter, commands); + while (hashtableNext(&iter, (void **)&c)) { char *tmpsafe; - c = (struct serverCommand *)dictGetVal(de); if (c->latency_histogram) { info = fillPercentileDistributionLatencies( info, getSafeInfoString(c->fullname, sdslen(c->fullname), &tmpsafe), c->latency_histogram); if (tmpsafe != NULL) zfree(tmpsafe); } - if (c->subcommands_dict) { - info = genValkeyInfoStringLatencyStats(info, c->subcommands_dict); + if (c->subcommands_ht) { + info = genValkeyInfoStringLatencyStats(info, c->subcommands_ht); } } - dictReleaseIterator(di); + hashtableResetIterator(&iter); return info; } diff --git a/src/server.h b/src/server.h index 531ca8e7c8..26ac7cc08a 100644 --- a/src/server.h +++ b/src/server.h @@ -66,7 +66,8 @@ typedef long long ustime_t; /* microsecond time type. */ #include "ae.h" /* Event driven programming library */ #include "sds.h" /* Dynamic safe strings */ -#include "dict.h" /* Hash tables */ +#include "dict.h" /* Hash tables (old implementation) */ +#include "hashtable.h" /* Hash tables (new implementation) */ #include "kvstore.h" /* Slot-based hash table */ #include "adlist.h" /* Linked lists */ #include "zmalloc.h" /* total memory usage aware version of malloc/free */ @@ -1677,8 +1678,8 @@ struct valkeyServer { int hz; /* serverCron() calls frequency in hertz */ int in_fork_child; /* indication that this is a fork child */ serverDb *db; - dict *commands; /* Command table */ - dict *orig_commands; /* Command table before command renaming. */ + hashtable *commands; /* Command table */ + hashtable *orig_commands; /* Command table before command renaming. */ aeEventLoop *el; _Atomic AeIoState io_poll_state; /* Indicates the state of the IO polling. */ int io_ae_fired_events; /* Number of poll events received by the IO thread. */ @@ -2557,12 +2558,12 @@ struct serverCommand { bit set in the bitmap of allowed commands. */ sds fullname; /* A SDS string representing the command fullname. */ struct hdr_histogram - *latency_histogram; /*points to the command latency command histogram (unit of time nanosecond) */ + *latency_histogram; /* Points to the command latency command histogram (unit of time nanosecond). */ keySpec legacy_range_key_spec; /* The legacy (first,last,step) key spec is * still maintained (if applicable) so that * we can still support the reply format of * COMMAND INFO and COMMAND GETKEYS */ - dict *subcommands_dict; /* A dictionary that holds the subcommands, the key is the subcommand sds name + hashtable *subcommands_ht; /* Subcommands hash table. The key is the subcommand sds name * (not the fullname), and the value is the serverCommand structure pointer. */ struct serverCommand *parent; struct ValkeyModuleCommand *module_cmd; /* A pointer to the module command data (NULL if native command) */ @@ -3291,9 +3292,9 @@ int changeListener(connListener *listener); void closeListener(connListener *listener); struct serverCommand *lookupSubcommand(struct serverCommand *container, sds sub_name); struct serverCommand *lookupCommand(robj **argv, int argc); -struct serverCommand *lookupCommandBySdsLogic(dict *commands, sds s); +struct serverCommand *lookupCommandBySdsLogic(hashtable *commands, sds s); struct serverCommand *lookupCommandBySds(sds s); -struct serverCommand *lookupCommandByCStringLogic(dict *commands, const char *s); +struct serverCommand *lookupCommandByCStringLogic(hashtable *commands, const char *s); struct serverCommand *lookupCommandByCString(const char *s); struct serverCommand *lookupCommandOrOriginal(robj **argv, int argc); int commandCheckExistence(client *c, sds *err); @@ -3327,7 +3328,7 @@ void serverLogRawFromHandler(int level, const char *msg); void usage(void); void updateDictResizePolicy(void); void populateCommandTable(void); -void resetCommandTableStats(dict *commands); +void resetCommandTableStats(hashtable *commands); void resetErrorTableStats(void); void adjustOpenFilesLimit(void); void incrementErrorCount(const char *fullerr, size_t namelen); @@ -4024,7 +4025,7 @@ int memtest_preserving_test(unsigned long *m, size_t bytes, int passes); void mixDigest(unsigned char *digest, const void *ptr, size_t len); void xorDigest(unsigned char *digest, const void *ptr, size_t len); sds catSubCommandFullname(const char *parent_name, const char *sub_name); -void commandAddSubcommand(struct serverCommand *parent, struct serverCommand *subcommand, const char *declared_name); +void commandAddSubcommand(struct serverCommand *parent, struct serverCommand *subcommand); void debugDelay(int usec); void killThreads(void); void makeThreadKillable(void);