Issue metadata
Sign in to add a comment
|
0.7%-0.8% regression in sizes at 480386:480386 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jun 19 2017
sakal@ could you please take a look? https://codereview.chromium.org/2944823002 has increased chrome.dll size. Is that expected?
,
Jun 20 2017
,
Jun 20 2017
This is caused by: https://codereview.webrtc.org/2854123003 Confirmed by rolling just that CL here: https://codereview.chromium.org/2944273002 Zhihuang, can you please fix this ASAP or we may have to revert the change?
,
Jun 20 2017
I'll take a look.
,
Jun 20 2017
+brucedawson: do you have some handy tooling to find out why the culprit CL adds 600K to chrome.dll?
,
Jun 20 2017
I would build before/after binaries with similar or identical settings (must have full debug information and no fastlink) and then use SymbolSort as described here: https://www.chromium.org/developers/windows-binary-sizes > SymbolSort -in new\chrome.dll.pdb -diff old\chrome.dll.pdb -out change.txt
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/9ed16097375fb8d9b45623c58d9086d33e503760 commit 9ed16097375fb8d9b45623c58d9086d33e503760 Author: zhihuang <zhihuang@webrtc.org> Date: Wed Jun 21 06:58:36 2017 Try to fix the binary size increase issue on Chromium. The target common_video used to depend on rtc_media_base which introduces the dependency on p2p. This probably causes the binary size increase on Win Chromium. Some chromium targets like src/media/gpu:gpu depends on common_video directly. BUG= chromium:734631 Review-Url: https://codereview.webrtc.org/2945233002 Cr-Commit-Position: refs/heads/master@{#18693} [modify] https://crrev.com/9ed16097375fb8d9b45623c58d9086d33e503760/webrtc/common_video/BUILD.gn [modify] https://crrev.com/9ed16097375fb8d9b45623c58d9086d33e503760/webrtc/media/BUILD.gn [modify] https://crrev.com/9ed16097375fb8d9b45623c58d9086d33e503760/webrtc/pc/BUILD.gn
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/c2e208a9249452590fa282ef5aba43e480bc5794 commit c2e208a9249452590fa282ef5aba43e480bc5794 Author: zhihuang <zhihuang@webrtc.org> Date: Wed Jun 21 07:30:49 2017 Revert of Try to fix the binary size increase issue on Chromium. (patchset #1 id:1 of https://codereview.webrtc.org/2945233002/ ) Reason for revert: The Android 32 (more config) bot is broken. Original issue's description: > Try to fix the binary size increase issue on Chromium. > > The target common_video used to depend on rtc_media_base which introduces > the dependency on p2p. This probably causes the binary size increase on Win > Chromium. Some chromium targets like src/media/gpu:gpu depends on common_video directly. > > BUG= chromium:734631 > > Review-Url: https://codereview.webrtc.org/2945233002 > Cr-Commit-Position: refs/heads/master@{#18693} > Committed: https://chromium.googlesource.com/external/webrtc/+/9ed16097375fb8d9b45623c58d9086d33e503760 TBR=kjellander@webrtc.org,deadbeef@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= chromium:734631 Review-Url: https://codereview.webrtc.org/2949953003 Cr-Commit-Position: refs/heads/master@{#18694} [modify] https://crrev.com/c2e208a9249452590fa282ef5aba43e480bc5794/webrtc/common_video/BUILD.gn [modify] https://crrev.com/c2e208a9249452590fa282ef5aba43e480bc5794/webrtc/media/BUILD.gn [modify] https://crrev.com/c2e208a9249452590fa282ef5aba43e480bc5794/webrtc/pc/BUILD.gn
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/130ca7e783d123ed04758056b5fceb3dbbf5c1fa commit 130ca7e783d123ed04758056b5fceb3dbbf5c1fa Author: zhihuang <zhihuang@webrtc.org> Date: Wed Jun 21 08:02:59 2017 Reland of Try to fix the binary size increase issue on Chromium. (patchset #1 id:1 of https://codereview.webrtc.org/2949953003/ ) Reason for revert: Relanding the orginal CL. The breakage would be a flakey build. Original issue's description: > Revert of Try to fix the binary size increase issue on Chromium. (patchset #1 id:1 of https://codereview.webrtc.org/2945233002/ ) > > Reason for revert: > The Android 32 (more config) bot is broken. > > Original issue's description: > > Try to fix the binary size increase issue on Chromium. > > > > The target common_video used to depend on rtc_media_base which introduces > > the dependency on p2p. This probably causes the binary size increase on Win > > Chromium. Some chromium targets like src/media/gpu:gpu depends on common_video directly. > > > > BUG= chromium:734631 > > > > Review-Url: https://codereview.webrtc.org/2945233002 > > Cr-Commit-Position: refs/heads/master@{#18693} > > Committed: https://chromium.googlesource.com/external/webrtc/+/9ed16097375fb8d9b45623c58d9086d33e503760 > > TBR=kjellander@webrtc.org,deadbeef@webrtc.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= chromium:734631 > > Review-Url: https://codereview.webrtc.org/2949953003 > Cr-Commit-Position: refs/heads/master@{#18694} > Committed: https://chromium.googlesource.com/external/webrtc/+/c2e208a9249452590fa282ef5aba43e480bc5794 TBR=kjellander@webrtc.org,deadbeef@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= chromium:734631 Review-Url: https://codereview.webrtc.org/2949883003 Cr-Commit-Position: refs/heads/master@{#18695} [modify] https://crrev.com/130ca7e783d123ed04758056b5fceb3dbbf5c1fa/webrtc/common_video/BUILD.gn [modify] https://crrev.com/130ca7e783d123ed04758056b5fceb3dbbf5c1fa/webrtc/media/BUILD.gn [modify] https://crrev.com/130ca7e783d123ed04758056b5fceb3dbbf5c1fa/webrtc/pc/BUILD.gn
,
Jun 21 2017
Here's a diff from SymbolSort over an official build. I hope this helps.
,
Jun 21 2017
The WebRTC change is being submitted shortly in https://codereview.chromium.org/2950933004/ sorry it took so long. Hopefully the sizes graph will recover.
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02493a9858bd5505aa82f4d4491278c909a15282 commit 02493a9858bd5505aa82f4d4491278c909a15282 Author: sakal <sakal@chromium.org> Date: Wed Jun 21 13:11:30 2017 Roll WebRTC 18665:18695 (17 commits) Changes: https://chromium.googlesource.com/external/webrtc/trunk/webrtc.git/+log/e6fddec..0f3c15e $ git log e6fddec..0f3c15e --date=short --no-merges --format=%ad %ae %s 2017-06-21 zhihuang@webrtc.org Reland of Try to fix the binary size increase issue on Chromium. (patchset #1 id:1 of https://codereview.webrtc.org/2949953003/ ) 2017-06-21 zhihuang@webrtc.org Revert of Try to fix the binary size increase issue on Chromium. (patchset #1 id:1 of https://codereview.webrtc.org/2945233002/ ) 2017-06-20 zhihuang@webrtc.org Try to fix the binary size increase issue on Chromium. 2017-06-20 tschumim@webrtc.org Test and fix for huge bwe drop after alr state. 2017-06-20 andersc@webrtc.org Support more formats in RTCVideoFrame 2017-06-20 mellem@webrtc.org Create VideoDecoderFactory interface and implementation. 2017-06-20 mellem@webrtc.org Create a hardware VideoDecoder implementation using Android MediaCodec. 2017-06-20 brucedawson@chromium.org Use constexpr to avoid a static initializer 2017-06-20 henrik.lundin@webrtc.org Improve the simulation stats aggregation in neteq_rtpplay 2017-06-20 ilnik@webrtc.org Hotfix for psnr regresion with fec tests caused by timing frames. 2017-06-20 henrik.lundin@webrtc.org Add Matlab plotting script generator to neteq_rtpplay 2017-06-20 aleloi@webrtc.org Added new AudioProcessing fuzzer 2017-06-20 henrik.lundin@webrtc.org Add henrik.lundin to fuzzers OWNERS 2017-06-20 tommi@webrtc.org Minor updates to VideoReceiveStream: * Change decoder thread to use new thread function type. * Reduce the time of when video_receiver_ receives callbacks on the process thread to match with Start/Stop of the decoder. * Not triggering shutdown unless the thread is running. 2017-06-19 denicija@google.com Bugfix:setting capture framerate always defaults to 30fps. 2017-06-19 emadomara@google.com Fix test break by the recent changes in IcerServer 2017-06-19 zijiehe@chromium.org Add reference counter of DxgiDuplicatorController to unload DXGI components TBR= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng BUG= 734631 Review-Url: https://codereview.chromium.org/2950933004 Cr-Commit-Position: refs/heads/master@{#481188} [modify] https://crrev.com/02493a9858bd5505aa82f4d4491278c909a15282/DEPS
,
Jun 21 2017
Some data, steps to follow, and a fix, all from a release 32-bit build:
Ths script shows that the increase is mostly in the .text (code) segment:
C:\src\chromium3\src>python tools\win\pe_summarize.py without_patch\chrome.dll with_patch\chrome.dll
Size of without_patch\chrome.dll is 35.828736 MB
name: mem size , disk size
.text: 28.394326 MB
.rdata: 6.091540 MB
.data: 0.835236 MB, 0.097792 MB
.tls: 0.000029 MB
.rodata: 0.005520 MB
.gfids: 0.003324 MB
_RDATA: 0.000288 MB
CPADinfo: 0.000036 MB
.rsrc: 0.130504 MB
.reloc: 1.102024 MB
Size of with_patch\chrome.dll is 36.129792 MB
name: mem size , disk size
.text: 28.619254 MB
.rdata: 6.161412 MB
.data: 0.836228 MB, 0.098304 MB
.tls: 0.000029 MB
.rodata: 0.005520 MB
.gfids: 0.003324 MB
_RDATA: 0.000288 MB
CPADinfo: 0.000036 MB
.rsrc: 0.130504 MB
.reloc: 1.107848 MB
Memory size change from without_patch\chrome.dll to with_patch\chrome.dll
.text: 224928 bytes change
.rdata: 69872 bytes change
.data: 992 bytes change
.reloc: 5824 bytes change
Total change: 301616 bytes
SymbolSort (SymbolSort.exe -in with_patch\chrome.dll.pdb -diff without_patch\chrome.dll.pdb -out change.txt) says that these are the biggest causes of increases in chrome.dll:
Increases in Total Size
Total Size Total Count Name
7872 1 _opus_encode_native
7200 1 mdct_twiddles960
6744 1 _celt_encode_with_ec
5609 1 WebRtcIsac_Fftradix
5182 1 tonality_analysis
5088 1 CELT_PVQ_U_DATA
4624 1 WebRtcIsac_kQKltLevelsShape
3840 1 fft_twiddles48000_960
3555 1 _WebRtcIsac_InitializePitch
3237 1 _silk_pitch_analysis_core_FLP
3172 1 _silk_Encode
3136 1 WebRtcIsac_kQKltLevelsGain
2592 1 WebRtcIsac_kKltT1Shape
_opus_encode_native is defined in opus_encoder.c so I did a /verbose link of chrome.dll and then analyze the output with "python tools\win\linker_verbose_tracking.py verbose.txt opus_encoder" which gave these results:
Database loaded - 10009 xrefs found
opus_encoder.obj pulled in for symbol "_opus_encoder_create" by
Command-line obj file: opus_interface.obj
So, opus_interface.obj is specified on the command-line (presumably in a source_set) and it referenced _opus_encoder_create which pulled in opus_encoder. Ah, source_sets.
Ideally opus_encoder wouldn't be pulled in to the chrome.dll build so I ran gn path to find out why it is:
C:\src\chromium3\src>gn path out\release //chrome:main_dll //third_party/webrtc/modules/audio_coding:webrtc_opus_c
//chrome:main_dll --[private]-->
//headless:headless_shell_browser_lib --[private]-->
//content/public/browser:browser --[public]-->
//content/public/browser:browser_sources --[private]-->
//content/browser:browser --[private]-->
//third_party/webrtc/media:rtc_media_base --[private]-->
//third_party/webrtc/api/audio_codecs:builtin_audio_encoder_factory --[private]-->
//third_party/webrtc/modules/audio_coding:builtin_audio_encoder_factory_internal --[private]-->
//third_party/webrtc/modules/audio_coding:webrtc_opus --[public]-->
//third_party/webrtc/modules/audio_coding:webrtc_opus_c
Fixing that is what we really want to do, but that path has been there for a while. So, a more practical fix for the moment is to change a couple of source sets to static_libraries in order to break the chain. This change was found to fix the regression on my local builds:
C:\src\chromium3\src\third_party\webrtc>git diff
diff --git a/media/BUILD.gn b/media/BUILD.gn
index 249ceb8..90fe508 100644
--- a/media/BUILD.gn
+++ b/media/BUILD.gn
@@ -32,7 +32,7 @@ config("rtc_media_warnings_config") {
}
}
-rtc_source_set("rtc_media_base") {
+rtc_static_library("rtc_media_base") {
# TODO(kjellander): Remove (bugs.webrtc.org/6828)
# Enabling GN check triggers cyclic dependency error:
# //webrtc/media:rtc_media_base ->
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 2720a1e..395544a 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -184,7 +184,7 @@ rtc_static_library("peerconnection") {
# need should use CreateModularCreatePeerConnectionFactory instead, using the
# "peerconnection" build target and other targets specific to their
# requrements. See comment in peerconnectionfactoryinterface.h.
-rtc_source_set("create_pc_factory") {
+rtc_static_library("create_pc_factory") {
sources = [
"createpeerconnectionfactory.cc",
]
,
Jun 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/ab97e18fa97a73d8ad0ac51c11d7111dd397984d commit ab97e18fa97a73d8ad0ac51c11d7111dd397984d Author: zhihuang <zhihuang@webrtc.org> Date: Thu Jun 22 08:28:59 2017 Fix the binary size regression on Chromium Windows. There is a dependency chain from Chromium windows main_dll to Opus which should never exist. We used to rely on rtc_static_library to break this chain. So this CL replaced some rtc_source_set with rtc_static_library. libvpx fix (https://chromium-review.googlesource.com/c/544107/) for ios-simulator linking issue is landed and this CL can be sumbitted once the new Chromium is rolled into WebRTC. BUG= chromium:734631 Review-Url: https://codereview.webrtc.org/2947273002 Cr-Commit-Position: refs/heads/master@{#18709} [modify] https://crrev.com/ab97e18fa97a73d8ad0ac51c11d7111dd397984d/webrtc/media/BUILD.gn [modify] https://crrev.com/ab97e18fa97a73d8ad0ac51c11d7111dd397984d/webrtc/pc/BUILD.gn
,
Jun 22 2017
The graph is recovered now. https://chromeperf.appspot.com/report?sid=7966402fb414206f84dfdfd6e53206a35e5523c740f3ecad3cb1e248d02cf60b
,
Jul 6 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by chiniforooshan@chromium.org
, Jun 19 2017