-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduced memory overhead of preparing LZ4-compressed data for server. #110
Reduced memory overhead of preparing LZ4-compressed data for server. #110
Conversation
Do not compress a whole serialized block, but instead only a reasonable-sized chunk. This removes some temporary buffers and reduces memory pressure. Also minor refactoring: - moved all serialization-format code to WireFormat class. - removed CodedOutputStream and CodedInputStream classes.
009b8af
to
b148348
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left inline comments.
Could you please provide some numbers of improvements (and absence of degradation)? Like "before vs after" perf tests results, etc. |
if (estimated_compressed_buffer_size <= 0) | ||
throw std::runtime_error("Failed to estimate compressed buffer size, LZ4 error: " + std::to_string(estimated_compressed_buffer_size)); | ||
|
||
compressed_buffer_.resize(estimated_compressed_buffer_size + HEADER_SIZE + EXTRA_COMPRESS_BUFFER_SIZE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a low hanging fruit for optimization: resize without initialization.
Do not compress a whole serialized block, but instead only a reasonable-sized chunk.
This removes some temporary buffers and reduces memory pressure.
Also minor refactoring:
Memory usage
Test executed against a single block of 5 columns by
INSERT
ing andSELECT
ing rows back from the server. Both binaries were built inRelWithDebInfo
mode.initial
- almost right after the program startbefore INSERTing
- a moment when the block is prepared in memory, but before actual call toClient::Insert
after INSERTing
- right afterClient::Insert
, NOTE: original Block is still in memory for validation.after SELECTing
- right afterclient.Select("SELECT * FROM ...")
, NOTE: original Block is still in memory for validation.Original implementation
Version in this PR
Comparison
Since the original version failed to insert 100M rows, we are going to compare 10M rows memory usage.
As you can see, original implementation peaks to 1117761536 bytes upon insertion, 503341056 of whose are related to the original
Block
residing in memory, that is 1117761536 - 503341056 = 614420480 bytes (~0.57Gib) of memory used only to INSERT and send data to the server.The modified implementation uses an insignificant amount of extra memory, untraceable with the current approach. Moreover, it is undetectable even upon insertion of 100M rows.
Conclusion
Modified version (presented in this PR) uses
O(1)
extra memory, vsO(n)
for the original version.