New issue
Advanced search Search tips

Issue 762815 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature

Blocking:
issue 717785



Sign in to add a comment

Add write cache to update engine

Project Member Reported by ahass...@chromium.org, Sep 7 2017

Issue description

Currently writes in the update engine are quite not efficient. Add a write cache to improve its performance.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/5192fe54e9ea90c3831a4beb799b8f19a9d061d0

commit 5192fe54e9ea90c3831a4beb799b8f19a9d061d0
Author: Amin Hassani <ahassani@google.com>
Date: Thu Oct 05 01:47:11 2017

update_engine: Add Flush() to FileDescriptor

This patch adds Flush() function to the file descriptor. This function
should be called after each operation in order to ensure that data
resides to disk before any checkpointing happen. In addition, if any
future file descriptor that caches the data is added, this ensures
integrity of written data.

BUG= chromium:762815 
TEST=brillo_update_payload verify

Change-Id: I12442142647605afef1b01dc932eb7cf182af85b
Reviewed-on: https://chromium-review.googlesource.com/674163
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>

[modify] https://crrev.com/5192fe54e9ea90c3831a4beb799b8f19a9d061d0/payload_consumer/delta_performer.cc
[modify] https://crrev.com/5192fe54e9ea90c3831a4beb799b8f19a9d061d0/payload_consumer/file_descriptor.cc
[modify] https://crrev.com/5192fe54e9ea90c3831a4beb799b8f19a9d061d0/payload_consumer/file_descriptor.h
[modify] https://crrev.com/5192fe54e9ea90c3831a4beb799b8f19a9d061d0/payload_consumer/fake_file_descriptor.h

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/acd7be816579a0c13ac6f746a2555b7c73f95a3b

commit acd7be816579a0c13ac6f746a2555b7c73f95a3b
Author: Amin Hassani <ahassani@google.com>
Date: Thu Oct 05 20:19:47 2017

update_engine: Add CachedFileDescriptor

Currently there are operations in the delta_performer which are not
efficient in writing into disk. For example BzipExtentWriter and
XZExtentWriter write in 16KB buffers which is not very efficient for
today's disks specially if we are writing with O_DSYNC flag. Another
problem is operations such as bspatch which write in very small number
of bytes in order of a few bytes to a few hundred bytes. Even though
bspatch open's its own file descriptor and currently have no O_DSYNC
added, writing in very small bytes into disk in general is not a good
idea for the purpose of the update engine.

This patch adds new wrapper for a FileDescriptor interface which caches
all the writes into a large buffer (like 1MB) and writes it into disk
when either a 'Flush' request comes or if the offset of the file has
been changed to a different position than what it is writing now. This
cached buffer can be wrapped around target_fd_ and every operation can
write into it. This centralizes all write oprerations through one file
descriptor and gives greater control on how writes should be performed.

BUG= chromium:762815 
TEST=FEATURES="test" emerge-amd-generic update_engine; brillo_update_payload verify

Change-Id: I4911d917ec9362bf66c4dd2ed19d89440042733c
Reviewed-on: https://chromium-review.googlesource.com/674164
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/acd7be816579a0c13ac6f746a2555b7c73f95a3b/Android.mk
[modify] https://crrev.com/acd7be816579a0c13ac6f746a2555b7c73f95a3b/update_engine.gyp
[add] https://crrev.com/acd7be816579a0c13ac6f746a2555b7c73f95a3b/payload_consumer/cached_file_descriptor.cc
[add] https://crrev.com/acd7be816579a0c13ac6f746a2555b7c73f95a3b/payload_consumer/cached_file_descriptor.h
[add] https://crrev.com/acd7be816579a0c13ac6f746a2555b7c73f95a3b/payload_consumer/cached_file_descriptor_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/a3fc20af5ea6b75c568122065f487b23aaabf1d2

commit a3fc20af5ea6b75c568122065f487b23aaabf1d2
Author: Amin Hassani <ahassani@google.com>
Date: Tue Oct 10 05:23:49 2017

update_engine: Activate CachedFileDescriptor

This patch activates CachedFileDescriptor for use in target_fd_. We
intend to use ExtentWriter for all payload operations. This patch alone
decreased the update time for a lumpy device from around 16 minutes down
to 2:30 minutes without necessarily putting pressure on memory. And that
is with O_DSYNC flag on.

BUG= chromium:762815 
TEST=FEATURES="test" emerge-amd-generic update_engine; brillo_update_payload verify

Change-Id: Ie35a5e4d320496a9793c202c43271ff02b95a788
Reviewed-on: https://chromium-review.googlesource.com/674165
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>

[modify] https://crrev.com/a3fc20af5ea6b75c568122065f487b23aaabf1d2/payload_consumer/delta_performer.cc

Blocking: 717785
Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/d9cb290966a77730010aa01f0db675c157cd5c8f

commit d9cb290966a77730010aa01f0db675c157cd5c8f
Author: Amin Hassani <ahassani@google.com>
Date: Wed Jan 03 22:23:33 2018

update_engine: Unify disk access for SOURCE_BSDIFF through update_engine

Switch bspatch to use ExtentReader and ExtentWriter. This allows to:
- Cache the writes.
- Unify all the reads through ExtentReader object.
- Unify all the writes through ExtentWrite object.

This patch allows running bspatch without being dependent on how bspatch reads
and writes from disk. The disk access can be controlled through update_engine
itself. Currently, SOURCE_BSDIFF operation can still cause bugs such as
b/31709028. But through this mechanism, those kind of problems can be
alleviated.

BUG= chromium:762815 
TEST=unittests pass; brillo_update_payload verify passes;

Change-Id: I3f8e29366743d0383d41bf3700e9db3135fbd82a
Reviewed-on: https://chromium-review.googlesource.com/683357
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/d9cb290966a77730010aa01f0db675c157cd5c8f/payload_consumer/delta_performer.cc

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 8 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment