Reduce eval related overhead introduced in v7.0 by evalCalcFunctionName#11521
Merged
oranagra merged 6 commits intoredis:unstablefrom Nov 29, 2022
Merged
Conversation
…n handling EVALSHA related commands
filipecosta90
added
the
action:run-benchmark
oranagra
reviewed
Nov 20, 2022
filipecosta90
changed the title
zeekling
approved these changes
Nov 22, 2022
sundb
reviewed
Nov 28, 2022
oranagra
approved these changes
Nov 29, 2022
oranagra
added
the
release-notes
Merged
oranagra
added a commit
that referenced
this pull request
Dec 12, 2022
…me (#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 <[email protected]> (cherry picked from commit 7dfd7b9)
madolson
pushed a commit
to madolson/redis
that referenced
this pull request
Apr 19, 2023
…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 <[email protected]>
enjoy-binbin
pushed a commit
to enjoy-binbin/redis
that referenced
this pull request
Jul 31, 2023
…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 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 ( 203b12e ) 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.
Notice that the current "fix" is still not enough to reach v6.2 numbers.
We need to improve 10% further to reach the v6.2 results ( even though we have new logic on 7.0 )
To benchmark the improvement we can do as follows with memtier:
EVALSHA
EVAL
Results based on OSS standalone achievable ops/sec