Issue metadata
Sign in to add a comment
|
2.1%-2.2% regression in sizes at 431519:431519 |
||||||||||||||||||||
Issue descriptionFady, the CL "Mus: Move InProcessCommandBuffer and GLInProcessContext" to gpu/ipc" seems to have increased the dll size significanlty. Could you take a look please?
,
Nov 11 2016
Possible fix: https://codereview.chromium.org/2492713006/#
,
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
,
Nov 12 2016
,
Dec 5 2016
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
,
Dec 5 2016
Issue 668287 has been merged into this issue.
,
Dec 5 2016
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.
,
Dec 6 2016
Adding mmoss as this is in regards to official builders.
,
Dec 6 2016
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
,
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
,
Dec 7 2016
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 |
|||||||||||||||||||||
Comment 1 by rmcilroy@chromium.org
, Nov 11 2016