AU: Optimize BzipExtentWriter's memory use.
This patch introduces the following two changes:
- Reduces the output buffer size from 1MiB to 16KiB.
- Avoids copying of the input data when there's no leftover data. This
covers the current DeltaPerformer usage of the writer.
BUG=chromium:234578
TEST=unit tests; tested full update on device
Change-Id: I44f74b4fd8b05b082fde84d5ac06d9b1c1177b81
Reviewed-on: https://gerrit.chromium.org/gerrit/49047
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: Darin Petkov <petkov@chromium.org>
diff --git a/bzip_extent_writer.cc b/bzip_extent_writer.cc
index 9d432c7..b4f864f 100644
--- a/bzip_extent_writer.cc
+++ b/bzip_extent_writer.cc
@@ -9,7 +9,7 @@
namespace chromeos_update_engine {
namespace {
-const vector<char>::size_type kOutputBufferLength = 1024 * 1024;
+const vector<char>::size_type kOutputBufferLength = 16 * 1024;
}
bool BzipExtentWriter::Init(int fd,
@@ -28,39 +28,45 @@
bool BzipExtentWriter::Write(const void* bytes, size_t count) {
vector<char> output_buffer(kOutputBufferLength);
- const char* c_bytes = reinterpret_cast<const char*>(bytes);
+ // Copy the input data into |input_buffer_| only if |input_buffer_| already
+ // contains unconsumed data. Otherwise, process the data directly from the
+ // source.
+ const char* input = reinterpret_cast<const char*>(bytes);
+ const char* input_end = input + count;
+ if (!input_buffer_.empty()) {
+ input_buffer_.insert(input_buffer_.end(), input, input_end);
+ input = &input_buffer_[0];
+ input_end = input + input_buffer_.size();
+ }
+ stream_.next_in = const_cast<char*>(input);
+ stream_.avail_in = input_end - input;
- input_buffer_.insert(input_buffer_.end(), c_bytes, c_bytes + count);
-
- stream_.next_in = &input_buffer_[0];
- stream_.avail_in = input_buffer_.size();
-
for (;;) {
stream_.next_out = &output_buffer[0];
stream_.avail_out = output_buffer.size();
int rc = BZ2_bzDecompress(&stream_);
TEST_AND_RETURN_FALSE(rc == BZ_OK || rc == BZ_STREAM_END);
-
+
if (stream_.avail_out == output_buffer.size())
break; // got no new bytes
-
+
TEST_AND_RETURN_FALSE(
next_->Write(&output_buffer[0],
output_buffer.size() - stream_.avail_out));
-
+
if (rc == BZ_STREAM_END)
CHECK_EQ(stream_.avail_in, static_cast<unsigned int>(0));
if (stream_.avail_in == 0)
break; // no more input to process
}
- // store unconsumed data in input_buffer_.
-
- vector<char> new_input_buffer(input_buffer_.end() - stream_.avail_in,
- input_buffer_.end());
- new_input_buffer.swap(input_buffer_);
-
+ // Store unconsumed data (if any) in |input_buffer_|.
+ if (stream_.avail_in || !input_buffer_.empty()) {
+ vector<char> new_input_buffer(input_end - stream_.avail_in, input_end);
+ new_input_buffer.swap(input_buffer_);
+ }
+
return true;
}
diff --git a/bzip_extent_writer_unittest.cc b/bzip_extent_writer_unittest.cc
index d724bc4..38ba117 100644
--- a/bzip_extent_writer_unittest.cc
+++ b/bzip_extent_writer_unittest.cc
@@ -53,8 +53,8 @@
extents.push_back(extent);
// 'echo test | bzip2 | hexdump' yields:
- const char test_uncompressed[] = "test\n";
- unsigned char test[] = {
+ static const char test_uncompressed[] = "test\n";
+ static const unsigned char test[] = {
0x42, 0x5a, 0x68, 0x39, 0x31, 0x41, 0x59, 0x26, 0x53, 0x59, 0xcc, 0xc3,
0x71, 0xd4, 0x00, 0x00, 0x02, 0x41, 0x80, 0x00, 0x10, 0x02, 0x00, 0x0c,
0x00, 0x20, 0x00, 0x21, 0x9a, 0x68, 0x33, 0x4d, 0x19, 0x97, 0x8b, 0xb9,
@@ -101,12 +101,16 @@
BzipExtentWriter bzip_writer(&direct_writer);
EXPECT_TRUE(bzip_writer.Init(fd(), extents, kBlockSize));
+ vector<char> original_compressed_data = compressed_data;
for (vector<char>::size_type i = 0; i < compressed_data.size();
i += kChunkSize) {
size_t this_chunk_size = min(kChunkSize, compressed_data.size() - i);
EXPECT_TRUE(bzip_writer.Write(&compressed_data[i], this_chunk_size));
}
EXPECT_TRUE(bzip_writer.End());
+
+ // Check that the const input has not been clobbered.
+ ExpectVectorsEq(original_compressed_data, compressed_data);
vector<char> output(kDecompressedLength + 1);
ssize_t bytes_read = pread(fd(), &output[0], output.size(), 0);