New issue
Advanced search Search tips

Issue 716389 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

0.2% regression in sizes at 467561:467561

Project Member Reported by rmcilroy@chromium.org, Apr 28 2017

Issue description

shend@ the CL "Generate StyleBackgroundData in ComputedStyleBase"  regressed linux binary size by 214KB. Could you take a look please.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=716389

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgksWBlwgM


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

linux

Comment 2 by shend@chromium.org, May 1 2017

Cc: agrieve@chromium.org
Could not repro on local Linux machine.

Ran //tools/binary_size/supersize diff with revert patch [1] and ToT. Output was:

Showing 6,516 symbols with total pss: 6816 bytes
.text=1616 bytes .rodata=3.75kb     other=1363 bytes total=6.66kb
Number of object files: 3

Used build instructions from [2].

agrieve, when I run supersize diff, it comes up with a lot of random symbols unrelated to my change. Is this expected, or am I doing something wrong? Can you tell me the best way to repro this issue? Thanks!

[1] https://codereview.chromium.org/2849123002
[2] https://cs.chromium.org/chromium/src/tools/binary_size/README.md
Did it show a more significant size difference when printing out the per-section stats?

Hard to say if you're doing something wrong without seeing exactly what you ran, but the main thing the people often get wrong is not using the same GN args as the bots. 

There is still some noise with diffs (and more on linux than with android). The noise is mainly comprised of compiler & linker generated symbols (e.g. .L__foo)

Comment 4 by shend@chromium.org, May 2 2017

Hi agrieve, thanks for the reply. The full header of the output is below. The per section sizes seem fine as well? I'm just going to revert this patch to see if the graph goes down.

Common Metadata:
    elf_arch=Advanced
    elf_file_name=chrome
    gn_args=dcheck_always_on=true exclude_unwind_tables=true ffmpeg_branding="Chrome" is_debug=false is_official_build=true proprietary_codecs=true symbol_level=1 use_goma=true
    map_file_name=chrome.map.gz
Old Metadata:
    elf_build_id=0fd5a5a13e1e9f88db9fdcccdf4c5b7e39a714dc
    elf_mtime=2017-05-01 16:16:35
    git_revision=dfbc564dff1166bd964f585c4aac9c55f044d3ad
New Metadata:
    elf_build_id=79c5ede5934887f3e72eeebc88c126153f54454c
    elf_mtime=2017-05-01 16:49:38
    git_revision=b96dfa5b9c0656cc9a8dc0e67f1ab3b547c45a84

Section Sizes (Total=448 bytes):
    .bss: -32 bytes (not included in totals)
    .data: 0 bytes (0.0%)
    .data.rel.ro: 64 bytes (14.3%)
    .rodata: 0 bytes (0.0%)
    .text: 384 bytes (85.7%)

2898 symbols added (+), 717 changed (~), 2901 removed (-), 378843 unchanged (not shown)
0 object files added, 1 removed

Showing 6,516 symbols with total pss: 6816 bytes
.text=1616 bytes .rodata=3.75kb     other=1363 bytes total=6.66kb
Number of object files: 3

Hmm, yeah, the per-section sizes are just the result of running readelf -S, so there's really no room for error there. There was also not associated jump on the android size graph (which is generally more sensitive), so it wouldn't surprise me to find out that the graph was just wrong in this case (although I don't know why that would be)

Comment 6 by shend@chromium.org, May 3 2017

Status: Fixed (was: Assigned)
Reverted the patch, graph has gone down. Will try to reland patch in smaller pieces to see what exactly is causing the issue.
Labels: -binary-size Performance-Size

Sign in to add a comment