New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users
Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocking:
issue 686608



Sign in to add a comment
chrome_child.dll grew 5MB recently (and nobody noticed?)
Project Member Reported by h...@chromium.org, Apr 24 2017 Back to list
See https://chromeperf.appspot.com/report?sid=7778cd057883709478caf1f8ed5af083422aefb955c6e80c573bb29c51afc8ed&start_rev=465181&end_rev=466768

Looks like this change: https://codereview.chromium.org/2762593002


I assume we should revert that. But the second question is how this was only noticed by myself and not some automatic perf system?
 
Comment 1 by h...@chromium.org, Apr 24 2017
Blocking: 686608
Cc: pmeenan@chromium.org
re:reverting https://crrev.com/d238dae172d63175b562f99fc988e47e5213a244
I think that we have landed a couple cls that depend on this already, so reverting could be tricky. 

It should be a matter of removing unnecessary libraries at headless child lib https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=84e360a7f1dddd9ce125ad6f1de090b7d207c569&l=591

I think the most straightforward solution would be to land a separate cl to bring this down in the next two days. wdyt?
I don't think chrome_child is linking headless_shell_lib?
Oh, sorry headless_shell_child_lib. I don't obviously see what's causing the increase.
Comment 7 by thakis@chromium.org, Apr 24 2017
The change only landed a bit over a weekend ago. If it's already difficult to revert, let's do that while it's still possible.
Comment 8 by wfh@chromium.org, Apr 24 2017
FWIW I agree with #7 if there is a way to revert, even if it involves multiple reverts, we should do so.
I'm trying a dry run: https://codereview.chromium.org/2837093003/
Yup, I agree on revert too, hopefully just reverting a few CLs in reverse order?

fwiw, SymbolSort didn't enlighten me:

c:\stuff>dir 466124\chrome-win32\chrome_child.dll
 Volume in drive C has no label.
 Volume Serial Number is 88D1-EBAA

 Directory of c:\stuff\466124\chrome-win32

2017-04-20  02:26 PM        66,627,072 chrome_child.dll
               1 File(s)     66,627,072 bytes
               0 Dir(s)  238,838,808,576 bytes free

c:\stuff>dir 466266\chrome-win32\chrome_child.dll
 Volume in drive C has no label.
 Volume Serial Number is 88D1-EBAA

 Directory of c:\stuff\466266\chrome-win32

2017-04-20  10:58 PM        75,011,072 chrome_child.dll
               1 File(s)     75,011,072 bytes
               0 Dir(s)  238,838,808,576 bytes free

c:\stuff>C:\src\SymbolSort\bin\Release\SymbolSort.exe -diff 466124\chrome-win32\chrome_child.dll -in 466266\chrome-win32\chrome_child.dll -out diff.txt
Loading symbols from 466266\chrome-win32\chrome_child.dll
Reading section info...
Reading source file info...
Reading global function symbols... 100% complete
Reading thunk symbols... 100% complete
Reading private data symbols... 100% complete
Reading global data symbols... 100% complete
Subtracting overlapping symbols... 100

Loading symbols from 466124\chrome-win32\chrome_child.dll
Reading section info...
Reading source file info...
Reading global function symbols... 100% complete
Reading thunk symbols... 100% complete
Reading private data symbols... 100% complete
Reading global data symbols... 100% complete
Subtracting overlapping symbols... 100

Processing raw symbols...
Building folder stats...
Computing section stats...
Merging duplicate symbols...
Merging template symbols...
Merging overloaded symbols...
Building tag cloud...
Elapsed Time: 00:00:12.7682445
diff.txt
110 KB View Download
Since you  have the binaries handy, what does this say?

python tools\win\pe_summarize.py c:\stuff\466124\chrome-win32\chrome_child.dll c:\stuff\466266\chrome-win32\chrome_child.dll

It could be good to know what section is showing the growth.

whole lotta code

c:\stuff>python c:\src\cr\src\tools\win\pe_summarize.py c:\stuff\466124\chrome-win32\chrome_child.dll c:\stuff\466266\chrome-win32\chrome_child.dll
Size of c:\stuff\466124\chrome-win32\chrome_child.dll is 66.627072 MB
      name:   mem size  ,  disk size
     .text: 49.876187 MB
    .rdata: 13.051306 MB
     .data:  1.843076 MB,  0.536576 MB
    .pdata:  2.526792 MB
      .tls:  0.000045 MB
   .rodata:  0.005648 MB
    .gfids:  0.003192 MB
    _RDATA:  0.032816 MB
     .rsrc:  0.052400 MB
    .reloc:  0.538000 MB

Size of c:\stuff\466266\chrome-win32\chrome_child.dll is 75.011072 MB
      name:   mem size  ,  disk size
     .text: 57.123275 MB
    .rdata: 13.752514 MB
     .data:  1.876004 MB,  0.543744 MB
    .pdata:  2.914560 MB
      .tls:  0.000045 MB
   .rodata:  0.005648 MB
    .gfids:  0.003192 MB
    _RDATA:  0.032816 MB
     .rsrc:  0.052400 MB
    .reloc:  0.578616 MB

