Skip to content

Implements lazy buffer rotations#415

Open
mzimbres wants to merge 1 commit intoboostorg:developfrom
mzimbres:lazy_buffer_rotations
Open

Implements lazy buffer rotations#415
mzimbres wants to merge 1 commit intoboostorg:developfrom
mzimbres:lazy_buffer_rotations

Conversation

@mzimbres
Copy link
Copy Markdown
Collaborator

@mzimbres mzimbres commented May 2, 2026

The read buffer won't rotate data if it can be increased without allocating more memory. The table below compares the performance before and after the changes

Strategy time(s) %usr %sys %CPU rotation
Eager 92.22 67.37 2.92 70.29 6.7 Gbps
Lazy 91.16 60.15 2.82 62.97 99.0 kbps

The biggest gain in the benchmark above is in CPU usage because the overall time cannot be decreased much since the client is far from saturating the CPU.

@mzimbres mzimbres requested a review from anarthal May 2, 2026 19:41
@cppalliance-bot
Copy link
Copy Markdown

cppalliance-bot commented May 2, 2026

An automated preview of the documentation is available at https://415.redis.prtest3.cppalliance.org/libs/redis/doc/html/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-05-03 17:50:39 UTC

@anarthal
Copy link
Copy Markdown
Collaborator

anarthal commented May 3, 2026

Thanks. Will have a look soon.

The read buffer won't rotate data if it can be increased without allocating
more memory. The table below compares the performance before and after the
changes

          | time(s) | %usr  | %sys | %CPU  | rotation
    ------|---------| ------|------|-------|------------
    Eager | 92.22   | 67.37 | 2.92 | 70.29 |   6.7 Gbps
    Lazy  | 91.16   | 60.15 | 2.82 | 62.97 |  99.0 kbps

The biggest gain in the benchmark above is in CPU usage because the overall
time cannot be decreased much since the client is far from saturating the CPU.
@mzimbres mzimbres force-pushed the lazy_buffer_rotations branch from 53e42d1 to 83dcec3 Compare May 3, 2026 17:46
Copy link
Copy Markdown
Collaborator

@anarthal anarthal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@anarthal
Copy link
Copy Markdown
Collaborator

anarthal commented May 6, 2026

Since GH seems buggy, I'll place my review in a comment to avoid losing it:

@anarthal
Copy link
Copy Markdown
Collaborator

anarthal commented May 6, 2026

I've confirmed with vtune that the reader now takes less proportion of CPU time. Memmove no longer appears in the flame graph. Feel free to merge this when you want.

Something that'd be interesting is rewriting the read buffer without a vector, directly with a unique_ptr<char>. My experience is that this improves performance more than expected. But we can defer this to later.

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.

3 participants