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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgst-QvQkM


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

win-32

Caused by: "Initialize logging for Chrome watcher process"
Commit: 3016c7260fec3bd2485a0377166bb0aa52112a15

Hi,

First time dealing with a size regression. Fortunately the CL is very simple. Is there a general approach here? 

I'm guessing the issue is pulling in chrome/common. Should I try breaking it up? How can I know if the change is effective?

Thanks!

I'd say the general approach is just to try and fix the regression. Up to you if you want to revert & reland, or do a fix-up. This was triaged quite late (as you can see), so clearly no big urgency. Still, it's a big regression, so should be fixed.

In terms of knowing when you've fixed it, just build with the same GN args as the perf bots and see if the file size shrinks.

If you'd like someone to help you look at why this happened (and on Windows only), +brucedawson is an expert.

Removing sheriff label as relevant owner has been found. Chrome Go is targeting M-61.

You can make some changes and then run "tools/binary_size/diagnose_bloat.py HEAD". https://chromium.googlesource.com/chromium/src.git/+/master/tools/binary_size/README.md

Currently working on https://codereview.chromium.org/2942643002

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

commit ea6c95a904cac342b91cc68b06d2851a3d9e6394
Author: manzagop <manzagop@chromium.org>
Date: Thu Jun 15 22:59:12 2017

Break up the chrome/common:common target

This CL breaks off some targets from chrome/common:common.

Context: https://crrev.com/2850863002 introduced a binary size
regression. In order to address it, chrome_logging.* needs to be
pulled out from chrome/common:common to a separate target, but this turns out to be hard. This CL is a stepping stone.

BUG= 717103 

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

[modify] https://crrev.com/ea6c95a904cac342b91cc68b06d2851a3d9e6394/chrome/common/BUILD.gn
[add] https://crrev.com/ea6c95a904cac342b91cc68b06d2851a3d9e6394/chrome/common/cloud_print/BUILD.gn
[modify] https://crrev.com/ea6c95a904cac342b91cc68b06d2851a3d9e6394/chrome/common/cloud_print/OWNERS

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

commit ce85174f53239960c7007e4b6c8bc6bde272f1a5
Author: manzagop <manzagop@chromium.org>
Date: Fri Jun 16 17:59:07 2017

Break up chrome/common:common: importer

This CL breaks off some targets from chrome/common:common.

Context: https://crrev.com/2850863002 introduced a binary size
regression. In order to address it, chrome_logging.* needs to be
pulled out from chrome/common:common to a separate target, but
this turns out to be hard. This CL is a stepping stone.

BUG= 717103 

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

[modify] https://crrev.com/ce85174f53239960c7007e4b6c8bc6bde272f1a5/chrome/common/BUILD.gn
[modify] https://crrev.com/ce85174f53239960c7007e4b6c8bc6bde272f1a5/chrome/common/importer/BUILD.gn

In parallel, I've tried the windows binary tools. 
https://www.chromium.org/developers/windows-binary-sizes

Not sure how to interpret though.

c:\src\chromium\src>python tools\win\pe_summarize.py c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll c:\src\chromium\src\out\watcher_build\chrome_watcher.dll
Size of c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll is 7.439872 MB
      name:   mem size  ,  disk size
     .text:  6.378913 MB
    .rdata:  0.697313 MB
     .data:  0.025576 MB,  0.003072 MB
    .idata:  0.125562 MB
    .gfids:  0.000376 MB
      .tls:  0.000777 MB
    .00cfg:  0.000260 MB
     .rsrc:  0.062892 MB
    .reloc:  0.167994 MB

Size of c:\src\chromium\src\out\watcher_build\chrome_watcher.dll is 0.087040 MB
      name:   mem size  ,  disk size
     .text:  0.056218 MB
    .rdata:  0.010721 MB
     .data:  0.001776 MB,  0.001024 MB
    .idata:  0.011528 MB
    .gfids:  0.000361 MB
    .00cfg:  0.000260 MB
     .rsrc:  0.002044 MB
    .reloc:  0.002697 MB

Memory size change from c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll to c:\src\chromium\src\out\watcher_build\chrome_watcher.dll
       .text: -6322695 bytes change
      .rdata: -686592 bytes change
       .data:  -23800 bytes change
      .idata: -114034 bytes change
      .gfids:     -15 bytes change