Memory size change from c:\stuff\466124\chrome-win32\chrome_child.dll to c:\stuff\466266\chrome-win32\chrome_child.dll
       .text: 7247088 bytes change
      .rdata:  701208 bytes change
       .data:   32928 bytes change
      .pdata:  387768 bytes change
      .reloc:   40616 bytes change
Total change: 8409608 bytes
This makes no sense. I feel like I must have the wrong pdbs somehow. [ I got them from https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Win_x64/466124/ and 466266/. ]

Maybe I'll try official signed builds instead.
Oh, continuous are probably symbol_level=1. Doh. Gimme a minute or 5.

OK, I pulled 60.0.3077.0 and 60.0.3078.0, got chrome_child.dll out of them, and got the pdbs for those via symchk /v.

c:\stuff\off>dir /s
...
 Directory of c:\stuff\off\3077

2017-04-24  04:56 PM    <DIR>          .
2017-04-24  04:56 PM    <DIR>          ..
2017-04-21  12:21 PM        61,403,992 chrome_child.dll
2017-04-21  03:10 AM     1,497,083,904 chrome_child.dll.pdb
               2 File(s)  1,558,487,896 bytes

 Directory of c:\stuff\off\3078

2017-04-24  04:56 PM    <DIR>          .
2017-04-24  04:56 PM    <DIR>          ..
2017-04-22  04:31 AM        68,366,168 chrome_child.dll
2017-04-22  04:36 AM     1,519,169,536 chrome_child.dll.pdb
...


c:\stuff\off>python c:\src\cr\src\tools\win\pe_summarize.py 3077\chrome_child.dll 3078\chrome_child.dll
Size of 3077\chrome_child.dll is 61.403992 MB
      name:   mem size  ,  disk size
     .text: 45.163597 MB
    .rdata: 12.642828 MB
     .data:  2.412004 MB,  0.475648 MB
    .pdata:  2.473812 MB
   .rodata:  0.009528 MB
    .gfids:  0.003192 MB
      .tls:  0.000045 MB
    _RDATA:  0.041776 MB
     .rsrc:  0.052344 MB
    .reloc:  0.530020 MB

Size of 3078\chrome_child.dll is 68.366168 MB
      name:   mem size  ,  disk size
     .text: 51.120413 MB
    .rdata: 13.261940 MB
     .data:  2.441380 MB,  0.479232 MB
    .pdata:  2.816676 MB
   .rodata:  0.009528 MB
    .gfids:  0.003192 MB
      .tls:  0.000045 MB
    _RDATA:  0.041776 MB
     .rsrc:  0.052344 MB
    .reloc:  0.570288 MB

Memory size change from 3077\chrome_child.dll to 3078\chrome_child.dll
       .text: 5956816 bytes change
      .rdata:  619112 bytes change
       .data:   29376 bytes change
      .pdata:  342864 bytes change
      .reloc:   40268 bytes change
Total change: 6988436 bytes


c:\stuff\off>C:\src\SymbolSort\bin\Release\SymbolSort.exe -diff 3077\chrome_child.dll -in 3078\chrome_child.dll -out diff.txt
Loading symbols from 3078\chrome_child.dll
Reading section info...
Reading source file info...
Reading global function symbols... 100% complete
Reading thunk symbols... 100% complete
Reading private data symbols... 100% complete
Reading global data symbols... 100% complete
Subtracting overlapping symbols... 100

Loading symbols from 3077\chrome_child.dll
Reading section info...
Reading source file info...
Reading global function symbols... 100% complete
Reading thunk symbols... 100% complete
Reading private data symbols... 100% complete
Reading global data symbols... 100% complete
Subtracting overlapping symbols... 100

Processing raw symbols...
Building folder stats...
Computing section stats...
Merging duplicate symbols...
Merging template symbols...
Merging overloaded symbols...
Building tag cloud...
Elapsed Time: 00:13:48.9638129


This seems like a more useful diff. From a quick look it looks like content/browser is getting more-used now. It was already linked in via

> gn path //out/Release //chrome:chrome_child //content/browser
//chrome:chrome_child --[private]-->
//chrome:child_dependencies --[public]-->
//chrome/browser/devtools:devtools --[private]-->
//content/public/browser:browser --[public]-->
//content/public/browser:browser_sources --[private]-->
//content/browser:browser

previous to this CL though, so I might be wrong about that (or this might have just caused so things to be non-discardable). Needs more digging.
Oops, wrong diff.
diff.txt
2.1 MB View Download
Most of the report is too diffuse to be interesting. It seems to be best to start at this section:

