New issue
Advanced search Search tips

Issue 779895 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Supersize creates inconsistent .size files across runs re. string literal.

Project Member Reported by huangs@google.com, Oct 31 2017

Issue description

OS: Ubuntu

What steps will reproduce the problem?
(1) Build chrome_public_apk, assuming location out/Release/apks/ChromePublic.apk
(2) Go to apks dir, use Supersize to create 2 .size files, e.g.:

cd out/Release/apks
../../../tools/binary_size/supersize archive 01.size --apk-file ChromePublic.apk -v
../../../tools/binary_size/supersize archive 02.size --apk-file ChromePublic.apk -v

(Note: 01.size and 02.size are zip files containing superficial differences due to time stamps -- but this bug goes beyond that).

(3) Use Supersize to generate diff, e.g.:
../../../tools/binary_size/supersize console 01.size 02.size
(inside console)
Print(Diff(), verbose=1)

What is the expected result?
Empty diff.

What happens instead?
Diff detected, mostly involving "string literal".

 

Comment 1 by huangs@google.com, Oct 31 2017

Note: I'm using an x86 Clank build with the following args.gn:

target_os = "android"
use_goma = true
is_debug = false
is_official_build = true
target_cpu = "x86"

This should not matter.  Sample Diff() output:

...
Showing 29 symbols (aliases not grouped for diffs) with total pss: 0 bytes
Histogram of symbols based on PSS:
    (-16,-8]: 2   (-4,-2]: 4   (-1,0): 12   (0,1): 2   [2,4): 1   [8,16): 1
     (-8,-4]: 2   (-2,-1]: 1      {0}: 1    [1,2): 2   [4,8): 1
.text=0 bytes    .rodata=0 bytes    .data.rel.ro=0 bytes    .data=0 bytes    .bss=0 bytes    total=0 bytes
Number of unique paths: 24

Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss
Index | Running Total | Section@Address | ...
------------------------------------------------------------
- 0)        -13 (57.9%) r@0x0        -13 (13->0)  num_aliases=2
               source_path=v8/src/interpreter/bytecode-decoder.cc       object_path=v8/v8_base/bytecode-decoder.o
               flags={}  name=string literal
+ 1)       -4.7 (21.0%) r@0x0        +8.3 (0->0)  num_aliases=1
               source_path=     object_path=
               flags={}  name=** aggregate padding of diff'ed symbols
                    full_name=
- 2)        -12 (56.6%) r@0x0        -8 (2->0)  num_aliases=2
               source_path=net/url_request/url_request_throttler_entry.cc       object_path=net/net/url_request_throttler_entry.o
               flags={}  name=string literal
~ 3)       -6.7 (29.9%) r@0x45cacdc  +6 (6->12)  num_aliases=1
               source_path=third_party/icu/source/i18n/dangical.cpp     object_path=third_party/icu/icui18n/dangical.o
               flags={}  name=string literal
~ 4)        -10 (47.7%) r@0x435e3da  -4 (15->11)  num_aliases=1
               source_path=net/quic/core/quic_crypto_client_handshaker.cc       object_path=net/net/quic_crypto_client_handshaker.o
               flags={}  name=string literal
...
Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a62dcc822dae872da6e4c4505b613bcddbf32df4

commit a62dcc822dae872da6e4c4505b613bcddbf32df4
Author: Andrew Grieve <agrieve@chromium.org>
Date: Tue Oct 31 17:12:27 2017

Supersize: Fix archive producing different results for the same input

A) Removes gzip header
B) Fixes sort key for string literals

Bug:  779895 
Change-Id: Ia1c9872c42c5564134215b0973d5d9e27dcf966d
Reviewed-on: https://chromium-review.googlesource.com/746377
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512854}
[modify] https://crrev.com/a62dcc822dae872da6e4c4505b613bcddbf32df4/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/a62dcc822dae872da6e4c4505b613bcddbf32df4/tools/binary_size/libsupersize/file_format.py

Status: Fixed (was: Untriaged)

Comment 4 by huangs@google.com, Nov 1 2017

Status: Available (was: Fixed)
It's better than before, but when I ran Supersize 20 times I still see some variation. As example, grouping by md5sum:

14508f7d2d8c8c9a697c1766d9ebba45  18.size
83c0d87c329c24d9f31d6ecb48df0d1a  16.size
a401530c3b08dc8a14b523c7f1d1c7b1  02.size
a401530c3b08dc8a14b523c7f1d1c7b1  08.size
a401530c3b08dc8a14b523c7f1d1c7b1  09.size
a401530c3b08dc8a14b523c7f1d1c7b1  15.size
bab0c2574097aff5bd667f15e6e33542  06.size
bdc470eb8566d3a86628c585a5b15ba9  01.size
bdc470eb8566d3a86628c585a5b15ba9  03.size
bdc470eb8566d3a86628c585a5b15ba9  04.size
bdc470eb8566d3a86628c585a5b15ba9  05.size
bdc470eb8566d3a86628c585a5b15ba9  07.size
bdc470eb8566d3a86628c585a5b15ba9  10.size
bdc470eb8566d3a86628c585a5b15ba9  11.size
bdc470eb8566d3a86628c585a5b15ba9  12.size
bdc470eb8566d3a86628c585a5b15ba9  13.size
bdc470eb8566d3a86628c585a5b15ba9  14.size
bdc470eb8566d3a86628c585a5b15ba9  17.size
bdc470eb8566d3a86628c585a5b15ba9  19.size
bdc470eb8566d3a86628c585a5b15ba9  20.size

So about 1-in-3 deviate from the majority. There should be more moles to whack?

Comment 5 by huangs@google.com, Nov 1 2017

Couldn't repro this on Zucchini yet. Meanwhile, quick way to repeat the experiment for ChromePublic.apk:

perl -e 'for $i (0..19) { printf("supersize archive --apk-file ChromePublic.apk %02d.size -v\n", $i); }' | sh

Aha! Easy fix. We just run archive 20 times, and take the majority output!

If you still have the .size files, could you diff them to see what's different?

Comment 7 by huangs@google.com, Nov 1 2017

Per discussion, most don't produce difference, and for the case where things are different, it looks like stuff in jump table.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba1cb82a81cb86d5de62356f612d9c210c7d61e5

commit ba1cb82a81cb86d5de62356f612d9c210c7d61e5
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed Nov 01 20:01:45 2017

Supersize: Fix archive producing different results for the same input (part 2)

Problem is the same, but didn't get the sort key exactly right.

This also adds a test for idempotency (but our test data is too trivial to catch
this bug).

Bug:  779895 
Change-Id: I3021ec2cf222c27b51cfaba1b90bd3b7ed1f912f
Reviewed-on: https://chromium-review.googlesource.com/748809
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513232}
[modify] https://crrev.com/ba1cb82a81cb86d5de62356f612d9c210c7d61e5/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/ba1cb82a81cb86d5de62356f612d9c210c7d61e5/tools/binary_size/libsupersize/integration_test.py
[modify] https://crrev.com/ba1cb82a81cb86d5de62356f612d9c210c7d61e5/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_readelf.py

Status: Fixed (was: Available)

Sign in to add a comment