Names for section[5] don't match. Aborting.

In terms of globals:

c:\src\chromium\src>python tools\win\pdb_compare_globals.py c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll.pdb c:\src\chromium\src\out\watcher_build\chrome_watcher.dll.pdb
13 interesting globals in c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll.pdb, 1 interesting globals in c:\src\chromium\src\out\watcher_build\chrome_watcher.dll.pdb
Symbols that are in c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll.pdb but not in c:\src\chromium\src\out\watcher_build\chrome_watcher.dll.pdb
0       0          512  2       Cr_z__dist_code
110     440          0  0       MOJO_RESULT_OK
0       0         1576  2       htmlStartClose
0       0         4000  2       kWebuiResources
0       0         3036  2       html40EntitiesTable
0       0         8192  3       g_CrcTable
0       0         1152  2       kUiResources
0       0         2848  2       kThemeResources
0       0         8192  2       crc_table
0       0          788  2       xmlIsBaseChar_srng
0       0         1152  2       static_ltree
0       0         3312  2       html40ElementTable

Symbols that are in c:\src\chromium\src\out\watcher_build\chrome_watcher.dll.pdb but not in c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll.pdb

Symbols that are changed from c:\src\chromium-alt\src\out\DebugBranded\chrome_watcher.dll.pdb to c:\src\chromium\src\out\watcher_build\chrome_watcher.dll.pdb

The size reductions look huge, particularly the .text (code) section.

The section names not matching usually means that the two builds used different settings (32-bit versus 64-bit for instance). You need gn args to make the comparison work. In this case it may be that a section in the new binary was completely eliminated. Anyway, the .text and .rdata changes are significant enough to mark this as very successful.

The ShowGlobals tool is focused on finding wasteful globals - those that are very big are duplicated many times. In this case you are more interested in seeing what symbols have gone away, whether they are 'interesting' or not. For this purpose I suspect that SymbolSort would be more relevant, if you want to dig in deeper. It's documented on the same page. Sample usage is:

SymbolSort -in new\chrome.dll.pdb -diff old\chrome.dll.pdb -out change.txt

Thanks for the tips Bruce!

Actually, this is comparing with prior to the offending change - so nothing's fixed yet! ;)

As a next step, I'm thinking that instead of breaking up chrome/common:common some more, I'll try to extract the specific function I care about to another file (since extracting the file from the target was difficult).

Let me know if you're not sure of how the other files are being pulled in - SymbolSort can tell you what symbols are being pulled in, and tools\win\linker_verbose_tracking.py can tell you why.

I've tried to document it as well as possible but it's messy stuff.

I've tried digging some more but not sure what to make of it. Putting the files here, but don't feel like you need to look at them.

I'm going to try to simply extract the function from the file.

Deleted: changes.txt 780 KB

changes.txt 780 KB View Download

Deleted: watcher_verbose_link 1.3 MB

watcher_verbose_link 1.3 MB View Download

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

commit 55aebefd499df868e163541642068290cf2eea11
Author: manzagop <manzagop@chromium.org>
Date: Wed Jun 21 14:38:48 2017

Break up chrome/common:common: safe_browsing

This CL breaks off some targets from chrome/common:common.

Context: https://crrev.com/2850863002 introduced a binary size
regression. In order to address it, chrome_logging.* needs to be
pulled out from chrome/common:common to a separate target, but
this turns out to be hard. This CL is a stepping stone.

BUG= 717103 

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

[modify] https://crrev.com/55aebefd499df868e163541642068290cf2eea11/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/55aebefd499df868e163541642068290cf2eea11/chrome/common/BUILD.gn
[modify] https://crrev.com/55aebefd499df868e163541642068290cf2eea11/chrome/common/safe_browsing/BUILD.gn
[modify] https://crrev.com/55aebefd499df868e163541642068290cf2eea11/chrome/common/safe_browsing/OWNERS
[modify] https://crrev.com/55aebefd499df868e163541642068290cf2eea11/chrome/utility/BUILD.gn

Still chipping away at this. 

- I tried extracting only the function I care about, but turns out the one I care about calls most of the others.

- I tried only adding the //chrome/common dependency, without calling any function from it, and that also produces a huge increase, which is not what I'd expect.