Increases in Total Size
  Total Size  Total Count  Name
       29524          549  trace_event_internal::AddTraceEventWithThreadIdAndTimestamp
       12164            0  sqlite3VdbeExec
       10408            1  protected: virtual class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > __cdecl cc::FragmentShader::GetShaderSource(void)const __ptr64
       10233            1  content::`anonymous namespace'::AccessibilityEnumMap::AccessibilityEnumMap
        9642            1  public: static bool __cdecl indexed_db::mojom::DatabaseStubDispatch::Accept(class indexed_db::mojom::Database * __ptr64,class mojo::Message * __ptr64)
        8495            0  sqlite3Pragma
        8028           26  trace_event_internal::AddTraceEventWithThreadIdAndTimestamp<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >

Two of the largest newly added functions are sqlite3 functions, and searching on sqlite3 shows a bunch more. But, not enough to account for more than a tiny fraction of the increase so that's only useful if it leads to a useful thread of the ball of yarn of new code being pulled in.

Thanks for the insights!

I'm trying to do a manual rebase of the patch, hopefully I can submit. 
Can anyone LGTM the patch (https://codereview.chromium.org/2762593002/) I need a committer to approve the rollback
Comment 19 by h...@chromium.org, Apr 25 2017
lgtm'd (i assume you mean https://codereview.chromium.org/2837093003/)
I managed to land the revert and looks like size is back to normal. I agree with #15 that it may be pulling more DEPS than it should, hopefully is an easy fix!
Sometimes a CL will pull in more than expected simply because there is a long and accidental chain of other functions or data in the .obj files pulled in. That is, you call Foo(), and that's all you want, but Bar() is in the same file and it gets pulled in and it references 5 MB of other stuff. Ideally the linker would discard the excess but that doesn't always happen perfectly.

A gn-specific variant on this is caused by source sets - you link with a new source set which means you are linking with all of those files, in a stronger sense than referencing a traditional library. You just want Foo() from foo.cc but you end up pulling in Bar() from bar.cc, and all of its global variables and tangled references.

In either case the trick, if the solution is not apparent, is to find some symbol which gets pulled in but shouldn't (perhaps the sqlite3 functions) and then find out why. I've created some tools that use the VC++ verbose linker output on Windows to systematically figure out why a particular function is pulled in so that you can find and fix these issues quickly and reliably. I've documented the techniques here:
    https://www.chromium.org/developers/windows-binary-sizes
If the instructions don't entirely make sense or don't work then drop me a line.

Thanks for the pointers Bruce, those are really useful.

Note that chrome_dll also experience an increase of 650KB (this was also noted recently in https://bugs.chromium.org/p/chromium/issues/detail?id=714888)

https://chromeperf.appspot.com/report?sid=7abd71420d1c74281b5c0fb6d97750629c34de951459b18435292f2555fa5e5b&start_rev=465788&end_rev=467144

This is more in line with what would be expected of adding headless library to Chrome. So yeah I'm guessing we are pulling unnecessary dependencies. 

It's interesting that you mention source_set, since that's what I used to have a render (child) specific library for headless [1]. I'll see if that could be the issue.


[1] source_set(headless_renderer) in https://codereview.chromium.org/2762593002/diff/560001/headless/BUILD.gn



Is it understood why the perf bots didn't catch this? If not, should we have a separate bug for that?
So I diffed 
gn desc out/Default chrome:main_child --all

All the headless dependencies should be there. 
I manage to remove the others, hopefully that brings the increase to the expected ~650 KB

@@ -314,6 +314,7 @@
   //base/win:base_win_features
   //base/win:eventlog_messages
   //breakpad:breakpad_handler
+  //breakpad:client
   //build/config/freetype:freetype
   //build/config/nacl:nacl_base(//build/toolchain/nacl:irt_x64)
   //build/config/nacl:nacl_base
@@ -545,6 +546,7 @@
   //components/crash/content/app:app
   //components/crash/content/app:app_non_mac_win
   //components/crash/content/app:lib
+  //components/crash/content/browser:browser
   //components/crash/core/common:common
   //components/crash/core/common:crash_keys
   //components/crx_file:crx_file
@@ -690,6 +692,7 @@
   //components/policy/proto:proto_internal_gen
   //components/pref_registry:pref_registry
   //components/prefs:prefs
+  //components/printing/browser:browser
   //components/printing/common:common
   //components/printing/renderer:renderer
   //components/proxy_config:proxy_config
@@ -724,6 +727,7 @@
   //components/search_engines:prepopulated_engines
   //components/search_engines:prepopulated_engines_action
   //components/search_engines:search_engines
+  //components/security_state/content:content
   //components/security_state/core:core
   //components/signin/core/account_id:account_id
   //components/signin/core/browser:browser
@@ -1095,6 +1099,20 @@
   //gpu/ipc/service:service
   //gpu/skia_bindings:skia_bindings
   //gpu/vulkan:features
+  //headless:gen_devtools_client_api
+  //headless:headless
+  //headless:headless_renderer
+  //headless:headless_shell_child_lib
+  //headless:pak
+  //headless:resources
+  //headless:resources_grit
+  //headless:tab_socket
+  //headless:tab_socket__generator
+  //headless:tab_socket__type_mappings
+  //headless:tab_socket_shared__generator
+  //headless:tab_socket_shared_cpp_sources
+  //headless:version_header
+  //headless:version_header_action
   //ipc:ipc(//build/toolchain/nacl:irt_x64)
   //ipc:ipc
   //ipc:mojom(//build/toolchain/nacl:irt_x64)
@@ -2420,4 +2438,5 @@

Cc: skyos...@chromium.org
I did a diff on an official build without the extra dependencies. 
Looks like largest difference comes from gen/Blink/Bindings. I'm not sure why
Looks like content/browser is still there

These are the top differences

 c:\tools\SymbolSort\bin\Debug\SymbolSort.exe -in  .\out\ReleaseHeadless\chrome_child.dll -diff  .\out\Release\chrome_child.dll -out C:\tmp\ss.of.diff
.txt

Raw Symbols Differences
Total Count  : 80060
Total Size   : 7979008
Unattributed : 1056823

File Contributions
--------------------------------------
Increases in Size
        Size   Count  Source Path
     6906481   52954  c:
     6821105   35338  c:\src\out\releaseheadless
     6763266   35214  c:\src\out\releaseheadless\gen
     6034885   43872  c:\src
     4118642   25083  c:\src\out\releaseheadless\gen\blink
     3223203   18353  c:\src\out\releaseheadless\gen\blink\bindings
     2700121   15393  c:\src\content
     2583461   12860  c:\src\content\browser
     1734715   11328  c:\src\out\releaseheadless\gen\blink\bindings\core\v8
     1488488    7025  c:\src\out\releaseheadless\gen\blink\bindings\modules\v8
      871596    9082  c:\users\dvallet\apps\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616
      864393    8986  c:\users\dvallet\apps\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\vc
      857671    8938  c:\users\dvallet\apps\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\vc\include
      838184    4473  c:\src\out\releaseheadless\gen\blink\core
      644752    3238  c:\src\out
      546127   12119  c:\src\base
      522126     458  c:\src\out\releaseheadless\gen\chrome\common
      521279    2914  c:\src\out\releaseheadless\gen\blink\core\inspector\protocol
      467826     185  c:\src\out\releaseheadless\gen\chrome\common\extensions
      465412    2151  c:\src\out\releaseheadless\gen\content
      457972     152  c:\src\out\releaseheadless\gen\chrome\common\extensions\api
      434848    2301  c:\src\content\browser\renderer_host
      433433    3086  c:\users\dvallet\apps\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\vc\include\xtree
ss.of.diff.txt
2.0 MB View Download
Cc: haraken@chromium.org
That's really odd. I would have expected all the v8 bindings to already be there in chrome_child.dll.

(+cc Kentaro in case he has ideas of where to look or who to ask.)
Does this mean that v8 bindings was previously not included in chrome_child.dll? Or does it mean that v8 bindings in chrome_child.dll increased recently?

David, correct me if I'm wrong but I think the symbols were already there but they just grew in size (?)
Look around line 11649 in the report in comment #26:

Increases in Total Size
  Total Size  Total Count  Name
       13810            1  content::`anonymous namespace'::AccessibilityEnumMap::AccessibilityEnumMap
       12093            1  public: static bool __cdecl indexed_db::mojom::DatabaseStubDispatch::Accept(class indexed_db::mojom::Database * __ptr64,class mojo::Message * __ptr64)
       11085            1  protected: virtual class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > __cdecl cc::FragmentShader::GetShaderSource(void)const __ptr64
        9940            1  public: virtual bool __cdecl content::RenderFrameHostImpl::OnMessageReceived(class IPC::Message const & __ptr64) __ptr64
        7952            1  kWebuiResources
        7883            1  public: class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > __cdecl cc::VertexShader::GetShaderString(void)const __ptr64
        7712            1  public: static bool __cdecl leveldb::mojom::LevelDBDatabaseStubDispatch::AcceptWithResponder(class leveldb::mojom::LevelDBDatabase * __ptr64,class mojo::Message * __ptr64,class std::unique_ptr<class mojo::MessageReceiverWithStatus,struct std::default_delete<class mojo::MessageReceiverWithStatus> >)
        7376            1  public: static bool __cdecl blink::mojom::WebBluetoothServiceStubDispatch::AcceptWithResponder(class blink::mojom::WebBluetoothService * __ptr64,class mojo::Message * __ptr64,class std::unique_ptr<class mojo::MessageReceiverWithStatus,struct std::default_delete<class mojo::MessageReceiverWithStatus> >)
        5150            1  content::`anonymous namespace'::GpuInfoAsDictionaryValue
        4944            1  private: virtual void __cdecl content::AccessibilityTreeFormatterWin::AddProperties(class content::BrowserAccessibility const & __ptr64,class base::DictionaryValue * __ptr64) __ptr64
        4919            1  public: virtual bool __cdecl content::WebContentsImpl::OnMessageReceived(class content::RenderFrameHostImpl * __ptr64,class IPC::Message const & __ptr64) __ptr64
        4833            1  public: virtual bool __cdecl content::RenderWidgetHostImpl::OnMessageReceived(class IPC::Message const & __ptr64) __ptr64
        4816            1  private: class gfx::Rect __cdecl cc::SurfaceAggregator::PrewalkTree(class cc::SurfaceId const & __ptr64,bool,int,struct cc::SurfaceAggregator::PrewalkResult * __ptr64) __ptr64
        4782            1  public: virtual bool __cdecl content::FileAPIMessageFilter::OnMessageReceived(class IPC::Message const & __ptr64) __ptr64
        4750            1  YUY2ToARGBRow_C
        4735            1  public: static bool __cdecl blink::mojom::PresentationServiceStubDispatch::Accept(class blink::mojom::PresentationService * __ptr64,class mojo::Message * __ptr64)
        4565            1  public: static bool __cdecl ui::mojom::GpuHostStubDispatch::Accept(class ui::mojom::GpuHost * __ptr64,class mojo::Message * __ptr64)
        4339            1  public: virtual void __cdecl cc::FragmentShader::Init(class gpu::gles2::GLES2Interface * __ptr64,unsigned int,int * __ptr64) __ptr64
        4047            1  private: void __cdecl content::RenderProcessHostImpl::CreateMessageFilters(void) __ptr64
        4020            1  public: static bool __cdecl blink::mojom::BackgroundFetchServiceStubDispatch::AcceptWithResponder(class blink::mojom::BackgroundFetchService * __ptr64,class mojo::Message * __ptr64,class std::unique_ptr<class mojo::MessageReceiverWithStatus,struct std::default_delete<class mojo::MessageReceiverWithStatus> >)
        3995            1  UYVYToARGBRow_C
        3878            1  NV12ToARGBRow_C

Searching for these symbols can confirm whether they grew or are brand new. content::`anonymous namespace'::AccessibilityEnumMap::AccessibilityEnumMap, for instance, appears to be brand new.
Ditto with public: static bool __cdecl indexed_db::mojom::DatabaseStubDispatch::Accept(class indexed_db::mojom::Database * __ptr64,class mojo::Message * __ptr64).
Ditto with YUY2ToARGBRow_C.

