-
Notifications
You must be signed in to change notification settings - Fork 18
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
#1649 Reuse query counters #1671
#1649 Reuse query counters #1671
Conversation
CodSpeed Performance ReportMerging #1671 will improve performances by 17.72%Comparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work!
lets land this.
I think that if we ever want to further improve performance here, the best way would be to make redis_query_name return a new enum named RedisCommandType
and then have a to_str()
method on that enum.
That way we can make this work completely with &'static str
and other usages of redis_query_name
across the codebase would also be able to avoid the allocation.
But again, I think we've hit the sweetspot of how much time to spend on this problem since this will resolve the performance impacts that are being warned about in the new metrics
release.
Yeah sounds good. We can probably change redis_query_name to improve performance opportunistically in the future. Do we want to raise an issue for it just for tracking purpose? |
I think just leave it, better in general to let performance work be driven by observed issues in the system rather then guessing which parts of the system are going to end up as the bottleneck. |
This PR stores the query counters in a HashMap and reuses them when processing messages instead of registering query counters in every transform. According to the benchmark report,
query_counter_pre_used
gains 14% improvement thanks to the fastertransform
.query_counter_fresh
becomes a bit slower but it's expected because the first transform runs create the counters and insert them into the hashmap.Progress towards #1649