I'll go back to trying to cleave the target.

Looked again at the top lines by frequency from the linker verbose file and mojo shows up a lot. Not sure if that means anything.

('        Loaded base.dll.lib(base.dll)\r\r\n', 536)
('     base.dll.lib(base.dll)\r\r\n', 536)
('        Referenced in base.dll.lib(base.dll)\r\r\n', 413)
('        Referenced in common.lib(common_message_generator.obj)\r\r\n', 245)
('        Referenced in preferences.mojom.obj\r\r\n', 157)
('        Referenced in base_requests.obj\r\r\n', 156)
('        Referenced in pref_service_factory.obj\r\r\n', 150)
('        Referenced in media_parser.mojom.obj\r\r\n', 150)
('        Referenced in translate.mojom.obj\r\r\n', 145)
('        Referenced in extension_unpacker.mojom.obj\r\r\n', 143)
('        Referenced in credential_manager.mojom.obj\r\r\n', 133)
('        Referenced in persistent_pref_store_client.obj\r\r\n', 126)
('        Referenced in inline_install.mojom.obj\r\r\n', 124)
('        Referenced in safe_archive_analyzer.mojom.obj\r\r\n', 123)
('        Referenced in device_manager.mojom.obj\r\r\n', 119)
('        Referenced in chrome_watcher_main.obj\r\r\n', 119)
('        Referenced in removable_storage_writer.mojom.obj\r\r\n', 117)
('        Referenced in manifest_parser.mojom.obj\r\r\n', 117)
('        Referenced in oauth2_token_service.obj\r\r\n', 114)
('        Referenced in tracked_preference_validation_delegate.mojom.obj\r\r\n', 113)
('        Referenced in thumbnail_capturer.mojom.obj\r\r\n', 111)
('        Referenced in file_patcher.mojom.obj\r\r\n', 111)
('        Referenced in install_static_util.lib(install_util.obj)\r\r\n', 110)
('        Referenced in page_load_metrics.mojom.obj\r\r\n', 110)
('        Referenced in mime_handler.mojom.obj\r\r\n', 109)
('        Referenced in device.mojom.obj\r\r\n', 108)
('        Referenced in chooser_service.mojom.obj\r\r\n', 107)
('        Referenced in wifi_credentials_getter.mojom.obj\r\r\n', 106)
('        Referenced in dial_device_description_parser.mojom.obj\r\r\n', 106)

I'm leaving Chrome in 1 week and don't think I'll be able to fix this by then. :(

The attempt to break up chrome/common:common some more is 
https://chromiumcodereview.appspot.com/2952243002/

It's currently tripping at link over some unresolved ipc symbols which I'm not clear how to include.

Assigning to Bruce for lack of a better idea...

I did comparisons using a release non-component build based off of today's build and measured about a 3.5 MB increase in chrome_watcher.dll from the suspect CL. Most of this comes from the .text section, but many sections get significantly larger and some new sections appear - .rodata and _RDATA. I might update pe_summarize.py so that it lists those as newly appeared instead of just aborting. Note, for instance, that the .data section's in-memory size grew much more than its on-disk size, which arguably means that the growth is worse that the file size indicates:

> python tools\win\pe_summarize.py slim\chrome_watcher.dll bloated\chrome_watcher.dll
Size of slim\chrome_watcher.dll is 0.476672 MB
      name:   mem size  ,  disk size
     .text:  0.367508 MB
    .rdata:  0.084460 MB
     .data:  0.016340 MB,  0.006656 MB
     .rsrc:  0.001280 MB
    .reloc:  0.014908 MB

Size of bloated\chrome_watcher.dll is 3.950080 MB
      name:   mem size  ,  disk size
     .text:  3.341898 MB
    .rdata:  0.449694 MB
     .data:  0.666948 MB,  0.012800 MB
   .rodata:  0.005520 MB
    _RDATA:  0.000288 MB
     .rsrc:  0.051984 MB
    .reloc:  0.085064 MB

I then ran SymbolSort:
> SymbolSort.exe -in bloated\chrome_watcher.dll.pdb -diff slim\chrome_watcher.dll.pdb -out change.txt

and looked at the "Increases in Total Size" section. At least part of the underlying issue was immediately obvious:

