Introduce shared query buffer for client reads

[uriyage]

This PR optimizes client query buffer handling in Valkey by introducing a shared query buffer that is used by default for client reads. This reduces memory usage by ~20KB per client by avoiding allocations for most clients using short (<16KB) complete commands. For larger or partial commands, the client still gets its own private buffer.

The primary changes are:

• Adding a shared query buffer shared_qb that clients use by default
• Modifying client querybuf initialization and reset logic
• Copying any partial query from shared to private buffer before command execution
• Freeing idle client query buffers when empty to allow reuse of shared buffer
• Master client query buffers are kept private as their contents need to be preserved for replication stream

In addition to the memory savings, this change shows a 3% improvement in latency and throughput when running with 1000 active clients.

The memory reduction may also help reduce the need to evict clients when reaching max memory limit, as the query buffer is the main memory consumer per client.

[madolson]

Overall it looks good to me. Can you document the procedure you were using when you saw the 3% performance boost? Did we also test it for pipelined clients?

I would also ideally like some protection in freeClient() to make sure if a client gets freed it somehow doesn't also free the shard query buffer.

[uriyage]

Performance Benchmark:

Server Setup:

• 3 million keys, each with 512 bytes

Benchmark Configuration:

• 1,000 clients running SET commands
• Server and benchmark running on separate ARM instances with 64 cores

Server Command:

./valkey-server --save

Benchmark Command:

./valkey-benchmark -t set -d 512 -r 3000000 -c 1000 --threads 50 -h <server_host_address> -n 50000000

Results (Average of 3 runs):

1. Without shared query buffer:

   • Throughput: 208,361 operations per second
   • Average latency: 4.806 milliseconds

2. With shared query buffer:

   • Throughput: 214,062 operations per second
   • Average latency: 4.692 milliseconds

[PingXie]

This change LTGM overall. Thanks @uriyage

[zuiderkwast]

Generally LGTM

[madolson]

I'm good with it. @uriyage Can you do the merge to get it up to date?

[codecov]

Codecov Report

Attention: Patch coverage is 96.22642% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 70.25%. Comparing base (045d475) to head (05224a5).
Report is 3 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #258      +/-   ##
============================================
+ Coverage     69.99%   70.25%   +0.25%     
============================================
  Files           109      109              
  Lines         59913    59957      +44     
============================================
+ Hits          41938    42122     +184     
+ Misses        17975    17835     -140     

Files	Coverage Δ
src/replication.c	87.21% <100.00%> (+<0.01%)	⬆️
src/server.c	88.93% <100.00%> (+0.02%)	⬆️
src/networking.c	85.40% <95.23%> (+0.27%)	⬆️

... and 16 files with indirect coverage changes

[soloestoy]

@uriyage can you give more tests about the cases that the whole command or argv exceed PROTO_IOBUF_LEN?

[uriyage]

@uriyage can you give move test results about the cases that the whole command or argv exceed PROTO_IOBUF_LEN?

I checked with 24K values (since with jemalloc we actually allocate more than 16K).

Commands:

./valkey-server --protected-mode no --save

./valkey-benchmark -t set -d 24000 -r 1000 -c 500 --threads 4 -h 192.31.233.204 -n 10000000

I get 83,544 without a shared query buffer and 83,238 with a shared query buffer.

So, essentially about the same, which is expected since with larger values, we revert to the current logic where we use a private buffer per client.

[uriyage]

I'm good with it. @uriyage Can you do the merge to get it up to date?

@madolson Done

[madolson]

@soloestoy Do you have any further followup?

[soloestoy]

I'm thinking if we can eliminate the limit of shared query buffer (and resize in cron if needed), maybe we can get more benefits.

[uriyage]

I'm thinking if we can eliminate the limit of shared query buffer (and resize in cron if needed), maybe we can get more benefits.

@soloestoy once we exceed the initial read size for the shared query buffer, we need to transfer the data to the client's private buffer. Leaving the data in the shared buffer isn't an option, therefore, the subsequent read will be anyway directed to the client's private query buffer.

[madolson]

Style comments from clang format change.