New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 664481 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

2.1%-2.2% regression in sizes at 431519:431519

Project Member Reported by rmcilroy@chromium.org, Nov 11 2016

Issue description

Fady, the CL "Mus: Move InProcessCommandBuffer and GLInProcessContext" to gpu/ipc" seems to have increased the dll size significanlty. Could you take a look please?
 
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 11 2016

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

commit 03f92f188350dd41ffb806b959b885d49a63f639
Author: fsamuel <fsamuel@chromium.org>
Date: Fri Nov 11 14:46:35 2016

Fix size regression: Make non-public deps

Mus: Move InProcessCommandBuffer and GLInProcessContext to gpu/ipc caused a
DLL size regression. I'm guessing that's because I made the deps public_deps on the
new targets. I've changed them back to deps. Hopefully that addresses the issue.

BUG= 664481 
TBR=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2492713006
Cr-Commit-Position: refs/heads/master@{#431560}

[modify] https://crrev.com/03f92f188350dd41ffb806b959b885d49a63f639/gpu/ipc/BUILD.gn

Status: Fixed (was: Assigned)
Status: Assigned (was: Fixed)
Doesn't look like this was actually fixed; sizes don't come back down: https://chromeperf.appspot.com/report?sid=360ede0bdbe363098ae928b46eed395c28e5fa687ad0cde6183fac88eafbf237&start_rev=431500&end_rev=431800
Cc: benhenry@chromium.org grt@chromium.org dcheng@chromium.org sullivan@chromium.org fsam...@chromium.org brucedaw...@chromium.org
 Issue 668287  has been merged into this issue.
Copying some relevant hints from the duplicate bug:

Comparing the before/after canary binaries shows that most of the size increase is from the .text segment, meaning new code:

Size of 56.0.2916.0\chrome.dll is 41.041512 MB
      name:   mem size  ,  disk size
     .text: 33.090450 MB
    .rdata:  6.048750 MB
     .data:  1.219108 MB,  0.248320 MB
   .rodata:  0.003808 MB
    .gfids:  0.001052 MB
      .tls:  0.000025 MB
    _RDATA:  0.000288 MB
  CPADinfo:  0.000036 MB
     .rsrc:  0.175144 MB
    .reloc:  1.457552 MB

Size of 56.0.2918.0\chrome.dll is 41.920104 MB
      name:   mem size  ,  disk size
     .text: 33.764130 MB
    .rdata:  6.209102 MB
     .data:  1.229928 MB,  0.258560 MB
   .rodata:  0.003808 MB
    .gfids:  0.001052 MB
      .tls:  0.000025 MB
    _RDATA:  0.000288 MB
  CPADinfo:  0.000036 MB
     .rsrc:  0.175144 MB
    .reloc:  1.490956 MB

Change from 56.0.2916.0\chrome.dll to 56.0.2918.0\chrome.dll
     .text: 673680 bytes change
    .rdata: 160352 bytes change
     .data: 10820 bytes change
    .reloc: 33404 bytes change
Total change: 878256 bytes

Comparing the global variable symbols between chrome.dll in 2916 and 2918 showed some non-trivial differences that, together with code size changes, could explain the regression - the yy_ec entry is a one-byte variable that is replicated 256 times. The others are just large globals:

Symbols that are in 2918.txt but not in 2916.txt
1       256     0       0       yy_ec
0       0        15718  2       gpu::gles2::ApplyFramebufferAttachmentCMAAINTELResourceManager::cmaa_frag_s2_
0       0        13246  2       gpu::gles2::ApplyFramebufferAttachmentCMAAINTELResourceManager::cmaa_frag_s1_
0       0         8192  2       gl::g_mantissa
0       0         5526  2       yycheck
0       0         5526  2       yytable
0       0         5072  3       gpu::gles2::GLES2DecoderImpl::command_info
0       0         5072  3       gpu::gles2::GLES2DecoderPassthroughImpl::command_info
0       0         2180  2       yy_chk
0       0         2180  2       yy_nxt
0       0         1650  2       yy_base
0       0         1650  2       yy_def
0       0         1640  2       yy_accept
0       0          964  2       yy_rule_can_match_eol
0       0          838  2       yydefact
0       0          838  2       yypact

When extra global variables are pulled in this increases .rdata and/or .data segments, and often associated code is also pulled in. Dead-code/dead-data discarding is, unfortunately, imperfect. I have found that tracking global variables is often easier than tracking code that is pulled in.

Cc: -grt@chromium.org mmoss@chromium.org
Adding mmoss as this is in regards to official builders.
I have a possible fix that is running on the try bots. It works on non-official release builds and will *probably* work on official release builds.

crrev.com/2556603002
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 6 2016

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

commit 255638d8476e0b6f7a5f8d60a707e10bfa5142d8
Author: brucedawson <brucedawson@chromium.org>
Date: Tue Dec 06 21:23:38 2016

Change gpu/ipc:command_buffer_sources to static_library

A 900 KB size regression popped up at R431519. This CL
fixes it by changing a source_set to a static_library. The effect is to
reduce the size of chrome.dll significantly, fixing the regression:

chrome.dll
     .text: -711776 bytes change
    .rdata: -160752 bytes change
     .data: -10848 bytes change
    .reloc: -33736 bytes change
Total change: -917112 bytes

A custom version of dia2dump.exe was used to find global variables that
appeared in chrome.dll when the regression happened. One of the globals
was cmaa_frag_s2_. This was declared in gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc
so a verbose link of chrome.dll was done and a script (linker_verbose_tracking.py)
was used to find out what was causing this .obj file to be pulled in.
It pointed to in_process_command_buffer.obj, and that led to the fix.

The fix was tested on a release build, but not an official build, so it
is possible that the savings will not apply to official builds. This
will be monitored.

BUG= 664481 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2556603002
Cr-Commit-Position: refs/heads/master@{#436731}

[modify] https://crrev.com/255638d8476e0b6f7a5f8d60a707e10bfa5142d8/gpu/ipc/BUILD.gn

Owner: brucedaw...@chromium.org
Status: Fixed (was: Assigned)
I verified an approximately 900 KB savings on the 32-bit canary build, version 57.0.2944.0. The sizes graphs also show the drop, for both 32-bit and 64-bit Chrome at R436731. Closing as fixed.

Sign in to add a comment