New issue
Advanced search Search tips

Issue 785277 link

Starred by 1 user
Status: WontFix
Owner:
agrieve@chromium.org
Closed: Nov 2017
Cc:
shend@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

36kb regression in resource_sizes (MonochromePublic.apk) at 516529:516529

Project Member Reported by agrieve@chromium.org, Nov 15 2017 
Grouping two alerts together. 
First was 16kb:
From: "[css-typed-om] Implement addition of numeric types. "
Commit: 58986c160a619dc9117e45966047c7812c7c8888
Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=516529

Second was: 20kb:
From: "[css-typed-om] Implement arithmetic operations on CSSNumericValue."
Commit: 83929013818468e61fc4c144739e73119cc145fe
Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=516677

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


It's not clear to me whether or not this much increase was expected.
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, Nov 15 2017 

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

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


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

Android Builder

Comment 2 by agrieve@chromium.org, Nov 15 2017 

Relevant output from tools/binary_size/diagnose_bloat.py --cloud 58986c160a619dc9117e45966047c7812c7c8888

Section Sizes (Total=16.7kb (17111 bytes)):
    .bss: -32 bytes (-32 bytes) (not included in totals)
    .data: 0 bytes (0 bytes) (0.0%)
    .data.rel.ro: 16 bytes (16 bytes) (0.1%)
    .rel.dyn: 16.1kb (16440 bytes) (96.1%)
    .rodata: 0 bytes (0 bytes) (0.0%)
    .text: 639 bytes (639 bytes) (3.7%)

Comment 3 by agrieve@chromium.org, Nov 15 2017 

Relevant output from: tools/binary_size/diagnose_bloat.py --cloud 83929013818468e61fc4c144739e73119cc145fe


Section Sizes (Total=21.2kb (21736 bytes)):
    .bss: 0 bytes (0 bytes) (not included in totals)
    .data: 0 bytes (0 bytes) (0.0%)
    .data.rel.ro: -32 bytes (-32 bytes) (-0.1%)
    .rel.dyn: 19.9kb (20392 bytes) (93.8%)
    .rodata: 0 bytes (0 bytes) (0.0%)
    .text: 1372 bytes (1372 bytes) (6.3%)

Comment 4 by agrieve@chromium.org, Nov 15 2017

Cc: -agrieve@chromium.org shend@chromium.org
Owner: agrieve@chromium.org
Status: WontFix (was: Assigned)
Hmm super weird! I haven't seen a change before that had it's increased cause by more relocations.

Spot checked the second commit with --verbose, and see:
rel.dyn (unpacked): -88 bytes (-88 bytes) (not included in totals)

So, in fact, the change actually *removed* some relocations, which means the regression is from relocation_packer doing a bad job.

There are already two separate efforts for improving packed relocations (main one is by switching to lld, which does a better job). So I don't think there's any action required here.
►

Sign in to add a comment