Cluster code micro-optimization - #4094
Conversation
A bunch of low-hanging, low-impact micro-optimizations. Mostly common subexpression extraction to local variables when beneficial. Barely noticeable benchmark impact but technically an improvement without readability impact IMO.
Concentrate around key extraction and node determination. Optimizes the most common path for key extraction which is that of single-key commands.
Common subexpression factorization
Getting the policy callback out of all the dicts is very indirect and wasteful for simple commands. Cache simple commands so the next time it only takes a single lookup. About 3.7% improvement on overall benchmark.
Avoid resolving the same command policies over and over. All resolvers are "static" in the short term, during a single pipeline execution, so cache their results for the duration to reduce the amount of work done.
Simple commands have simple responses, so a fast path for those improves response parsing speed in the average
Another big speedup in command policy resolution
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
Were meant for pre-PR testing only
|
Hi @klaussfreire, thank you for your contribution! I'll take a look at it shortly. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Reviewed by Cursor Bugbot for commit f3bff9e. Configure here.
Hi. Is there anything I can do to help move this forward? |

Description of change
A bunch of low-hanging micro-optimizations.
Mostly common subexpression extraction to local variables when beneficial. Barely noticeable benchmark impact but technically an improvement without readability impact IMO.
One of the most impactful changes concentrates around key extraction and node determination.
It optimizes the most common path for key extraction which is that of single-key commands,
precomputing key position during command parsing to have a simple fast path in get_keys.
This last one does produce a large impact in cluster benchmarks, of around 20%.
Some optimizations also apply to regular (non-cluster) clients, but the main concern of this optimization effort was around cluster ops, so they were not benchmarked.
Benchmark results
This uses a benchmark similar to those in basic_operations, but with a cluster client.
Two pipeline patterns were used:
Both patterns include value payloads of around 50 bytes each.
The first one stresses multiple command types with various return types, while the second one stresses a streamlined single-command pipeline with limited response data.
The benchmark was run in AWS against a 3-node valkey EC cluster (EC versions of Redis OSS does not support setex), using hiredis 3.3.1. The setup builds a mostly CPU-bound case with a very fast but still realistic network between client and cluster.
Large pipelines show as dnf because they get deadlocked, something that will be covered on another PR.
Numbers are given in requests per second (RPS) and relative uplift.
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Note
Medium Risk
Changes sit on cluster slot/key routing and command key parsing; behavior should match prior logic but mistakes could misroute commands or break edge cases (subcommands, negative key positions).
Overview
Cluster routing and pipelines get the biggest wins:
CommandsParsernow tags single-key commands with_single_key_posat init and takes a fast path inget_keys, plus_is_keyed_commandso cluster pipelines can cache keyedCommandPoliciesfor simple commands._determine_nodescaches resolved policy callbacks per command name (capped at 5000 entries), and policy lambdas take(self, command, …)so one unifiedpolicy_cb(self, …)call replaces per-policy branching. Pipeline execution reuses local refs, per-batchpolicy_cache, and the keyed-command cache when inferring policies.Smaller hot-path tweaks include a no-options fast path in
Redis.parse_response, earlystrhandling inEncoder.encode,bool_okwithout always decoding to str, boundedsplitin policy resolvers, a localreaderin async hiredisread_response, and benchmarkStringJoiningConnectionjoining list commands beforesendall.Reviewed by Cursor Bugbot for commit b37fdbb. Bugbot is set up for automated code reviews on this repo. Configure here.