Skip to content

Commit

Permalink
Increase rioConnsetWrite max chunk size to 16K (#817)
Browse files Browse the repository at this point in the history
Fixes #796

Currently rioConnWite uses 1024 bytes chunk when feeding the replication
sockets on RDB write. This value seems too small and we will end up with
high syscall overhead.
**This PR sets the max chunk size to 16K.**

Using a simple test program we did not observe any significant
improvement in read/write times going with chunks bigger than 4K but
that might be la bottleneck on network throughput. We did observe sweet
point of CPU utilization at 16K when using TLS.

```
lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 24.04 LTS
Release:	24.04
Codename:	noble
```

```
uname -a
Linux ip-172-31-22-140 6.8.0-1009-aws #9-Ubuntu SMP Fri May 17 14:39:23 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
```
All files were compiled with O3 optimization level.
```
gcc --version
gcc (Ubuntu 13.2.0-23ubuntu4) 13.2.0
```

**Results:**

Chunk Size | write-time sec | writes total | write cpu-time (usr+sys) |
read-time sec | read total syscalls | read cpu-time (usr+sys)
-- | -- | -- | -- | -- | -- | --
1K | 0.162946 | 102400 | 0.185916 | 0.168479 | 2447 | 0.026945
4K | 0.163036 | 25600 | 0.122629 | 0.168627 | 715 | 0.023382
8K | 0.163942 | 12800 | 0.121131 | 0.168887 | 704 | 0.039388
16K | 0.163614 | 6400 | 0.104742 | 0.168202 | 2483 | 0.025574
64K | 0.16279 | 1600 | 0.098792 | 0.168854 | 1068 | 0.046929
1K - TLS | 0.32648 | 102400 | 0.366961 | 0.330785 | 102400 | 0.337377
4K - TLS | 0.164296 | 25600 | 0.183326 | 0.169032 | 25600 | 0.129952
8K - TLS | 0.163977 | 12800 | 0.163118 | 0.169484 | 12800 | 0.098432
16K - TLS | 0.164861 | 6400 | 0.150666 | 0.169878 | 6383 | 0.094794
64K - TLS | 0.163704 | 6400 | 0.156125 | 0.169323 | 6388 | 0.089971

---------

Signed-off-by: Ran Shidlansik <[email protected]>
Signed-off-by: ranshid <[email protected]>
Signed-off-by: Binbin <[email protected]>
Co-authored-by: Madelyn Olson <[email protected]>
Co-authored-by: Binbin <[email protected]>
  • Loading branch information
3 people authored Jul 24, 2024
1 parent 3cca268 commit 5c073a5
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/rio.c
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ static size_t rioConnsetWrite(rio *r, const void *buf, size_t len) {
* parallelize while the kernel is sending data in background to
* the TCP socket. */
while (len) {
size_t count = len < 1024 ? len : 1024;
size_t count = len < RIO_CONNSET_WRITE_MAX_CHUNK_SIZE ? len : RIO_CONNSET_WRITE_MAX_CHUNK_SIZE;
int broken = 0;
for (j = 0; j < r->io.connset.numconns; j++) {
if (r->io.connset.state[j] != 0) {
Expand Down
5 changes: 5 additions & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ struct hdr_histogram;
* and hold back additional reading based on this factor. */
#define CHILD_COW_DUTY_CYCLE 100

/* When child process is performing write to connset it iterates on the set
* writing a chunk of the available data to send on each connection.
* This constant defines the maximal size of the chunk to use. */
#define RIO_CONNSET_WRITE_MAX_CHUNK_SIZE 16384

/* Instantaneous metrics tracking. */
#define STATS_METRIC_SAMPLES 16 /* Number of samples per metric. */
#define STATS_METRIC_COMMAND 0 /* Number of commands executed. */
Expand Down

0 comments on commit 5c073a5

Please sign in to comment.