New issue
Advanced search Search tips

Issue 834339 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

43.9 KiB regression in resource_sizes (MonochromePublic.apk) at 551586:551586

Project Member Reported by huangs@google.com, Apr 18 2018

Issue description

Caused by “[builtins] Re-enable embedded builtins”

Chromium Commit: abc123abc123abc123abc123abc123abc123abcd

Review: https://chromium-review.googlesource.com/#/c/1013570/

Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=480214

Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase

Based on the graph: 43.9 KiB native code.

It looks like this increase was probably unexpected or might be avoidable.
Please have a look and either:

Close as “Won't Fix” with a short justification, or
Land a revert / fix-up.



 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Apr 18 2018

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=834339

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=0547188f589d1316fd9adee4b028eab33f188846996483639bfab8c9e5783dfe


Bot(s) for this bug's original alert(s):

Android Builder Perf
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Apr 18 2018

Owner: v8-autoroll@chromium.org
Status: Assigned (was: Untriaged)
Assigning to v8-autoroll@chromium.org because this is the only CL in range:
Update V8 to version 6.8.34.

Summary of changes available at:
https://chromium.googlesource.com/v8/v8/+log/059b1287..bde5be65

Please follow these instructions for assigning/CC'ing issues:
https://github.com/v8/v8/wiki/Triaging%20issues

Please close rolling in case of a roll revert:
https://v8-roll.appspot.com/
This only works with a Google account.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel

TBR=hablich@chromium.org,machenbach@chromium.org,kozyatinskiy@chromium.org,sergiyb@chromium.org

Change-Id: Iae7c5b0f4356f1f808e55c7ea16f912ca0ce88e1
Reviewed-on: https://chromium-review.googlesource.com/1016044
Reviewed-by: v8 autoroll <v8-autoroll@chromium.org>
Commit-Queue: v8 autoroll <v8-autoroll@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551586}

Comment 3 by huangs@google.com, Apr 18 2018

Owner: jgruber@chromium.org
Justification for picking the CL: When I run:

tools/binary_size/diagnose_bloat.py --cloud bf455d0506522c0fc24b9aa5b4b267433fd0d114

I get:

*******
Common Metadata:
    apk_file_name=apks/MonochromePublic.apk
    elf_arch=arm
    elf_file_name=lib.unstripped/libmonochrome.so
    gn_args=ffmpeg_branding="Chrome" goma_dir="/b/build/slave/cache/goma_client" is_chrome_branded=true is_debug=false 

...

Section Sizes (Total=44.0kb (45063 bytes)):
    .bss: 0 bytes (0 bytes) (not included in totals)
    .data: 0 bytes (0 bytes) (0.0%)
    .data.rel.ro: 0 bytes (0 bytes) (0.0%)
    .dex: 0 bytes (0 bytes) (0.0%)
    .other: -263kb (-269377 bytes) (-597.8%)
    .pak.nontranslated: 0 bytes (0 bytes) (0.0%)
    .pak.translations: 0 bytes (0 bytes) (0.0%)
    .rel.dyn: 4 bytes (4 bytes) (0.0%)
    .rodata: 256 bytes (256 bytes) (0.6%)
    .text: 306kb (314176 bytes) (697.2%)

76 symbols added (+), 230 changed (~), 12 removed (-), 956604 unchanged (not shown)
Of changed symbols, 169 grew, 149 shrank
Number of unique symbols 603939 -> 603989 (+50)
2 paths added, 0 removed, 41 changed

Showing 318 symbols (231 -> 291 unique) with total pss: 26278 bytes
Histogram of symbols based on PSS:
    (-524288,-262144]: 1   (-128,-64]: 6    (-4,-2]: 3     [2,4): 29      [64,128): 15       [2048,4096): 1
      (-32768,-16384]: 1    (-64,-32]: 7    (-2,-1]: 2     [4,8): 26     [128,256): 3    [262144,524288): 1
         (-1024,-512]: 3    (-32,-16]: 80    (-1,0): 4    [8,16): 27     [256,512): 3
          (-512,-256]: 6     (-16,-8]: 19     (0,1): 2   [16,32): 26    [512,1024): 2
          (-256,-128]: 2      (-8,-4]: 15     [1,2): 4   [32,64): 28   [1024,2048): 2
.text=306kb      .rodata=-18.1kb    .data.rel.ro=0 bytes    .data=0 bytes    .bss=0 bytes    .dex=0 bytes    .dex.method=0 bytes    .pak.translations=0 bytes    .pak.nontranslated=0 bytes    .other=-263kb     total=25.7kb
Number of unique paths: 48

Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss, x=.dex, m=.dex.method, p=.pak.translations, P=.pak.nontranslated, o=.other
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
+ 0)     311316 (1177.7%) t@0x169cb00  311316 (0->311296) $root_gen_dir/v8/embedded.cc
~ 1)      39428 (149.1%) o@0x0        -271888 (1436976->1165088) v8/snapshot_blob_32.bin
+ 2)      20634 (78.1%) r@0x0        -18794 (0->0)      {no path}
               ** aggregate padding of diff'ed symbols
~ 3)      23418 (88.6%) t@0x1865c24  2784 (27216->30000) {no path}
               ** lld merge strings
+ 4)      24900 (94.2%) t@0x134db00  1482 (0->1480)     v8/src/builtins/builtins.cc
               v8::internal::Builtins::IsIsolateIndependent

...

~ 27)     25485 (96.4%) t@0x135caa8  116 (916->1032)    v8/src/compiler.cc


******************************Resource Sizes Diff******************************
MonochromePublic.apk_Breakdown (+45,063 bytes)
    +1,410 bytes Zip Overhead
  -271,888 bytes V8 Snapshots size
      +142 bytes unwind_cfi (dev and canary only) size
  +315,396 bytes Native code size
        +3 bytes Package metadata size
MonochromePublic.apk_Specifics
   +44,921 bytes normalized apk size
  +315,396 bytes main lib size

I  78732 See detailed diff results here: out/binary-size-results/diffs/f3150125ff552dc17f7706a76fc39e5c90c27347..bf455d0506522c0fc24b9aa5b4b267433fd0d114/diff_results.txt

Diff Summary
    +44921 bytes MonochromePublic.apk_Specifics normalized apk size for range: f3150125ff552dc17f7706a76fc39e5c90c27347..bf455d0506522c0fc24b9aa5b4b267433fd0d114
I  78733 Enter supersize console via: tools/binary_size/supersize console out/binary-size-results/f3150125ff552dc17f7706a76fc39e5c90c27347/MonochromePublic.apk.size out/binary-size-results/bf455d0506522c0fc24b9aa5b4b267433fd0d114/MonochromePublic.apk.size

*******

So items (1) and (2) are big additions followed by big reduction.  (1) is due to embedded.cc, which matches description of https://chromium-review.googlesource.com/c/v8/v8/+/1013570 .  A more thorough scan can be obtained from

tools/binary_size/diagnose_bloat.py bde5be65 --reference-rev c7e6cf7^ --subrepo v8 --all -v
 
but I cannot build chrome_public_pak for some reason, and this would be time consuming.
Status: WontFix (was: Assigned)
I'm confused what items you mean by '(1)' and '(2)'. 

The CL is expected to significantly increase binary size (due to embedded.cc) and significantly decrease the snapshot size. There's no 1-to-1 correlation since the serialized format of the snapshot differs from the raw binary.

Closing as WontFix, but please feel free to reopen in case I missed something that seems unexpected.

Sign in to add a comment