Increases in Total Size
  Total Size  Total Count  Name
      262144            1  ff_cos_131072
      131072            1  ff_cos_65536
       65536            1  ff_cos_32768
       65536            1  jpeg_nbits_table
       53248            1  av_crc_table
       32768            1  ff_cos_16384
       20771            1  encode_one_block
       16384            1  ff_cos_8192
       16384            2  gFDot6INVERSE

I've seen these symbols before when they inappropriately ended up in chrome.dll. That means that this logging change has pulled them in, somehow. The next step is to do a verbose link of chrome_watcher.dll by adding this line to its shared_library definition:

    ldflags += [ "/verbose" ]

Then, build chrome_watcher.dll (just a relink is fine) and redirect it to verbose00.txt.

ff_cos_131072 is defined in fft_template.c which is #included by fft_fixed.c (sigh...) so we run this to see what is bringing it in:

> python tools\win\linker_verbose_tracking.py verbose00.txt fft_fixed
Database loaded - 4583 xrefs found
No references to fft_fixed.obj found. Directly specified in sources or a source_set?

Hmmm...

fft_fixed.c is part of ffmpeg_c_sources which, in non-component builds, is a static_library, specifically to avoid this problem. So we have a contradiction.

Ah. fft_template.c is also included by fft_float.c, which gives us this:

python tools\win\linker_verbose_tracking.py verbose00.txt fft_float
Database loaded - 4583 xrefs found
fft_float.obj pulled in for symbol "_ff_fft_init" by
        mdct_float.obj

mdct_float.obj pulled in for symbol "_ff_mdct_init" by
        vorbisdec.obj

vorbisdec.obj pulled in for symbol "_ff_vorbis_decoder" by
        allcodecs.obj

allcodecs.obj pulled in for symbol "_avcodec_register_all" by
        allformats.obj

allformats.obj pulled in for symbol "_av_register_all" by
        Command-line obj file: ffmpeg_glue.obj


So now we have a better problem statement. ffmpeg_glue.obj is being explicitly linked into chrome_watcher.dll and this is pulling in a lot of excess code and data and presumably accounts for much or all of this regression. ffmpeg_glue.cc is part of the //media/filters source_set. So then we look for why //media/filters is being pulled in:

>gn path out\release //media/filters //chrome/chrome_watcher
//chrome/chrome_watcher:chrome_watcher --[private]-->
//chrome/common:common --[public]-->
//media:media --[public]-->
//media/filters:filters

And there we have it. //chrome/common pulls in //media which pulls in //media/filters which is a source_set so it gets forcibly linked in. The only possibility of salvation is /opt:ref removing the extraneous code and data but that often doesn't work and we don't want to count on it for so much.

Now that the problem is understood we get to move on to the next task which is finding a fix.

Fixin' it:

Step 1 was to change //media:filters from a source_set to a static_library. That made an inconsequential improvement but is probably required as part of the final fix.
Step 2 was to do another verbose link and run linker_verbose_tracking.py again. This time it says that the root which is pulling in fft_float.obj is test_mojo_media_client.obj!

Uh, what? Why are we linking with test code?

That .obj file is part of the //media/mojo/services:lib source_set. That's terribly dangerous because that means that test code is being forcibly pulled in to all sorts of places where it isn't needed.

Commenting out that .cc file led to a missing symbol:
media_service_factory.obj : error LNK2019: unresolved external symbol "public: __thiscall media::TestMojoMediaClient::TestMojoMediaClient(void)"

So I commented out CreateMediaServiceForTesting() in media_service_factory.cc.

That shaved about 700 KB off of the delta reported by pe_summarize.py, which still leaves us with a 3.4 MB discrepancy.

media_service_factory.cc is part of the //media/mojo/services:lib source_set. I think that the proper fix is that that source_set must be converted to a static_library and CreateMediaServiceForTesting() must be moved to a separate source file. I'll create a CL for that now.

BTW, I also tried running tools\win\pdb_compare_globals.py to compare the before/after (slim/bloated) chrome_watcher.dll.pdb global variables. The requires having ShowGlobals.exe in your path and the results will depend on exactly what settings it was compiled with. I filtered the results to just show the variables that are 1 KiB or larger. The results are:

