Run lz4 compression in-process for ramdisks#2283
Run lz4 compression in-process for ramdisks#2283Databean wants to merge 3 commits intogoogle:mainfrom
Conversation
| TEST(Lz4LegacyTest, RoundTrip) { | ||
| std::string original_data = | ||
| "This is some test data to compress and decompress."; | ||
| for (int i = 0; i < 10; ++i) { |
There was a problem hiding this comment.
| for (int i = 0; i < 10; ++i) { | |
| constexpr const int kCopiesOfData = 10; | |
| for (int i = 0; i < kCopiesOfData; ++i) { |
There was a problem hiding this comment.
Named kTimesToDuplicate since the size doubles with each addition here.
| size_t total_size = 10 << 20; | ||
| std::string original_data(total_size, 'a'); | ||
| for (size_t i = 0; i < total_size; ++i) { |
There was a problem hiding this comment.
| size_t total_size = 10 << 20; | |
| std::string original_data(total_size, 'a'); | |
| for (size_t i = 0; i < total_size; ++i) { | |
| size_t total_size_mb = 10 << 20; | |
| std::string original_data(total_size_mb, 'a'); | |
| for (size_t i = 0; i < total_size_mb; ++i) { |
There was a problem hiding this comment.
Named kTotalSize, I think _mb is confusing since while the variable holds a value representing a whole number of megabytes, the unit of the value is still bytes.
If you prefer, could also write it this way which keeps the units consistent:
constexpr size_t kTotalSizeMb = 10;
std::string original_data(kTotalSizeMb << 20, 'a');| size_t total_size = 10 << 20; | ||
| std::string original_data(total_size, 'a'); | ||
| for (size_t i = 0; i < total_size; ++i) { | ||
| original_data[i] = (char)(i % 256); |
There was a problem hiding this comment.
Does the actual data here matter? Could this all just be 'A'?
It feels like this is either too significant and is testing something behaviorally and should thus be called out somehow, or too insignificant and could be simplified.
There was a problem hiding this comment.
Removed the arbitrary data.
| size_t block_size = 8 << 20; | ||
| std::string data(block_size, 'x'); | ||
| ASSERT_THAT((*writer)->Write(data.data(), data.size()), | ||
| IsOkAndValue((uint64_t)block_size)); |
There was a problem hiding this comment.
| size_t block_size = 8 << 20; | |
| std::string data(block_size, 'x'); | |
| ASSERT_THAT((*writer)->Write(data.data(), data.size()), | |
| IsOkAndValue((uint64_t)block_size)); | |
| size_t block_size_mb = 8 << 20; | |
| std::string data(block_size_mb, 'x'); | |
| ASSERT_THAT((*writer)->Write(data.data(), data.size()), | |
| IsOkAndValue((uint64_t)block_size_mb)); |
There was a problem hiding this comment.
You could drop the comment and do this.
There was a problem hiding this comment.
Exposed kLz4LegacyFrameBlockSize in the header so it can be tied more closely to the implementation.
| ASSERT_THAT((*writer)->Write(data.data(), data.size()), | ||
| IsOkAndValue((uint64_t)block_size)); | ||
|
|
||
| // It should be closed now |
There was a problem hiding this comment.
| // It should be closed now | |
| // It should be closed now; try adding a bit more. |
#gemini Bug: b/494034973
Databean
force-pushed
the
lz4_legacy_writer
branch
2 times, most recently
from
ccd8b99 to
0c7bfd6
Compare
This adds Lz4LegacyWriter, which provides support for creating LZ4 Legacy frames used by the Linux kernel. It handles initial magic numbers, block-based compression (up to 8MB), and frame termination. Frame termination is triggered when a write call of size less than or equal to the block size is made. Added unit tests to verify round-trip compression/decompression and frame closure behavior. #gemini Bug: b/494034973 Test: bazel test //cuttlefish/io:lz4_legacy_test
This replaces the external dependency on the lz4 host binary with the internal Lz4LegacyWriter, avoiding a subprocess call. #gemini Bug: b/494034973 Test: Create device with kernel replacement
Databean
force-pushed
the
lz4_legacy_writer
branch
from
0c7bfd6 to
d104b08
Compare
2 participants
This avoids a subprocess launch and integrates more with the common IO abstraction library.
#gemini
Bug: b/494034973