All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=703622

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


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

win
win-32

Looks like this was due to: https://chromium.googlesource.com/chromium/src/+/1432276fb82ae8e9b732faa03ab65f35d686017f

Sadrul, does that seem plausible?

I don't think so? It moves some code from one component to another. So it shouldn't cause a regression (especially by such a large amount) I think ... is it possible the skia roll at the next revision (crrev.com/458277) would cause something like this?

Yup, seems fairly likely, thanks.

IIUC we're blaming this commit?
https://chromium.googlesource.com/chromium/src/+/b3a64fe5febbf538bb12d356728c3d5dba119ca7

That's a small change to Skia's image encoders which Chrome does not use (and AFAICT does not compile).

Can you explain what this graph is measuring?  The chrome executable is 3x bigger?  I have a feeling that we're missing something, this is really strange.

Yeah, you're right, something funny is going on here. Over to the sizes owner.

One graph says the regression happened at 458277, but the other one says it happened at 458276. Visually the 458276 regression point looks more reasonable. That makes this the problematic CL:

https://codereview.chromium.org/2751103008

I'll probably grab this bug. I'm doing a chrome.exe build right now and I have the tools to figure out where the bloat comes from.

Because chrome.exe is fairly small it is easy to cause a large regression if the wrong code gets pulled in, so I'm sure this is real.

Aside: it would be nice if the bugs or the graphs page would make it obvious what build hit the regression - what gn args will reproduce the problem. But, in this case the bug is very easy to reproduce. I tried with these settings that I had lying around and it shows chrome.exe as 3.2 MB at R458530:

is_component_build = false
is_debug = false
target_cpu = "x86"
enable_nacl = false
treat_warnings_as_errors = false
use_goma = true
is_win_fastlink = true

 Issue 704300  has been merged into this issue.

two separate bisects have identified the culprit CL, I think we should just revert and do the investigation after a revert. Shall I prep the revert?

a revert of this CL has been created in https://codereview.chromium.org/2770953002/

gpu/config, which becomes a new dependency of chrome is probably poorly setup to be depended-on in multiple places in the split-DLL world

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

commit aa9787bbd7919d6114c6bb40fe8e30cdae424ba1
Author: wfh <wfh@chromium.org>
Date: Thu Mar 23 01:23:13 2017

Revert of gpu: Add a util method to set crash-keys from a GPUInfo. (patchset #2 id:20001 of https://codereview.chromium.org/2751103008/ )

Reason for revert:
causes size regression see  crbug.com/703622 

Original issue's description:
> gpu: Add a util method to set crash-keys from a GPUInfo.
>
> Use the newly introduced util method to set crash-keys from a GPUInfo
> in both chrome and android_webview. This will also be used in mus.
>
> BUG= 643746 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;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/2751103008
> Cr-Commit-Position: refs/heads/master@{#458276}
> Committed: https://chromium.googlesource.com/chromium/src/+/1432276fb82ae8e9b732faa03ab65f35d686017f

TBR=boliu@chromium.org,bradnelson@chromium.org,rsesek@chromium.org,piman@chromium.org,jam@chromium.org,sadrul@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 643746 , 703622 

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

[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/android_webview/common/aw_content_client.cc
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/chrome/BUILD.gn
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/chrome/common/crash_keys.cc
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/chrome/common/crash_keys.h
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/chrome/test/BUILD.gn
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/components/nacl/broker/BUILD.gn
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/gpu/config/gpu_util.cc
[modify] https://crrev.com/aa9787bbd7919d6114c6bb40fe8e30cdae424ba1/gpu/config/gpu_util.h

Looks like the revert did help. Thanks for taking care of this!

> gpu/config, which becomes a new dependency of chrome is probably poorly setup
> to be depended-on in multiple places in the split-DLL world

Would it help to make //gpu/config a 'component' type target instead of 'source_set'? [1]

[1] https://cs.chromium.org/chromium/src/gpu/config/BUILD.gn?type=cs&sq=package:chromium&l=21

Is there a trybot equivalent of this check? (i.e. can I check whether a modified CL reintroduces the regression without landing it in tree first?)

I believe you should be able to use https://www.chromium.org/developers/telemetry/performance-try-bots

> Would it help to make //gpu/config a 'component' type target instead of 'source_set'? [1]

component is the same as static_library in shipping builds, but a dynamic library in component builds, so you'd need EXPORT macros and whatnot. The target you linked to is fairly small, so the size increase probably comes from that target's deps being large. So my guess is that that wouldn't help, and you'll have to make a small target containing just what you want to have in both exe and dll, and make the exe depend on just that.

piman: Note that the size regression is on chrome.exe, not on either of chrome.dll or chrome_child.dll -- so this isn't really about split-dll.

> So my guess is that that wouldn't help, 

You are right, it didn't.

> and you'll have to make a small target containing just what you want to have
> in both exe and dll, and make the exe depend on just that.

That's what I ended up doing :) https://codereview.chromium.org/2751103008/#msg38 Thanks!

FYI: the CL has relanded (r459202) with the proposed fix, and sizes has not regressed from that. (it does look like there's a more recent regression, possibly from r459244)

Marking fixed, based on comments above.