Commit 7c4f6e17 authored by filipe oliveira's avatar filipe oliveira Committed by Oran Agra
Browse files

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: default avatarOran Agra <oran@redislabs.com>
(cherry picked from commit 7dfd7b91)
parent fba49b10
...@@ -385,17 +385,17 @@ uint64_t evalGetCommandFlags(client *c, uint64_t cmd_flags) { ...@@ -385,17 +385,17 @@ uint64_t evalGetCommandFlags(client *c, uint64_t cmd_flags) {
int evalsha = c->cmd->proc == evalShaCommand || c->cmd->proc == evalShaRoCommand; int evalsha = c->cmd->proc == evalShaCommand || c->cmd->proc == evalShaRoCommand;
if (evalsha && sdslen(c->argv[1]->ptr) != 40) if (evalsha && sdslen(c->argv[1]->ptr) != 40)
return cmd_flags; return cmd_flags;
uint64_t script_flags;
evalCalcFunctionName(evalsha, c->argv[1]->ptr, funcname); evalCalcFunctionName(evalsha, c->argv[1]->ptr, funcname);
char *lua_cur_script = funcname + 2; char *lua_cur_script = funcname + 2;
dictEntry *de = dictFind(lctx.lua_scripts, lua_cur_script); c->cur_script = dictFind(lctx.lua_scripts, lua_cur_script);
uint64_t script_flags; if (!c->cur_script) {
if (!de) {
if (evalsha) if (evalsha)
return cmd_flags; return cmd_flags;
if (evalExtractShebangFlags(c->argv[1]->ptr, &script_flags, NULL, NULL) == C_ERR) if (evalExtractShebangFlags(c->argv[1]->ptr, &script_flags, NULL, NULL) == C_ERR)
return cmd_flags; return cmd_flags;
} else { } else {
luaScript *l = dictGetVal(de); luaScript *l = dictGetVal(c->cur_script);
script_flags = l->flags; script_flags = l->flags;
} }
if (script_flags & SCRIPT_FLAG_EVAL_COMPAT_MODE) if (script_flags & SCRIPT_FLAG_EVAL_COMPAT_MODE)
...@@ -502,7 +502,12 @@ void evalGenericCommand(client *c, int evalsha) { ...@@ -502,7 +502,12 @@ void evalGenericCommand(client *c, int evalsha) {
return; 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. */ /* Push the pcall error handler function on the stack. */
lua_getglobal(lua, "__redis__err__handler"); lua_getglobal(lua, "__redis__err__handler");
...@@ -531,7 +536,9 @@ void evalGenericCommand(client *c, int evalsha) { ...@@ -531,7 +536,9 @@ void evalGenericCommand(client *c, int evalsha) {
} }
char *lua_cur_script = funcname + 2; 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); luaScript *l = dictGetVal(de);
int ro = c->cmd->proc == evalRoCommand || c->cmd->proc == evalShaRoCommand; int ro = c->cmd->proc == evalRoCommand || c->cmd->proc == evalShaRoCommand;
......
...@@ -608,9 +608,10 @@ void functionKillCommand(client *c) { ...@@ -608,9 +608,10 @@ void functionKillCommand(client *c) {
* Note that it does not guarantee the command arguments are right. */ * Note that it does not guarantee the command arguments are right. */
uint64_t fcallGetCommandFlags(client *c, uint64_t cmd_flags) { uint64_t fcallGetCommandFlags(client *c, uint64_t cmd_flags) {
robj *function_name = c->argv[1]; robj *function_name = c->argv[1];
functionInfo *fi = dictFetchValue(curr_functions_lib_ctx->functions, function_name->ptr); c->cur_script = dictFind(curr_functions_lib_ctx->functions, function_name->ptr);
if (!fi) if (!c->cur_script)
return cmd_flags; return cmd_flags;
functionInfo *fi = dictGetVal(c->cur_script);
uint64_t script_flags = fi->f_flags; uint64_t script_flags = fi->f_flags;
return scriptFlagsToCmdFlags(cmd_flags, script_flags); return scriptFlagsToCmdFlags(cmd_flags, script_flags);
} }
...@@ -620,11 +621,14 @@ static void fcallCommandGeneric(client *c, int ro) { ...@@ -620,11 +621,14 @@ static void fcallCommandGeneric(client *c, int ro) {
replicationFeedMonitors(c,server.monitors,c->db->id,c->argv,c->argc); replicationFeedMonitors(c,server.monitors,c->db->id,c->argv,c->argc);
robj *function_name = c->argv[1]; robj *function_name = c->argv[1];
functionInfo *fi = dictFetchValue(curr_functions_lib_ctx->functions, function_name->ptr); dictEntry *de = c->cur_script;
if (!fi) { if (!de)
de = dictFind(curr_functions_lib_ctx->functions, function_name->ptr);
if (!de) {
addReplyError(c, "Function not found"); addReplyError(c, "Function not found");
return; return;
} }
functionInfo *fi = dictGetVal(de);
engine *engine = fi->li->ei->engine; engine *engine = fi->li->ei->engine;
long long numkeys; long long numkeys;
......
...@@ -156,6 +156,7 @@ client *createClient(connection *conn) { ...@@ -156,6 +156,7 @@ client *createClient(connection *conn) {
c->original_argc = 0; c->original_argc = 0;
c->original_argv = NULL; c->original_argv = NULL;
c->cmd = c->lastcmd = c->realcmd = NULL; c->cmd = c->lastcmd = c->realcmd = NULL;
c->cur_script = NULL;
c->multibulklen = 0; c->multibulklen = 0;
c->bulklen = -1; c->bulklen = -1;
c->sentlen = 0; c->sentlen = 0;
...@@ -2068,6 +2069,7 @@ void resetClient(client *c) { ...@@ -2068,6 +2069,7 @@ void resetClient(client *c) {
redisCommandProc *prevcmd = c->cmd ? c->cmd->proc : NULL; redisCommandProc *prevcmd = c->cmd ? c->cmd->proc : NULL;
freeClientArgv(c); freeClientArgv(c);
c->cur_script = NULL;
c->reqtype = 0; c->reqtype = 0;
c->multibulklen = 0; c->multibulklen = 0;
c->bulklen = -1; c->bulklen = -1;
......
...@@ -1118,6 +1118,7 @@ typedef struct client { ...@@ -1118,6 +1118,7 @@ typedef struct client {
time_t ctime; /* Client creation time. */ time_t ctime; /* Client creation time. */
long duration; /* Current command duration. Used for measuring latency of blocking/non-blocking cmds */ 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 */ 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 lastinteraction; /* Time of the last interaction, used for timeout */
time_t obuf_soft_limit_reached_time; time_t obuf_soft_limit_reached_time;
int authenticated; /* Needed when the default user requires auth. */ int authenticated; /* Needed when the default user requires auth. */
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment