New issue
Advanced search Search tips

Issue 768505 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 696813
issue 717785



Sign in to add a comment

Make puffin to use BitExtent instead of ByteExtent.

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

Issue description

Currently we use byte addressing for addressing deflate locations in the puffin. But this is not a good idea if we need to break large deflate blocks into smaller ones for memory efficiency, then we need to address deflates based on bits. 

Currently we end Huff or Puff operation based on if we see the end of block bit, but this doesn't allow breaking a deflate block into smaller subblocks. So first change these operations to end based on the size of the input.

Then Add a new struct BitExtent for addressing all the deflate locations and modify code based on that.
 
Summary: Make puffin to use BitExtent instead of ByteExtent. (was: End Puffing and Huffing based on the onput size)
Description: Show this description
Blocking: 696813
Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/puffin/+/ec69c2b78e2359774678d084510c575cbc593471

commit ec69c2b78e2359774678d084510c575cbc593471
Author: Amin Hassani <ahassani@google.com>
Date: Tue Oct 10 00:39:07 2017

puffin: End puffing and huffing based on the input size.

End Puff and Huff operations based on the size of the input instead of looking
if the final block bit is set. This way we can break the deflate blocks into
smaller blocks that are on a byte alignment.

BUG= chromium:768505 
TEST=unittests pass.

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

[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/puff_reader.cc
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/bit_reader.h
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/puffin_unittest.cc
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/puffer.cc
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/bit_io_unittest.cc
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/puff_reader.h
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/unittest_common.h
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/bit_reader.cc
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/huffer.cc
[modify] https://crrev.com/ec69c2b78e2359774678d084510c575cbc593471/src/puff_io_unittest.cc

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/puffin/+/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7

commit 26bcfdd46ee56055b61b6e8f23f0ff57626a95a7
Author: Amin Hassani <ahassani@google.com>
Date: Tue Oct 17 20:02:53 2017

puffin: Use empty puff writer

There was a piece of code (Puffer.Puff) that was only being used in
puffdiff but it was also being shipped with puffpatch too. That code
figures out the size of the puff by puffing into a buffer, There was two
problem with that. First, the code shouldn't have been mixed with the
client code, and second, it needs retry of the Puffer.PuffDeflate to do
the puffing again if the buffer size was small. This patch fixes that
problem by removing that function and adding checks in the
Puffer.PuffDeflate to not check for boundary or write into the buffer if
the given buffer size was nullptr. This way we can create a PuffWriter
with null buffer that can count the number of bytes needed for a puff
buffer. Along with, all PuffDeflate and HuffDeflate functions based on
the byte array buffer where removed and equivalent functions based on
BitReader/BitWriter/PuffReader/PuffWriter were made public.

Additionally this CL, adds a new function for finding the location of
Puffs using the aforementioned functionality in the utils.cc (which will
only be in used in puffdiff).

Furthermore, this CL fixes a long undiscovered bug, for when the length
of the literals was exactly 127. The PuffWriter would write a wrong
value to the output and this would cause problem. The unittests for this
bug was also added. Since this bug was in PuffWriter.cc and most of the
changes are in PuffWriter.cc this was a good candidate for fixing this
bug.

BUG= chromium:768505 
TEST=unittests pass; brillo_update_payload {generate|verify} pass;

Change-Id: Ie5eb12dc637623b8f3ed08709590a323d9f830f8
Reviewed-on: https://chromium-review.googlesource.com/703680
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/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/puffin_unittest.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/utils_unittest.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/include/puffin/huffer.h
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/main.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/include/puffin/utils.h
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/puff_writer.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/include/puffin/puffer.h
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/unittest_common.h
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/puffer.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/puffpatch.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/bit_reader.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/huffer.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/puff_io_unittest.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/utils.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/puffin_stream.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/include/puffin/puffdiff.h
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/puffdiff.cc
[modify] https://crrev.com/26bcfdd46ee56055b61b6e8f23f0ff57626a95a7/src/sample_generator.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/puffin/+/7074da6508f1ab7cb860342e2434c5f5804bafbf

commit 7074da6508f1ab7cb860342e2434c5f5804bafbf
Author: Amin Hassani <ahassani@google.com>
Date: Fri Oct 20 23:00:07 2017

puffin: Introduce deflate bit addressing

This CL changes the addressing scheme of deflate blocks from Byte to
Bit. This CL allows breaking a given deflate stream into its
subblocks. A proper function is added to located the deflate subblocks
from a deflate stream. Proper unittests is added to check for new
functionality and its corner cases.

BUG= chromium:768505 
TEST=unittests pass; brillo_update_payload {generate|verify} pass;

Change-Id: I9e953d0b7cafca417c09ec56a49ea2e93f027304
Reviewed-on: https://chromium-review.googlesource.com/703681
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/7074da6508f1ab7cb860342e2434c5f5804bafbf/puffin.gyp
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puffin_stream.h
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puffin_unittest.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puffin_stream.cc
[add] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/patching_unittest.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/include/puffin/common.h
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/main.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/stream_unittest.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/include/puffin/puffer.h
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puffin.proto
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puffer.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puff_reader.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/huffer.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/sample_generator.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/utils_unittest.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/Makefile
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puffpatch.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puff_io_unittest.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/utils.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/include/puffin/puffdiff.h
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/include/puffin/utils.h
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puff_writer.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/unittest_common.h
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/bit_reader.cc
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/bit_reader.h
[modify] https://crrev.com/7074da6508f1ab7cb860342e2434c5f5804bafbf/src/puffdiff.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/puffin/+/8128bdfaf6b05150c105590cb7208fe03d32a6be

commit 8128bdfaf6b05150c105590cb7208fe03d32a6be
Author: Amin Hassani <ahassani@google.com>
Date: Thu Oct 26 22:57:44 2017

puffin: Faster PuffinStream seek

Currently we linearly look for the current puff in a puff stream,
linearly, this CL makes the Seek operation faster by using binary
search.

BUG= chromium:768505 
TEST=unittests pass; brillo_update_payload {generate|verify} pass;

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

[modify] https://crrev.com/8128bdfaf6b05150c105590cb7208fe03d32a6be/src/puffin_stream.h
[modify] https://crrev.com/8128bdfaf6b05150c105590cb7208fe03d32a6be/src/puffin_stream.cc

Blocking: 717785
Status: Fixed (was: Started)

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

Status: Archived (was: Fixed)

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

Status: Fixed (was: Archived)

Sign in to add a comment