From 41d23efe42447dde9a77898a1bf26539f669d06f Mon Sep 17 00:00:00 2001 From: filipe oliveira Date: Tue, 29 Nov 2022 12:20:22 +0000 Subject: [PATCH] Reduce eval related overhead introduced in v7.0 by evalCalcFunctionName (#11521) As being discussed in #10981 we see a degradation in performance between v6.2 and v7.0 of Redis on the EVAL command. After profiling the current unstable branch we can see that we call the expensive function evalCalcFunctionName twice. The current "fix" is to basically avoid calling evalCalcFunctionName and even dictFind(lua_scripts) twice for the same command. Instead we cache the current script's dictEntry (for both Eval and Functions) in the current client so we don't have to repeat these calls. The exception would be when doing an EVAL on a new script that's not yet in the script cache. in that case we will call evalCalcFunctionName (and even evalExtractShebangFlags) twice. Co-authored-by: Oran Agra (cherry picked from commit 7dfd7b9197bbe216912049eebecbda3f1684925e) --- src/eval.c | 19 +++++++++++++------ src/functions.c | 12 ++++++++---- src/networking.c | 2 ++ src/server.h | 1 + 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/eval.c b/src/eval.c index e2ee41cb5bf3..81908567ddd8 100644 --- a/src/eval.c +++ b/src/eval.c @@ -385,17 +385,17 @@ uint64_t evalGetCommandFlags(client *c, uint64_t cmd_flags) { int evalsha = c->cmd->proc == evalShaCommand || c->cmd->proc == evalShaRoCommand; if (evalsha && sdslen(c->argv[1]->ptr) != 40) return cmd_flags; + uint64_t script_flags; evalCalcFunctionName(evalsha, c->argv[1]->ptr, funcname); char *lua_cur_script = funcname + 2; - dictEntry *de = dictFind(lctx.lua_scripts, lua_cur_script); - uint64_t script_flags; - if (!de) { + c->cur_script = dictFind(lctx.lua_scripts, lua_cur_script); + if (!c->cur_script) { if (evalsha) return cmd_flags; if (evalExtractShebangFlags(c->argv[1]->ptr, &script_flags, NULL, NULL) == C_ERR) return cmd_flags; } else { - luaScript *l = dictGetVal(de); + luaScript *l = dictGetVal(c->cur_script); script_flags = l->flags; } if (script_flags & SCRIPT_FLAG_EVAL_COMPAT_MODE) @@ -502,7 +502,12 @@ void evalGenericCommand(client *c, int evalsha) { return; } - evalCalcFunctionName(evalsha, c->argv[1]->ptr, funcname); + if (c->cur_script) { + funcname[0] = 'f', funcname[1] = '_'; + memcpy(funcname+2, dictGetKey(c->cur_script), 40); + funcname[42] = '\0'; + } else + evalCalcFunctionName(evalsha, c->argv[1]->ptr, funcname); /* Push the pcall error handler function on the stack. */ lua_getglobal(lua, "__redis__err__handler"); @@ -531,7 +536,9 @@ void evalGenericCommand(client *c, int evalsha) { } char *lua_cur_script = funcname + 2; - dictEntry *de = dictFind(lctx.lua_scripts, lua_cur_script); + dictEntry *de = c->cur_script; + if (!de) + de = dictFind(lctx.lua_scripts, lua_cur_script); luaScript *l = dictGetVal(de); int ro = c->cmd->proc == evalRoCommand || c->cmd->proc == evalShaRoCommand; diff --git a/src/functions.c b/src/functions.c index 9a2a5fc2e71d..4c42c51131da 100644 --- a/src/functions.c +++ b/src/functions.c @@ -608,9 +608,10 @@ void functionKillCommand(client *c) { * Note that it does not guarantee the command arguments are right. */ uint64_t fcallGetCommandFlags(client *c, uint64_t cmd_flags) { robj *function_name = c->argv[1]; - functionInfo *fi = dictFetchValue(curr_functions_lib_ctx->functions, function_name->ptr); - if (!fi) + c->cur_script = dictFind(curr_functions_lib_ctx->functions, function_name->ptr); + if (!c->cur_script) return cmd_flags; + functionInfo *fi = dictGetVal(c->cur_script); uint64_t script_flags = fi->f_flags; return scriptFlagsToCmdFlags(cmd_flags, script_flags); } @@ -620,11 +621,14 @@ static void fcallCommandGeneric(client *c, int ro) { replicationFeedMonitors(c,server.monitors,c->db->id,c->argv,c->argc); robj *function_name = c->argv[1]; - functionInfo *fi = dictFetchValue(curr_functions_lib_ctx->functions, function_name->ptr); - if (!fi) { + dictEntry *de = c->cur_script; + if (!de) + de = dictFind(curr_functions_lib_ctx->functions, function_name->ptr); + if (!de) { addReplyError(c, "Function not found"); return; } + functionInfo *fi = dictGetVal(de); engine *engine = fi->li->ei->engine; long long numkeys; diff --git a/src/networking.c b/src/networking.c index 7f9996c19754..421885ecd781 100644 --- a/src/networking.c +++ b/src/networking.c @@ -156,6 +156,7 @@ client *createClient(connection *conn) { c->original_argc = 0; c->original_argv = NULL; c->cmd = c->lastcmd = c->realcmd = NULL; + c->cur_script = NULL; c->multibulklen = 0; c->bulklen = -1; c->sentlen = 0; @@ -2068,6 +2069,7 @@ void resetClient(client *c) { redisCommandProc *prevcmd = c->cmd ? c->cmd->proc : NULL; freeClientArgv(c); + c->cur_script = NULL; c->reqtype = 0; c->multibulklen = 0; c->bulklen = -1; diff --git a/src/server.h b/src/server.h index 68757cc3d504..f3bcf5ab7a49 100644 --- a/src/server.h +++ b/src/server.h @@ -1118,6 +1118,7 @@ typedef struct client { time_t ctime; /* Client creation time. */ long duration; /* Current command duration. Used for measuring latency of blocking/non-blocking cmds */ int slot; /* The slot the client is executing against. Set to -1 if no slot is being used */ + dictEntry *cur_script; /* Cached pointer to the dictEntry of the script being executed. */ time_t lastinteraction; /* Time of the last interaction, used for timeout */ time_t obuf_soft_limit_reached_time; int authenticated; /* Needed when the default user requires auth. */