254 interesting globals in slim\chrome_watcher.dll.pdb, 815 interesting globals in bloated\chrome_watcher.dll.pdb
Symbols that are in slim\chrome_watcher.dll.pdb but not in bloated\chrome_watcher.dll.pdb

Symbols that are in bloated\chrome_watcher.dll.pdb but not in slim\chrome_watcher.dll.pdb
0	0	  1024	3	kf_low_motion_minq_8
0	0	  1024	3	rtc_minq_8
0	0	  1024	3	rtc_minq_10
0	0	  1024	3	arfgf_high_motion_minq_10
0	0	  1024	3	rtc_minq_12
0	0	  1024	3	arfgf_high_motion_minq_12
0	0	  1024	3	inter_minq_8
0	0	  8192	3	ff_cos_4096
0	0	  1024	3	arfgf_low_motion_minq_8
0	0	  1024	3	sad_per_bit4lut_8
0	0	  4168	3	WTF::Partitions::fast_malloc_allocator_
0	0	 32768	3	ff_cos_16384
0	0	  1536	3	gEntries
0	0	 16384	3	ff_cos_8192
0	0	262144	3	ff_cos_131072
0	0	  1024	3	ff_cos_512
0	0	  1024	3	sad_per_bit4lut_12
0	0	  1024	3	kf_low_motion_minq_10
0	0	  1024	3	sad_per_bit4lut_10
0	0	  2048	3	ff_cos_1024
0	0	  1024	3	sad_per_bit16lut_12
0	0	  1024	3	sad_per_bit16lut_10
0	0	  1024	3	kf_low_motion_minq_12
0	0	  1024	3	kf_high_motion_minq_8
0	0	  4168	3	WTF::Partitions::array_buffer_allocator_
0	0	 65536	3	ff_cos_32768
0	0	  4096	3	kGammaToLinearTabF
0	0	 53248	3	av_crc_table
0	0	  4096	3	ff_cos_2048
0	0	  1024	3	arfgf_low_motion_minq_10
0	0	  1024	3	arfgf_low_motion_minq_12
0	0	  4168	3	WTF::Partitions::buffer_allocator_
0	0	  1024	3	kf_high_motion_minq_12
0	0	  1024	3	kf_high_motion_minq_10
0	0	  5240	3	WTF::Partitions::layout_allocator_
0	0	  1024	3	sad_per_bit16lut_8
0	0	131072	3	ff_cos_65536
0	0	  1024	3	inter_minq_12
0	0	  2048	3	fixed_divide
0	0	  1024	3	arfgf_high_motion_minq_8
0	0	  1024	3	inter_minq_10

Symbols that are changed from slim\chrome_watcher.dll.pdb to bloated\chrome_watcher.dll.pdb

This suggests that most of the bloat comes from ffmpeg, but WTF::Partitions got pulled in as well. It's not clear whether the whack-a-mole method of fixing all of the places where //chrome/common pulls in too many source sets is the most practical solution to this specific problem, but I believe it has value and may help fix or prevent other binary-size regressions. The final fix for this specific regression may require linking with something other than //chrome/common so that these other targets are never referenced in the first place.

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

commit 9dfa695ca9c87195cf609032d933ccac9987821d
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Jul 07 22:26:02 2017

Fix pe_summarize.py to handle mismatched sections

pe_summarize.py is used to summarize the sizes of the sections in a PE
file (a DLL or EXE). It can also summarize the differences between two
PE files which is handy when investigating size regressions. The
original version of this script assumed that section names matched
exactly but this assumption does not always hold. This assumption breaks
when comparing 32-bit PE files to their 64-bit counterparts, and it
breaks if the size regression includes some new sections being added.

So, this change updates the script to handle these mismatches, to
increase flexibility and prevent confusion.

I tested this change with the before/after versions of
chrome_watcher.dll associated with  crbug.com/717103 . It listed all
the changed sections and gave consistent total differences.

BUG:  717103 

Change-Id: I923e2655605333a72f0c9e3b9eecf6f7ec2a62d3
Reviewed-on: https://chromium-review.googlesource.com/563763
Reviewed-by: Stanislav Chiknavaryan <stanisc@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485074}
[modify] https://crrev.com/9dfa695ca9c87195cf609032d933ccac9987821d/tools/win/pe_summarize.py

