Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce rewriteClientCommandVector usage on EXPIRE command #11602

Merged
merged 5 commits into from
Dec 9, 2022

Conversation

filipecosta90
Copy link
Contributor

@filipecosta90 filipecosta90 commented Dec 9, 2022

As being discussed in #10981 (comment) there is overhead on 7.0 EXPIRE command that is not present on 6.2.7.

We can see that on the unstable profile there are around 7% of CPU cycles spent on rewriteClientCommandVector that are not present on 6.2.7. This was introduced in #8474.
This PR reduces the overhead by using 2X rewriteClientCommandArgument instead of rewriteClientCommandVector. In this scenario rewriteClientCommandVector creates 4 arguments. the above usage of rewriteClientCommandArgument reduces the overhead in half.
This PR should also improve PEXPIREAT performance by avoiding at all rewriteClientCommandArgument usage.

6.2.7 profile:

image

unstable profile:
image

If we populate data as follows:

memtier_benchmark --port 6379 --server localhost   --key-minimum=1 --key-maximum 100000 "--data-size" "100" "--command" "HSET __key__ field __value__" "--command-key-pattern" "P" "-c" "50" "-t" "2" "--hide-histogram"

And then benchmark:

memtier_benchmark  --pipeline 10 --key-minimum=1 --key-maximum 100000 "--data-size" "100" --command "EXPIRE __key__ 3600" --command-key-pattern="R" -c 50 -t 2 --hide-histogram --test-time 180

we get the following results on the achievable ops/sec:

test Version 6.2.7 unstable branch 8th December 2022 ( 10e4f44 ) % change unstable vs 6.2.7 this PR % change this PR vs 6.2.7
EXPIRE ( pipeline 10 ) 876843 770614 -12.1% 821241 -6.3%

src/expire.c Outdated Show resolved Hide resolved
@oranagra oranagra merged commit c3fb48d into redis:unstable Dec 9, 2022
@filipecosta90
Copy link
Contributor Author

@oranagra after your change I did a new check.
We're with a 5% GAP vs 6.2.7 =)

test Version 6.2.7 unstable branch 9th December 2022 ( 528bb11 ) % change unstable vs 6.2.7
EXPIRE ( pipeline 10 ) 876843 830212 -5.3%

@filipecosta90 filipecosta90 added the action:run-benchmark Triggers the benchmark suite for this Pull Request label Dec 9, 2022
@oranagra oranagra mentioned this pull request Dec 11, 2022
oranagra pushed a commit that referenced this pull request Dec 12, 2022
There is overhead on Redis 7.0 EXPIRE command that is not present on 6.2.7.

We could see that on the unstable profile there are around 7% of CPU cycles
spent on rewriteClientCommandVector that are not present on 6.2.7.
This was introduced in #8474.
This PR reduces the overhead by using 2X rewriteClientCommandArgument instead of
rewriteClientCommandVector. In this scenario rewriteClientCommandVector creates 4 arguments.
the above usage of rewriteClientCommandArgument reduces the overhead in half.

This PR should also improve PEXPIREAT performance by avoiding at all
rewriteClientCommandArgument usage.

Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit c3fb48d)
madolson pushed a commit to madolson/redis that referenced this pull request Apr 19, 2023
There is overhead on Redis 7.0 EXPIRE command that is not present on 6.2.7. 

We could see that on the unstable profile there are around 7% of CPU cycles
spent on rewriteClientCommandVector that are not present on 6.2.7.
This was introduced in redis#8474.
This PR reduces the overhead by using 2X rewriteClientCommandArgument instead of
rewriteClientCommandVector. In this scenario rewriteClientCommandVector creates 4 arguments.
the above usage of rewriteClientCommandArgument reduces the overhead in half.

This PR should also improve PEXPIREAT performance by avoiding at all
rewriteClientCommandArgument usage. 

Co-authored-by: Oran Agra <oran@redislabs.com>
enjoy-binbin pushed a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
There is overhead on Redis 7.0 EXPIRE command that is not present on 6.2.7. 

We could see that on the unstable profile there are around 7% of CPU cycles
spent on rewriteClientCommandVector that are not present on 6.2.7.
This was introduced in redis#8474.
This PR reduces the overhead by using 2X rewriteClientCommandArgument instead of
rewriteClientCommandVector. In this scenario rewriteClientCommandVector creates 4 arguments.
the above usage of rewriteClientCommandArgument reduces the overhead in half.

This PR should also improve PEXPIREAT performance by avoiding at all
rewriteClientCommandArgument usage. 

Co-authored-by: Oran Agra <oran@redislabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:run-benchmark Triggers the benchmark suite for this Pull Request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants