chrome.exe size regression |
|||||
Issue descriptionchrome.exe grew by 150% on August 11 in 54.0.2827.0. The growth is visible here: https://chromeperf.appspot.com/report?sid=582173b76c57270bf0a173be5498609344065d382cc9f0fe7f5314d058bdb827&start_rev=410570&end_rev=411887
,
Sep 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1504ef7199454f8096cf1ecdece90a8476aa4a31 commit 1504ef7199454f8096cf1ecdece90a8476aa4a31 Author: grt <grt@chromium.org> Date: Thu Sep 15 21:34:53 2016 chrome_initial does not need //ui/gfx on Windows. chrome.exe appears to have grown by 150% when skia rolled in r411308. chrome.exe shouldn't need skia or gfx, so let's try not depending on them! BUG= 647223 R=brucedawson@chromium.org Review-Url: https://codereview.chromium.org/2345503006 Cr-Commit-Position: refs/heads/master@{#418986} [modify] https://crrev.com/1504ef7199454f8096cf1ecdece90a8476aa4a31/chrome/BUILD.gn
,
Sep 15 2016
I double checked and I am seeing that if is_chrome_branded = false then the regression remains: C:\src\chromium3\src>type out\release\args.gn is_chrome_branded = true is_component_build = false is_debug = false target_cpu = "x86" C:\src\chromium3\src>dir out\release\chrome.exe 09/15/2016 02:17 PM 921,600 chrome.exe C:\src\chromium3\src>type out\release_no_branding\args.gn is_component_build = false is_debug = false target_cpu = "x86" C:\src\chromium3\src>dir out\release_no_branding\chrome.exe 09/15/2016 02:57 PM 3,036,160 chrome.exe C:\src\chromium3\src>git log -2 --oneline 5a8bc1a chrome_initial does not need //ui/gfx on Windows. dce171b CrOS MD - use vectorized icons for ethernet and VPN. This is unexpected. This means that while the official release binary will *probably* be the right size, there is clearly still some fragility in the setup, so probably best to leave this bug open for future investigation.
,
Sep 16 2016
I found the difference between Chrome and Chromium branded builds: r355292 added a dependency on //ui/base to //components/version_info:version_info. This is a problem because the latter is needed for //components/startup_metric_utils/browser:lib. This might be more tricky to fix, so I propose that we merge r418986 back to M54 while we work on the rest of this. What do you think?
,
Sep 16 2016
That sounds reasonable. Fixing the immediate regression with a simple change, and fixing the longer-term risk with a more complex change. Did you find out why the initial regression wasn't noticed? It seems like the ideal test-case for sizes monitoring.
,
Sep 19 2016
Yes. While chrome.exe's size was shown on the dashboard, it wasn't being monitored for changes. It is now. Requesting merge of r418986 to M54 as per comments 4 and 5.
,
Sep 19 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Sep 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d9efb8c742f159c782a6574be6299feb3cf403d commit 9d9efb8c742f159c782a6574be6299feb3cf403d Author: Greg Thompson <grt@chromium.org> Date: Mon Sep 19 09:51:52 2016 chrome_initial does not need //ui/gfx on Windows. chrome.exe appears to have grown by 150% when skia rolled in r411308. chrome.exe shouldn't need skia or gfx, so let's try not depending on them! BUG= 647223 R=brucedawson@chromium.org Review-Url: https://codereview.chromium.org/2345503006 Cr-Commit-Position: refs/heads/master@{#418986} (cherry picked from commit 1504ef7199454f8096cf1ecdece90a8476aa4a31) TBR=grt@chromium.org Review URL: https://codereview.chromium.org/2348223002 . Cr-Commit-Position: refs/branch-heads/2840@{#408} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/9d9efb8c742f159c782a6574be6299feb3cf403d/chrome/BUILD.gn
,
Sep 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a237cc46f9274d80df60813c10b69ec3067d7c2c commit a237cc46f9274d80df60813c10b69ec3067d7c2c Author: grt <grt@chromium.org> Date: Tue Sep 20 22:56:19 2016 Break chrome_initial's dependence on //components/startup_metric_utils/browser:lib We can't have chrome.exe depending on browser code since doing so can unexpectedly cause the size of chrome.exe to bloat. This change accomplishes the breakage by passing the main exe entry TimeTicks through to chrome.dll directly by way of ChromeMain and ChromeMainDelegate rather than via an environment variable. BUG= 647223 R=brucedawson@chromium.org Review-Url: https://codereview.chromium.org/2345933002 Cr-Commit-Position: refs/heads/master@{#419881} [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/chrome/BUILD.gn [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/chrome/app/chrome_exe_main_win.cc [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/chrome/app/chrome_main.cc [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/chrome/app/chrome_main_delegate.cc [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/chrome/app/chrome_main_delegate.h [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/chrome/app/main_dll_loader_win.cc [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/chrome/app/main_dll_loader_win.h [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/components/startup_metric_utils/browser/startup_metric_utils.cc [modify] https://crrev.com/a237cc46f9274d80df60813c10b69ec3067d7c2c/components/startup_metric_utils/browser/startup_metric_utils.h
,
Sep 21 2016
chrome.exe is smallish again, and monitoring is now on. Calling this fixed.
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d9efb8c742f159c782a6574be6299feb3cf403d commit 9d9efb8c742f159c782a6574be6299feb3cf403d Author: Greg Thompson <grt@chromium.org> Date: Mon Sep 19 09:51:52 2016 chrome_initial does not need //ui/gfx on Windows. chrome.exe appears to have grown by 150% when skia rolled in r411308. chrome.exe shouldn't need skia or gfx, so let's try not depending on them! BUG= 647223 R=brucedawson@chromium.org Review-Url: https://codereview.chromium.org/2345503006 Cr-Commit-Position: refs/heads/master@{#418986} (cherry picked from commit 1504ef7199454f8096cf1ecdece90a8476aa4a31) TBR=grt@chromium.org Review URL: https://codereview.chromium.org/2348223002 . Cr-Commit-Position: refs/branch-heads/2840@{#408} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/9d9efb8c742f159c782a6574be6299feb3cf403d/chrome/BUILD.gn |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by grt@chromium.org
, Sep 15 2016