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

Improve async cluster client performance. #877

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

nihohit
Copy link
Contributor

@nihohit nihohit commented Jun 27, 2023

This change reverts most of the performance degradation introduced recently. Using this benchmark I measured ~320K TPS on version 0.23, ~160K TPS on main, and ~295K TPS after this change.

The cause of the degradation seems to be that existing connections, which were received on the happy path, were wrapped 4 times - 2 boxes, 2 shares. Maybe more, if the compiler had to move the connection to heap for the boxed/shared generator. This change removes all of these unnecessary allocations.

Existing connections, which were received on the happy path,
were wrapped 4 times - 2 boxes, 2 shares. Maybe more, if the compiler
had to move the connection to heap for the boxed/shared generator. This
change removes all of these unnecessary allocations.
Now all the calls pass `true`, so the argument is no longer needed.
@nihohit
Copy link
Contributor Author

nihohit commented Jun 27, 2023

I also checked, and ConnectionMap can be changed to hold connections instead of connection futures as values. I don't see a meaningful perf change there, so didn't add it to this PR.

@jaymell
Copy link
Contributor

jaymell commented Jun 27, 2023

This looks good -- thanks for cleaning this up!

@jaymell jaymell merged commit f0ec381 into redis-rs:main Jun 28, 2023
9 checks passed
@nihohit nihohit deleted the fix-perf branch June 28, 2023 07:11
altanozlu pushed a commit to altanozlu/redis-rs that referenced this pull request Aug 16, 2023
* Remove unnecessary .boxed().shared() calls.

Existing connections, which were received on the happy path,
were wrapped 4 times - 2 boxes, 2 shares. Maybe more, if the compiler
had to move the connection to heap for the boxed/shared generator. This
change removes all of these unnecessary allocations.

* Remove argument from get_or_create_conn.

Now all the calls pass `true`, so the argument is no longer needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants