refactor: reduce memory copying during incremental synchronization #2689
Conversation
RiversJin
changed the title
RiversJin
force-pushed
the
refact/replication
branch
from
269c716 to
d0f82d9
Compare
| } else { | ||
| char* bulk_data = reinterpret_cast<char *>(evbuffer_pullup(input, static_cast<ssize_t>(incr_bulk_len_ + 2))); | ||
| std::string bulk_string = std::string(bulk_data, incr_bulk_len_); | ||
| evbuffer_drain(input, incr_bulk_len_ + 2); |
There was a problem hiding this comment.
Not related to this issue directly
I'm not familiar with evbuffer api, but would
std::string bulk_string(0, incr_bulk_len_);
evbuffer_remove(input, bulk_string.data(), incr_bulk_len_ + 2);Being ok since it avoid adjust the input internal? (The bad things is that std::string would zero-initialize itself, which introducing a round of copying 😅) @git-hulk @PragmaTwice
There was a problem hiding this comment.
oh let's not optimize this if no benchmark shows it's better...
RiversJin
force-pushed
the
refact/replication
branch
from
d0f82d9 to
6b6eedf
Compare
…ronization Optimized the logic for handling Psync incremental data on replica nodes, reducing an unnecessary data copy and lowering the loop complexity in the corresponding logic.
RiversJin
force-pushed
the
refact/replication
branch
from
6b6eedf to
514b57d
Compare
|
You can run Additional, please ensure Read https://kvrocks.apache.org/community/contributing for more information. |
|
Also, although it's not mandatory, it's good to make your further changes small additional commits instead of amending to the existing commit, to make reviewers easy to know what you change after a round of reviewing. |
ok, fixed. I was a bit surprised to find that different versions of clang-format can produce different results under the same configuration file... |
|
|
Thank you for your contribution! |
Optimized the logic for handling Psync incremental data on replica nodes, reducing an unnecessary data copy and lowering the loop complexity in the corresponding logic.