Looking at the increase by individual symbols instead of by directories seems to work better. And, it gives an actionable question: find one of the symbols that should not have been pulled in and then use tools\win\linker_verbose_tracking.py (documented at https://www.chromium.org/developers/windows-binary-sizes) to figure out why. Repeat until all mysteries are solved.

Thanks Bruce for the pointers!

I'm not sure though if I'm using the tool properly. 
I generated the link info (around 1.4GB) and did the following: 
-  AccessibilityEnumMap::AccessibilityEnumMap, defined in https://cs.chromium.org/chromium/src/content/browser/accessibility/accessibility_tree_formatter_utils_win.cc?l=47

I executed the following:
 python .\tools\win\linker_verbose_tracking.py C:\tmp\chrome.child.link.txt accessibility_tree_formatter_utils_win.obj

==> And got no results. I did notice that the objet was mention in the linker output

- mojom::DatabaseStubDispatch::Accept. I executed: 
python .\tools\win\linker_verbose_tracking.py C:\tmp\chrome.child.link.txt indexed_db.mojom.obj
==> No results


Am I doing this correctly?

It looks like the trailing ".obj" is supposed to be emitted. I meant to add a check for that but I guess I never did. Try this:

python .\tools\win\linker_verbose_tracking.py C:\tmp\chrome.child.link.txt accessibility_tree_formatter_utils_win

If that doesn't work or if the results are inscrutable I can certainly take a look.

It does omit the .obj. The full output is: 
python .\tools\win\linker_verbose_tracking.py C:\tmp\chrome.child.link.txt accessibility_tree_formatter_utils_win
Database loaded - 0 xrefs found
No references to accessibility_tree_formatter_utils_win.obj found.

Perhaps I'm missing a flag for the verbose linker. I've attached all the lines that match 'accessibility_tree_formatter_utils_win.obj' in my output.
formatter_utils_win.txt
88.2 KB View Download
I *think* that what that means is that that file is specified on the command line, probably as part of a source set. The linker is pulling it in because you explicitly told it to link that file. That is the problem with source sets - the fully link all of the files contained within, and then we have to hope that the linker can discard what is not referenced later in the process.

The .obj files in .lib files, in contrast, are pulled in on demand (which causes a different set of issues).

Ping me if you want to discuss or I can try to reproduce your results, but it looks like the problem is that a source_set is being pulled in and the linker is unable to discard all of its contents, even though they are not logically needed. I can tweak the tool to make that clearer - you're really only the second user :-)
0 xrefs found seems wrong - I think that means that the script didn't find anything in the verbose output. Please share it with me and I'll take a look.
I've put it on https://x20.corp.google.com/users/dv/dvallet/chrome.child.link.txt.zip

Yeah I'll look a bit more into it. But maybe I need more pointers on how to hunt for the source set
I looked around a bit and here's what I did/found. First, I turned on verbose linking for chrome_child.dll:

diff --git a/chrome/BUILD.gn b/chrome/BUILD.gn
index a959bfb..c80a8ea 100644
--- a/chrome/BUILD.gn
+++ b/chrome/BUILD.gn
@@ -464,6 +464,7 @@ if (is_win) {
         "/DELAYLOAD:dxva2.dll",
         "/DELAYLOAD:esent.dll",
         "/DELAYLOAD:wininet.dll",
+        "/VERBOSE",
       ]

       if (symbol_level == 2) {

I ran your command and note that it said 12120 xrefs found. So something is wrong with your verbose linker output (since it shows zero). I don't know what it would be.

> python .\tools\win\linker_verbose_tracking.py chrome_child_00_initial.txt accessibility_tree_formatter_utils_win
Database loaded - 12120 xrefs found
No references to accessibility_tree_formatter_utils_win.obj found.

The "no references" message means that the script didn't find a symbol that caused this object file to be pulled in. So either it wasn't linked at all (we know that's not true) or else it was linked directly - i.e.; in a source_set. A quick search of *.gn* for accessibility_tree_formatter_utils_win.cc finds that it is in source_set("browser"). Okay. So it is pulled in because we explicitly requested it.

I changed "browser" to a static library and tried again:

>python tools\win\linker_verbose_tracking.py chrome_child_01_browser_to_static_library.txt accessibility_tree_formatter_utils_win
Database loaded - 12120 xrefs found
accessibility_tree_formatter_utils_win.obj pulled in for symbol "class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> > __cdecl content::AccessibilityEventToString(int)" (?AccessibilityEventToString@content@@YA?AV?$basic_string@_WU?$char_traits@_W@std@@V?$allocator@_W@2@@std@@H@Z" by
        browser_accessibility_event_win.obj

browser_accessibility_event_win.obj pulled in for symbol "public: static class content::BrowserAccessibilityEvent * __cdecl content::BrowserAccessibilityEvent::Create(enum content::BrowserAccessibilityEvent::Source,enum ui::AXEvent,class content::BrowserAccessibility const *)" (?Create@BrowserAccessibilityEvent@content@@SAPEAV12@W4Source@12@W4AXEvent@ui@@PEBVBrowserAccessibility@2@@Z" by
        browser_accessibility_manager.obj

browser_accessibility_manager.obj pulled in for symbol "public: void __cdecl content::BrowserAccessibilityManager::UserIsNavigatingAway(void)" (?UserIsNavigatingAway@BrowserAccessibilityManager@content@@QEAAXXZ" by
        web_contents_impl.obj

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


So now accessibility_tree_formatter_utils_win is still being pulled in, but indirectly. "Command-line obj file" means that guest_mode.obj was specified directly - probably another source_set. In this case source_set("browser_sources"). So I changed that to a static_library and tried again:

>python tools\win\linker_verbose_tracking.py chrome_child_02_browser_sources_to_static_library.txt accessibility_tree_formatter_utils_win
Database loaded - 12155 xrefs found
accessibility_tree_formatter_utils_win.obj pulled in for symbol "class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> > __cdecl content::AccessibilityEventToString(int)" (?AccessibilityEventToString@content@@YA?AV?$basic_string@_WU?$char_traits@_W@std@@V?$allocator@_W@2@@std@@H@Z" by
        browser_accessibility_event_win.obj

browser_accessibility_event_win.obj pulled in for symbol "public: static class content::BrowserAccessibilityEvent * __cdecl content::BrowserAccessibilityEvent::Create(enum content::BrowserAccessibilityEvent::Source,enum ui::AXEvent,class content::BrowserAccessibility const *)" (?Create@BrowserAccessibilityEvent@content@@SAPEAV12@W4Source@12@W4AXEvent@ui@@PEBVBrowserAccessibility@2@@Z" by
        browser_accessibility_manager.obj

browser_accessibility_manager.obj pulled in for symbol "public: void __cdecl content::BrowserAccessibilityManager::UserIsNavigatingAway(void)" (?UserIsNavigatingAway@BrowserAccessibilityManager@content@@QEAAXXZ" by
        web_contents_impl.obj

web_contents_impl.obj pulled in for symbol "public: static class content::WebContents * __cdecl content::WebContents::Create(struct content::WebContents::CreateParams const &)" (?Create@WebContents@content@@SAPEAV12@AEBUCreateParams@12@@Z" by
        headless_web_contents_impl.obj

headless_web_contents_impl.obj pulled in for symbol "public: __cdecl headless::HeadlessWebContents::Builder::~Builder(void)" (??1Builder@HeadlessWebContents@headless@@QEAA@XZ" by
        headless_shell.obj

headless_shell.obj pulled in for symbol "int __cdecl headless::HeadlessShellMain(struct content::ContentMainParams const &)" (?HeadlessShellMain@headless@@YAHAEBUContentMainParams@content@@@Z" by
        Command-line obj file: chrome_main.obj


And that seems to explain things, or at least some things. headless_shell.obj pulls in headless_web_contents_impl.obj which pulls in web_contents_impl.obj which pulls in browser_accessibility_manager.obj which pulls in browser_accessibility_event_win.obj which pulls in accessibility_tree_formatter_utils_win.obj.

I uploaded my changes plus your CL to crrev.com/2858113005.

My build settings are:
is_component_build = false
is_debug = false
target_cpu = "x64"

Note that the two static_library changes shrink chrome_child.dll by about 5 KB. Not enough to be interesting, but sometimes multiple changes are needed in order to finally trigger the big savings.

BTW, the reason the script was finding nothing for the author of the CL is because their verbose output is UTF16 and my script assumes ASCII or utf-8 or something like that. I'll land some changes to improve the diagnostics and maybe handle that.
Thanks for the pointers Bruce!
I investigated the next symbol: 

python .\tools\win\linker_verbose_tracking.py C:\tmp\chrome.child.link.2.txt indexed_db.mojom
Database loaded - 12153 xrefs found
No references to indexed_db.mojom.obj found. Directly specified in sources or a source_set?

Which gets referenced in 
https://cs.chromium.org/chromium/src/out/Debug/gen/content/common/indexed_db/indexed_db.mojom.cc

A wild guess is that is generated in a here: https://cs.chromium.org/chromium/src/content/common/BUILD.gn?rcl=84b8a5f1c2f1c62aadd4c943f3f093860cdc470d&l=576

mojom("mojo_bindings")

=> Checking this doc: https://docs.google.com/document/d/1pLsOZ2PQ_FNZHx_U94ZJprDfGLQUgUkCssCoyC9TVVY/edit#heading=h.pl8mmvgw8xx7 
It suggests that mojom targets are actually source sets, so that might be a problem for this build.

I'll drop an email to the mojo team to see if that's the case and if it can be solved. If not I guess I'll move on to the next symbol




Another symbol: 
 python .\tools\win\linker_verbose_tracking.py C:\tmp\chrome.child.link.txt webui_resources_map
Database loaded - 12242 xrefs found
No references to webui_resources_map.obj found. Directly specified in sources or a source_set?

webui_resource_map is declared here as a grit resource: https://cs.chromium.org/chromium/src/ui/resources/BUILD.gn?rcl=a11f72c868235d4673cd96c7eea24ad9e7eb4b55&l=36

This I assume is part of webui_resources.pak which is repacked in headless: https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=04bee6a1d135e6665c04d1e748cf7baeb6769788&l=28
And added in headless component. However I have commented out adding the pak dependency here https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=04bee6a1d135e6665c04d1e748cf7baeb6769788&l=360 and the object is still pulled for some reason


Continuing Bruce investigation on #c37:

The issue here is that headless_shell pulls a dependency to web_contents_impl

this dependency doesn't exist in the current child_dll:
 python .\tools\win\linker_verbose_tracking.py C:\tmp\chrome.child.link.release.txt web_contents_impl                                                 

Database loaded - 11504 xrefs found                                                                                                                          No references to web_contents_impl.obj found. Directly specified in sources or a source_set?  

Note that it is mentioned directly, but not pulled indirectly like headless library. 

My question are: 
- Do we really need to have a dependency to WebContents in headless child, given that this is not necessary for standard Chrome. 
- Do removing the indirect dependency result in a (significant) decrease in size in DLL. 

The problem is that this doesn't seem to be straightforward with the current headless implementation, and it might need significant regfractoring work.

Re: mojo, there's no way to use the headless mojo bindings in runtime mode, so we could add a build time check that avoids compiling them in. I'm not sure why the linker doesn't realize this but maybe it isn't trivial.

Only the browser part of headless should be using WebContents -- it's not even available in the renderer. Do you know which object is pulling it into the child dll? Are we just splitting headless/headless_child in the wrong place?
Project Member Comment 44 by bugdroid1@chromium.org, May 5 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3647b59ef9a6cf01783c7cf7b5ea83eab3296007

commit 3647b59ef9a6cf01783c7cf7b5ea83eab3296007
Author: brucedawson <brucedawson@chromium.org>
Date: Fri May 05 22:40:30 2017

Improve linker_verbose_tracking.py

linker_verbose_tracking.py is used to analyze the verbose output from
the VC++ linker to help understand binary-size regressions. However some
of its messages are clear only to the original author, and it completely
fails on the utf-16 files created when redirecting the linker output
from powershell.

This change updates the script to handle both utf-16 and utf-8 files,
and improves the messages printed in a couple of cases.

R=dvallet@chromium.org
BUG= 714841 

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

[modify] https://crrev.com/3647b59ef9a6cf01783c7cf7b5ea83eab3296007/tools/win/linker_verbose_tracking.py

Thanks for your thoughts Bruce & Sami!

So I continued looking into why is WebContents is pulled in and if this is the main cause of grief for the size regression. 

So, it turns out that if I put the code in HeadlessShell::OnStart and  HeadlessContentMainDelegate::RunProcess behind an #if !defined(CHROME_MULTIPLE_DLL_CHILD) block. I can verify that WebContentsImpl is not pulled in indirectly anymore and the dll size goes from 75MB to 67MB in component build, which I think it is almost all the size regression problem.

I'll work tomorrow on a working patch and verifying on an official build, but I'd say we're close to the solution.
SO I got a working patch and did an oficial build. This is the current difference in size: 
ls .\out\Release\chrome_child.dll
-a----        5/10/2017   3:32 PM       71081472 chrome_child.dll
ls .\out\ReleaseHeadless\chrome_child.dll
-a----        5/10/2017  12:53 PM       71152640 chrome_child.dll
ls .\out\Release\chrome.dll
-a----        5/10/2017   3:21 PM       50447360 chrome.dll
ls .\out\ReleaseHeadless\chrome.dll
-a----        5/10/2017  12:25 PM       50616832 chrome.dll

So that's roughly ~70KB increase in chrome_child_dll and ~165KB increase in chrome_dll. 

This seems in line with the ~150KB increase in Linux headless. I'll add the relevant people to the reland cl to double check.
Comment 47 by wfh@chromium.org, May 12 2017
those sizes seem a bit off, I get from examining an official build, in this case, 60.0.3097.0:

32-bit
chrome.dll - 34,992,472
chrome_child.dll - 47,093,080

64-bit
chrome.dll - 49,562,968
chrome_child.dll - 61,548,888

is the 70kb and 165kb increase on your sizes - which would make it a 0.1% and 0.3% increase, or on the sizes from the official builders, which would make it a 0.15% and 0.48% increase?

0.5% is quite high, but I suppose within the realms of acceptable (I'm not the final decider on this anyway, just providing more data).
Comment 48 by h...@chromium.org, May 12 2017
> those sizes seem a bit off, I get from examining an official build, in this case, 60.0.3097.0:

wfh: when you say official, do you really mean PGO?

This situation where the builds we ship (PGO) don't match the official build config is seriously confusing. (crbug.com/673025)

dvallet: How did you configure your official build?
Comment 49 by wfh@chromium.org, May 12 2017
>wfh: when you say official, do you really mean PGO?

yes, I mean the builds from the official build archive, which are PGO... yes I agree it's quite confusing, I don't think it's possible to actually self-generate the same type of builds that we ship to users - I starred that bug :)

FWIW - I don't think that a 0.5% size regression necessarily blocks this CL from landing - I think we can land and then examine the real size diffs on the build archive as well...
My build config is: 
is_chrome_branded = true
is_debug = false
is_official_build = true

+1 for landing and checking the real size digg
Owner: dvallet@chromium.org
Status: Assigned
Looks like the real size increase for chrome_child.dll was indeed 0.1% for chrome_child and 0.3% for chrome.dll.
There's also a ~0.15% increase in mini_installer.exe

https://chromeperf.appspot.com/report?sid=fcb4b715aa409ce28f65cb5c3afed7a56c89cb910a7a0b81e41ad0c785892b7d&start_rev=471498&end_rev=471573

This hasn't triggered any alert.

I don't think expressing size deltas as percentages is very useful; chrome is large enough that even fairly large size increases look like a small percentage.

What's headless -- which capabilities does it add, and what code does it pull in that isn't in chrome already? I'm guessing it went through launch review, and this size increase is in line with the estimate that was approved there?
Sorry, I'll add more context:
We are adding a headless run-time environment for Chrome, running with the flag --headless. This pulls a new library from src/headless

Overall launch bug: https://bugs.chromium.org/p/chromium/issues/detail?id=705916
Bug for --headless flag in Linux: https://bugs.chromium.org/p/chromium/issues/detail?id=612904. You can see in this design doc that the impact in linux was ~150KB, but up to ~600KB was deemed accectable:
https://docs.google.com/document/d/1aIJUzQr3eougZQp90bp4mqGr5gY6hdUice8UPa-Ys90/edit#heading=h.qxqfzv2lj12s


The absolute increase is 160KB for chrome.dll and 70KB for chrome_child.dll
If nobody has strong feelings against it I'm inclined to close this bug. 
Thanks everybody for their help!

Comment 56 by phistuck@gmail.com, May 15 2017
#55 - perhaps you should file a new issue for the "(and nobody noticed?)" part?
Status: Fixed
Let's close this bug. I'll file a new bug for the "nobody noticed" part.
Filed  bug 722400  for monitoring.
Sign in to add a comment