With the initial (not yet landed) fix applied the pdb_compare_globals.py script shows the sin/cos arrays all gone (yay!) but only about a 100 KB file-size savings. Next step, figure out why sad_per_bit4lut_8 is being pulled in - it seems like a good representative of another class of data. That is defined in vp9_rd.c linker_verbose_tracking.py says it is pulled in (after a few levels of indirection) because of:

mime_util.obj pulled in for symbol "bool __cdecl mime_util::IsSupportedImageMimeType(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &)" (?IsSupportedImageMimeType@mime_util@@YA_NABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z" by
        Command-line obj file: drop_data.obj
        Command-line obj file: manifest_icon_selector.obj

That's because of source_set("common_sources") and source_set("browser"). After changing those I get:

web_contents_impl.obj pulled in for symbol "public: class content::BrowserPluginGuest * __thiscall content::WebContentsImpl::GetBrowserPluginGuest(void)const " (?GetBrowserPluginGuest@WebContentsImpl@content@@QBEPAVBrowserPluginGuest@2@XZ" by
        Command-line obj file: guest_mode.obj

which is because of source_set("browser_sources"). After changing that I get:

mime_util.obj pulled in for symbol "bool __cdecl media::IsSupportedMediaMimeType(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &)" (?IsSupportedMediaMimeType@media@@YA_NABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z" by
        mime_util.obj

Fun fact: there are four mime_util.cc files. And, the linker_verbose_tracking.py gets confused by this because it records references purely by their .obj names, discarding the containing .lib file. That's why this last run didn't trace all the way back to a command-line obj file. I think I need to fix this. And, the number of changes needed to (so far) not gain anymore ground makes me think that this path is not profitable.

It's time to try the other way: remove //chrome/common from the list of deps and then gradually add back what is needed.

InitChromeLogging is the only missing symbol when //chrome/common is removed. It is defined in logging_chrome.cc so we need to move this out of //chrome/common, to //chrome/minimal. After two iterations of adding dependencies to resolve missing string constant symbols I ended up with this:

static_library("minimal") {
  sources = [
    "logging_chrome.cc",
    "logging_chrome.h",
  ]
  defines = []

  configs += [
    "//build/config:precompiled_headers",
    "//build/config/compiler:wexit_time_destructors",
  ]

  public_deps = [
    "//chrome/common:constants", # needed for env_vars::kHeadless and env_vars::kLogFileName
    "//content/public/common:static_switches" # needed for switches::kEnableLogging and switches::kLoggingLevel
  ]
}

Unfortunately using this instead of //chrome/common only saved about 500 KB, leaving us still far over budget. Commenting out the call to InitChromeLogging allows for easier experimentation because it doesn't change the binary size significantly and lets us comment out deps with impunity. Commenting out "//content/public/common:static_switches" left chrome_watcher.dll unchanged at 3,366,400 bytes, so that isolates the problem. Apparently //chrome/common:constants is much more than just some constants. And indeed, its list of deps includes //media:media_features and much more - some of those are the problems.

We needed the constants static_library for env_vars.cc, for const char kHeadless[] and const char kLogFileName[]. That's it. If I make those available through other mechanisms (copy/paste of the constants for now) then I can build chrome_watcher.dll with a call to InitChromeLogging that is 502,784. This is a cost of 26,112 bytes for enabling logging which seems entirely reasonable. I'll clean this up into something sane and submit it.

Also, changing the title to better reflect the issue. This was a ~4.1 MB regression in the size of chrome_watcher.dll. It was detected as a 257 KB regression in the size of mini_installer.exe - apparently our compression works really well.

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

commit 8dd579aa48ea778a0caa2257c875593bbba86b93
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Mon Jul 10 22:31:59 2017

Improve linker_verbose_tracking.py

linker_verbose_tracking.py is a useful script for understanding why
particular symbols and object files get pulled in to Chrome binaries
on Windows. Previously this script did its tracking purely by object
file name, which breaks down for things like mime_util.obj because there
are *four* object files with that name.

This change updates the script so that it tracks the chain of
dependencies using fully-specified object file names - basically
foo.lib(bar.obj) instead of just bar.obj. This avoids the dead-ends and
misleading information that occasionally happened with the previous
version.

In order to make this usable (specifying a fully-qualified object file
name to search for is unwieldy) the search string is now understood to
be any sub-string of the object file name. The set of matches is printed
so that the search can easily be refined. It works much better now.

This was made necessary by the investigation of the linked bug.

R=stanisc@chromium.org
BUG= 717103 

Change-Id: I25682e6d5b534c69d3743456918c60d42f746e10
Reviewed-on: https://chromium-review.googlesource.com/565221
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Stanislav Chiknavaryan <stanisc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485425}
[modify] https://crrev.com/8dd579aa48ea778a0caa2257c875593bbba86b93/tools/win/linker_verbose_tracking.py

Getting the solution mentioned in comment #30 to build was straightforward - see crrev.com/c/565226. However this solution #includes files from distant lands, and doing so without having a dependency is illegal (caught by gn analyze). Resolving this without major reorganizing of BUILD.gn files or reintroducing the size regression was not looking promising.

So, I'm trying a third solution: reimplement InitChromeLogging as InitChromeWatcherLogging, directly inside of chrome_watcher_main.cc. This gives me a solution which builds and passes gn analyze. The copy/paste of code is unfortunate but can be minimized if we don't need all of the features of InitChromeLogging. See crrev.com/c/567030 for the current state of this change.

Unfortunately that change still doesn't give us the behavior that we want - because it doesn't link with chrome\common\chrome_paths.cc it doesn't have a path provider for chrome::DIR_LOGS so the logs still go to the wrong directory. So, more copy/paste is needed.

Another problem is that when logging is enabled with "--enable-logging --v=1" on the command-line the watcher process doesn't get these arguments, so it seems to never turn on logging anyway. This defect seems to exist with the existing code (with the size regression) as well.

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

commit 5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Jul 13 20:54:52 2017

Fix chrome_watcher logging size regression

When crrev.com/2850863002 added a call to InitChromeLogging() it had to
pull in //chrome/common:common, and that necessarily pulls in several MB
of code that is not wanted. The tendrils of dependencies are many and
varied and since a sword was disallowed there was no practical way to
cut this Gordian knot. See the investigation in  bug 717103  for details.

So, InitChromeLogging() was copied, named as InitChromeWatcherLogging(),
and made to work. This gets the size-cost of the logging code down to
~25 KB.

A non_code_constants target was added to //chrome/common because the
existing constants target was actually the main cause of excess code
being pulled in.

While testing this change it was discovered that the command-line
logging flags were not passed to the chrome_watcher process, so that
logging was not actually being enabled. This was also fixed.

It may be possible to delete more of the copied code, depending on what
logging features are actually needed. Thoughts?

Bug:  717103 ,  715060 
Change-Id: I536b0813fff32e513dfd3e0bbb8fc3cb715546c1
Reviewed-on: https://chromium-review.googlesource.com/567030
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486473}
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/app/chrome_watcher_command_line_win.cc
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/chrome_watcher/BUILD.gn
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/chrome_watcher/chrome_watcher_main.cc
[modify] https://crrev.com/5c9466ebd9a5800fa1631cadddda6c7b1ce65ae5/chrome/common/BUILD.gn

Well that was fun!

- tools\win\pe_summarize.py is now much more robust
- tools\win\linker_verbose_tracking.py is now much more reliable
- Size regression is fixed
- Link time for chrome_watcher.dll is back to being short
- Command-line logging options are now passed to the chrome_watcher process

The original graph shows that mini_installer.exe has shrunk by about 200 KB which is close to the size of the original regression. The difference is probably genuine growth caused by chrome_watcher.dll increasingly slightly in size from having logging properly initialized.

Actually it looks like virtually the entire regression is gone - I was comparing the size of the 64-bit regression to the size-improvement of the 32-bit version.

Whoa. Thank you Bruce for cleaning after me.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/80eb799a93794f4803f70e11819477c82d006a8c

commit 80eb799a93794f4803f70e11819477c82d006a8c
Author: Greg Thompson <grt@chromium.org>
Date: Wed Aug 02 21:01:37 2017

Report the sizes of chrome_elf.dll and chrome_watcher.dll.

BUG= 717103 

Change-Id: I294bad9b0527e82d141abdc1dc4926ea092b2416
Reviewed-on: https://chromium-review.googlesource.com/595747
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/80eb799a93794f4803f70e11819477c82d006a8c/scripts/slave/chromium/sizes.py