Skip to content

Commit

Permalink
Reduce eval related overhead introduced in v7.0 by evalCalcFunctionNa…
Browse files Browse the repository at this point in the history
…me (redis#11521)

As being discussed in redis#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 <oran@redislabs.com>
(cherry picked from commit 7dfd7b9)
  • Loading branch information
filipecosta90 authored and oranagra committed Dec 6, 2022
1 parent 4272efe commit 41d23ef
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 10 deletions.
19 changes: 13 additions & 6 deletions src/eval.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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;

Expand Down
12 changes: 8 additions & 4 deletions src/functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/networking.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down

0 comments on commit 41d23ef

Please sign in to comment.