AU: Speed up updates by using buffered writes.
Given that SSD writes are really slow, this improves performance significantly.
This reduces update time on AGZ from 375 to 100 seconds.
BUG=6901
TEST=unit tests, gmerged on device and measured speed
Change-Id: Idac7743f6eaa8f26878a2d1f6b9401513e2ca600
Review URL: http://codereview.chromium.org/3471006
diff --git a/SConstruct b/SConstruct
index a9bf9eb..70803e7 100644
--- a/SConstruct
+++ b/SConstruct
@@ -193,6 +193,7 @@
env['LIBS'] += ['bz2', 'gcov']
sources = Split("""action_processor.cc
+ buffered_file_writer.cc
bzip.cc
bzip_extent_writer.cc
cycle_breaker.cc
@@ -232,6 +233,7 @@
unittest_sources = Split("""action_unittest.cc
action_pipe_unittest.cc
action_processor_unittest.cc
+ buffered_file_writer_unittest.cc
bzip_extent_writer_unittest.cc
cycle_breaker_unittest.cc
decompressing_file_writer_unittest.cc
diff --git a/buffered_file_writer.cc b/buffered_file_writer.cc
new file mode 100644
index 0000000..03b8c77
--- /dev/null
+++ b/buffered_file_writer.cc
@@ -0,0 +1,60 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/buffered_file_writer.h"
+
+namespace chromeos_update_engine {
+
+BufferedFileWriter::BufferedFileWriter(FileWriter* next, size_t buffer_size)
+ : next_(next),
+ buffer_(new char[buffer_size]),
+ buffer_size_(buffer_size),
+ buffered_bytes_(0) {}
+
+BufferedFileWriter::~BufferedFileWriter() {}
+
+int BufferedFileWriter::Open(const char* path, int flags, mode_t mode) {
+ return next_->Open(path, flags, mode);
+}
+
+ssize_t BufferedFileWriter::Write(const void* bytes, size_t count) {
+ size_t copied_bytes = 0;
+ while (copied_bytes < count) {
+ // Buffers as many bytes as possible.
+ size_t remaining_free = buffer_size_ - buffered_bytes_;
+ size_t copy_bytes = count - copied_bytes;
+ if (copy_bytes > remaining_free) {
+ copy_bytes = remaining_free;
+ }
+ const char* char_bytes = reinterpret_cast<const char*>(bytes);
+ memcpy(&buffer_[buffered_bytes_], char_bytes + copied_bytes, copy_bytes);
+ buffered_bytes_ += copy_bytes;
+ copied_bytes += copy_bytes;
+
+ // If full, sends the buffer to the next FileWriter.
+ if (buffered_bytes_ == buffer_size_) {
+ ssize_t rc = WriteBuffer();
+ if (rc < 0)
+ return rc;
+ }
+ }
+ return count;
+}
+
+int BufferedFileWriter::Close() {
+ // Flushes the buffer first, then closes the next FileWriter.
+ ssize_t rc_write = WriteBuffer();
+ int rc_close = next_->Close();
+ return (rc_write < 0) ? rc_write : rc_close;
+}
+
+ssize_t BufferedFileWriter::WriteBuffer() {
+ if (buffered_bytes_ == 0)
+ return 0;
+ ssize_t rc = next_->Write(&buffer_[0], buffered_bytes_);
+ buffered_bytes_ = 0;
+ return rc;
+}
+
+} // namespace chromeos_update_engine
diff --git a/buffered_file_writer.h b/buffered_file_writer.h
new file mode 100644
index 0000000..0457285
--- /dev/null
+++ b/buffered_file_writer.h
@@ -0,0 +1,53 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_BUFFERED_FILE_WRITER_H__
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_BUFFERED_FILE_WRITER_H__
+
+#include <base/scoped_ptr.h>
+#include <gtest/gtest_prod.h> // for FRIEND_TEST
+
+#include "update_engine/file_writer.h"
+
+// BufferedFileWriter is an implementation of FileWriter that buffers all data
+// before passing it onto another FileWriter.
+
+namespace chromeos_update_engine {
+
+class BufferedFileWriter : public FileWriter {
+ public:
+ BufferedFileWriter(FileWriter* next, size_t buffer_size);
+ virtual ~BufferedFileWriter();
+
+ virtual int Open(const char* path, int flags, mode_t mode);
+ virtual ssize_t Write(const void* bytes, size_t count);
+ virtual int Close();
+
+ private:
+ FRIEND_TEST(BufferedFileWriterTest, CloseErrorTest);
+ FRIEND_TEST(BufferedFileWriterTest, CloseNoDataTest);
+ FRIEND_TEST(BufferedFileWriterTest, CloseWriteErrorTest);
+ FRIEND_TEST(BufferedFileWriterTest, ConstructorTest);
+ FRIEND_TEST(BufferedFileWriterTest, WriteBufferNoDataTest);
+ FRIEND_TEST(BufferedFileWriterTest, WriteBufferTest);
+ FRIEND_TEST(BufferedFileWriterTest, WriteErrorTest);
+ FRIEND_TEST(BufferedFileWriterTest, WriteTest);
+ FRIEND_TEST(BufferedFileWriterTest, WriteWrapBufferTest);
+
+ // If non-empty, sends the buffer to the next FileWriter.
+ ssize_t WriteBuffer();
+
+ // The FileWriter that we write all buffered data to.
+ FileWriter* const next_;
+
+ scoped_array<char> buffer_;
+ size_t buffer_size_;
+ size_t buffered_bytes_;
+
+ DISALLOW_COPY_AND_ASSIGN(BufferedFileWriter);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_BUFFERED_FILE_WRITER_H__
diff --git a/buffered_file_writer_unittest.cc b/buffered_file_writer_unittest.cc
new file mode 100644
index 0000000..1c39a4e
--- /dev/null
+++ b/buffered_file_writer_unittest.cc
@@ -0,0 +1,132 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <gtest/gtest.h>
+
+#include "update_engine/buffered_file_writer.h"
+#include "update_engine/file_writer_mock.h"
+
+using std::string;
+using testing::_;
+using testing::InSequence;
+using testing::Return;
+
+namespace chromeos_update_engine {
+
+static const int kTestFlags = O_CREAT | O_LARGEFILE | O_TRUNC | O_WRONLY;
+static const mode_t kTestMode = 0644;
+
+MATCHER_P2(MemCmp, expected, n, "") { return memcmp(arg, expected, n) == 0; }
+
+class BufferedFileWriterTest : public ::testing::Test {
+ protected:
+ FileWriterMock file_writer_mock_;
+};
+
+TEST_F(BufferedFileWriterTest, CloseErrorTest) {
+ BufferedFileWriter writer(&file_writer_mock_, 100);
+ writer.buffered_bytes_ = 1;
+ InSequence s;
+ EXPECT_CALL(file_writer_mock_, Write(&writer.buffer_[0], 1))
+ .Times(1)
+ .WillOnce(Return(50));
+ EXPECT_CALL(file_writer_mock_, Close()).Times(1).WillOnce(Return(-20));
+ EXPECT_EQ(-20, writer.Close());
+}
+
+TEST_F(BufferedFileWriterTest, CloseNoDataTest) {
+ BufferedFileWriter writer(&file_writer_mock_, 50);
+ InSequence s;
+ EXPECT_CALL(file_writer_mock_, Write(_, _)).Times(0);
+ EXPECT_CALL(file_writer_mock_, Close()).Times(1).WillOnce(Return(-30));
+ EXPECT_EQ(-30, writer.Close());
+}
+
+TEST_F(BufferedFileWriterTest, CloseWriteErrorTest) {
+ BufferedFileWriter writer(&file_writer_mock_, 100);
+ writer.buffered_bytes_ = 2;
+ InSequence s;
+ EXPECT_CALL(file_writer_mock_, Write(&writer.buffer_[0], 2))
+ .Times(1)
+ .WillOnce(Return(-10));
+ EXPECT_CALL(file_writer_mock_, Close()).Times(1).WillOnce(Return(-20));
+ EXPECT_EQ(-10, writer.Close());
+}
+
+TEST_F(BufferedFileWriterTest, ConstructorTest) {
+ BufferedFileWriter writer(&file_writer_mock_, 123);
+ EXPECT_EQ(&file_writer_mock_, writer.next_);
+ EXPECT_EQ(123, writer.buffer_size_);
+ EXPECT_TRUE(writer.buffer_ != NULL);
+ EXPECT_EQ(0, writer.buffered_bytes_);
+}
+
+TEST_F(BufferedFileWriterTest, OpenTest) {
+ BufferedFileWriter writer(&file_writer_mock_, 100);
+ EXPECT_CALL(file_writer_mock_, Open("foo", kTestFlags, kTestMode))
+ .Times(1)
+ .WillOnce(Return(20));
+ EXPECT_EQ(20, writer.Open("foo", kTestFlags, kTestMode));
+}
+
+TEST_F(BufferedFileWriterTest, WriteBufferNoDataTest) {
+ BufferedFileWriter writer(&file_writer_mock_, 300);
+ EXPECT_CALL(file_writer_mock_, Write(_, _)).Times(0);
+ EXPECT_EQ(0, writer.WriteBuffer());
+}
+
+TEST_F(BufferedFileWriterTest, WriteBufferTest) {
+ BufferedFileWriter writer(&file_writer_mock_, 200);
+ writer.buffered_bytes_ = 100;
+ EXPECT_CALL(file_writer_mock_, Write(&writer.buffer_[0], 100))
+ .Times(1)
+ .WillOnce(Return(-10));
+ EXPECT_EQ(-10, writer.WriteBuffer());
+}
+
+TEST_F(BufferedFileWriterTest, WriteErrorTest) {
+ string kData = "test_data";
+ BufferedFileWriter writer(&file_writer_mock_, 4);
+ InSequence s;
+ EXPECT_CALL(file_writer_mock_, Write(MemCmp("test", 4), 4))
+ .Times(1)
+ .WillOnce(Return(3));
+ EXPECT_CALL(file_writer_mock_, Write(MemCmp("_dat", 4), 4))
+ .Times(1)
+ .WillOnce(Return(-10));
+ EXPECT_EQ(-10, writer.Write(kData.data(), kData.size()));
+ EXPECT_EQ(0, writer.buffered_bytes_);
+}
+
+TEST_F(BufferedFileWriterTest, WriteTest) {
+ string kData = "test_data";
+ BufferedFileWriter writer(&file_writer_mock_, 100);
+ writer.buffered_bytes_ = 10;
+ EXPECT_CALL(file_writer_mock_, Write(_, _)).Times(0);
+ EXPECT_EQ(kData.size(), writer.Write(kData.data(), kData.size()));
+ EXPECT_EQ(0, memcmp(kData.data(), &writer.buffer_[10], kData.size()));
+ EXPECT_EQ(kData.size() + 10, writer.buffered_bytes_);
+}
+
+TEST_F(BufferedFileWriterTest, WriteWrapBufferTest) {
+ string kData = "test_data1";
+ BufferedFileWriter writer(&file_writer_mock_, 3);
+ writer.buffered_bytes_ = 1;
+ writer.buffer_[0] = 'x';
+ InSequence s;
+ EXPECT_CALL(file_writer_mock_, Write(MemCmp("xte", 3), 3))
+ .Times(1)
+ .WillOnce(Return(3));
+ EXPECT_CALL(file_writer_mock_, Write(MemCmp("st_", 3), 3))
+ .Times(1)
+ .WillOnce(Return(3));
+ EXPECT_CALL(file_writer_mock_, Write(MemCmp("dat", 3), 3))
+ .Times(1)
+ .WillOnce(Return(3));
+ EXPECT_EQ(kData.size(), writer.Write(kData.data(), kData.size()));
+ EXPECT_EQ(0, memcmp("a1", &writer.buffer_[0], 2));
+ EXPECT_EQ(2, writer.buffered_bytes_);
+}
+
+} // namespace chromeos_update_engine
diff --git a/download_action.cc b/download_action.cc
index 5096703..9779f2a 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -17,6 +17,9 @@
namespace chromeos_update_engine {
+// Use a buffer to reduce the number of IOPS on SSD devices.
+const size_t kFileWriterBufferSize = 128 * 1024; // 128 KiB
+
DownloadAction::DownloadAction(HttpFetcher* http_fetcher)
: writer_(NULL),
http_fetcher_(http_fetcher),
@@ -41,8 +44,15 @@
if (install_plan_.is_full_update) {
kernel_file_writer_.reset(new DirectFileWriter);
rootfs_file_writer_.reset(new DirectFileWriter);
- split_file_writer_.reset(new SplitFileWriter(kernel_file_writer_.get(),
- rootfs_file_writer_.get()));
+ kernel_buffered_file_writer_.reset(
+ new BufferedFileWriter(kernel_file_writer_.get(),
+ kFileWriterBufferSize));
+ rootfs_buffered_file_writer_.reset(
+ new BufferedFileWriter(rootfs_file_writer_.get(),
+ kFileWriterBufferSize));
+ split_file_writer_.reset(
+ new SplitFileWriter(kernel_buffered_file_writer_.get(),
+ rootfs_buffered_file_writer_.get()));
split_file_writer_->SetFirstOpenArgs(
install_plan_.kernel_install_path.c_str(),
O_WRONLY | O_CREAT | O_TRUNC | O_LARGEFILE,
diff --git a/download_action.h b/download_action.h
index c0c36f1..f6d5a11 100644
--- a/download_action.h
+++ b/download_action.h
@@ -11,13 +11,13 @@
#include <string>
+#include <base/scoped_ptr.h>
#include <curl/curl.h>
-#include "base/scoped_ptr.h"
#include "update_engine/action.h"
#include "update_engine/decompressing_file_writer.h"
#include "update_engine/delta_performer.h"
-#include "update_engine/file_writer.h"
+#include "update_engine/buffered_file_writer.h"
#include "update_engine/http_fetcher.h"
#include "update_engine/install_plan.h"
#include "update_engine/omaha_hash_calculator.h"
@@ -105,6 +105,8 @@
scoped_ptr<SplitFileWriter> split_file_writer_;
scoped_ptr<DirectFileWriter> kernel_file_writer_;
scoped_ptr<DirectFileWriter> rootfs_file_writer_;
+ scoped_ptr<BufferedFileWriter> kernel_buffered_file_writer_;
+ scoped_ptr<BufferedFileWriter> rootfs_buffered_file_writer_;
// Used to apply a delta update:
scoped_ptr<DeltaPerformer> delta_performer_;
diff --git a/file_writer.h b/file_writer.h
index 1a0cc23..a534b96 100644
--- a/file_writer.h
+++ b/file_writer.h
@@ -54,7 +54,7 @@
private:
int fd_;
-
+
DISALLOW_COPY_AND_ASSIGN(DirectFileWriter);
};
@@ -69,7 +69,7 @@
}
private:
FileWriter* writer_;
-
+
DISALLOW_COPY_AND_ASSIGN(ScopedFileWriterCloser);
};
diff --git a/file_writer_mock.h b/file_writer_mock.h
new file mode 100644
index 0000000..004c3b1
--- /dev/null
+++ b/file_writer_mock.h
@@ -0,0 +1,22 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_FILE_WRITER_MOCK_H__
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_FILE_WRITER_MOCK_H__
+
+#include "gmock/gmock.h"
+#include "update_engine/file_writer.h"
+
+namespace chromeos_update_engine {
+
+class FileWriterMock : public FileWriter {
+ public:
+ MOCK_METHOD3(Open, int(const char* path, int flags, mode_t mode));
+ MOCK_METHOD2(Write, ssize_t(const void* bytes, size_t count));
+ MOCK_METHOD0(Close, int());
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_FILE_WRITER_MOCK_H__