New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 636468 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocked on:
issue 627216



Sign in to add a comment

browser_tests taking an exceedingly long time to link on Windows

Project Member Reported by scottmg@chromium.org, Aug 10 2016

Issue description

At the moment, it takes a very long time to link browser_tests after even only trivial changes.

[safe-browsing-test]d:\src\cr3\src>tim ninja -C out\Debug browser_tests
ninja: Entering directory `out\Debug'
[0->2/2 ~0] LINK browser_tests.exe browser_tests.exe.pdb
LINK : ./browser_tests.exe not found or not built by the last incremental link; performing full link

real: 6m8.797s

Update a chrome/browser file, and rebuild:

[safe-browsing-test]d:\src\cr3\src>tim ninja -C out\Debug browser_tests
ninja: Entering directory `out\Debug'
[0->5/5 ~0] LINK browser_tests.exe browser_tests.exe.pdb

real: 6m32.359s


This is nuuuuuuuts! Too much linking make Homer something something.


One of the problems is that until 2015 Update 3 final, incremental linking is just broken in common cases. Turning on /verbose:incr and linking browser_tests results in this message (with a trivial touch of thread_hop_resource_throttle.cc):

[timing]d:\src\cr3\src>ninja -C out\Debug browser_tests
ninja: Entering directory `out\Debug'
[1->4/5 ~0] LINK(DLL) chrome.dll chrome.dll.lib chrome.dll.pdb
LibDef: Total time = 0.000s
Pass 1: Interval #1, time = 55.344s
  Wait PDB close: Total time = 6.797s
  Wait type merge: Total time = 0.031s
Pass 2: Interval #2, time = 76.562s
Final: Total time = 131.906s
[0->5/5 ~0] LINK browser_tests.exe browser_tests.exe.pdb
LINK : symbol '"public: virtual void * __thiscall ThreadHopResourceThrottle::`vector deleting destructor'(unsigned int)" (??_EThreadHopResourceThrottle@@UAEPAXI@Z)' was previously strong but is now weak; performing full link
LibDef: Total time = 0.000s
Pass 1: Interval #1, time = 103.969s
  Wait PDB close: Total time = 5.640s
  Wait type merge: Total time = 0.016s
Pass 2: Interval #2, time = 135.406s
Final: Total time = 239.375s
Final: Total time = 241.469s

That seems to be fixed in final U3 https://connect.microsoft.com/VisualStudio/feedback/details/2565201/incremental-linking-doesnt-work-when-library-has-static-instance-of-class-with-virtual-destructor

Note that the prerelease U3 we have packaged currently isn't sufficient: https://cs.chromium.org/chromium/src/build/vs_toolchain.py?rcl=0&l=308 so we need to repackage that.

More updates coming (having some computer trouble but I'll update with more timings)


 
https://crrev.com/ccb02e5b095c62d0a799b40a40f9a7ebebb8298d adds a GN bool "win_linker_timing" which turns on a bit of timing spam for links, but is really helpful to see why incremental links are failing to be incremental and are instead redonkulously slow.
Blockedon: 627216
Confirmed that we can keep the .lib for browser, etc. on 2015 (incremental linking for objs changing inside .libs was fixed from between 2013 and 2015).

So, when linking with the final U3 and a small change to chrome/BUILD.gn, chrome.dll is down to 6s from 131s, and browser_tests (which includes chrome.dll) is down to 20s from 6m8s. Thank goodness!

[timing]d:\src\cr3\src>tim ninja -C out\Debug browser_tests
ninja: Entering directory `out\Debug'
[1->4/5 ~0] LINK(DLL) chrome.dll chrome.dll.lib chrome.dll.pdb
IncrPass2: Interval #1, time = 0.875s
  Wait PDB close: Total time = 0.109s
IncrPass2: Interval #2, time = 4.984s
Final: Total time = 5.859s
[0->5/5 ~0] LINK browser_tests.exe browser_tests.exe.pdb
IncrPass2: Interval #1, time = 1.219s
  Wait PDB close: Total time = 0.109s
IncrPass2: Interval #2, time = 5.219s
Final: Total time = 6.438s

real: 0m19.750s
https://codereview.chromium.org/2239533002/. Bruce is in the process of packaging Update 3 for depot_tools in  issue 627216  which is all we need after that CL lands.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 11 2016

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

commit 3a480c53288755252ea84683e6166e1619d85bb1
Author: scottmg <scottmg@chromium.org>
Date: Thu Aug 11 04:07:22 2016

Don't disable incremental linking for chrome.dll in component

R=brettw@chromium.org
BUG= 636468 

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

[modify] https://crrev.com/3a480c53288755252ea84683e6166e1619d85bb1/chrome/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 12 2016

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

commit 28a2bfe1e7db4c74017c86f5b8ba9ec2b61c238e
Author: scottmg <scottmg@chromium.org>
Date: Fri Aug 12 21:49:07 2016

Enable incremental linking for unit_tests on win component

browser_tests seems ok so far, and unit_tests is a bit smaller currently.

R=brettw@chromium.org
BUG= 636468 

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

[modify] https://crrev.com/28a2bfe1e7db4c74017c86f5b8ba9ec2b61c238e/chrome/test/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 8 2016

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

commit 02343c661ee0482cb73ad0cf861999f2c57eeb11
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Sep 08 01:53:00 2016

Switch to VS 2015 Update 3

VS 2015 Update 3 offers numerous improvements such as improved
language conformance, fixes to avoid linker crashes, and
improved code-gen for faster performance. Update 3 also resolves
some problems with incremental linking silently falling back to
a full link.

It also fixes an internal compiler error in /analyze builds.

The landmines change is needed because otherwise things like the
out\Release\cdb directory do not get regenerated.

The process for creating this package is:
- Create a clean Windows VM running Windows Server 2012 R2 (en_windows_server_2012_r2_vl_with_update_x64_dvd_6052766.iso, SHA1: 247EAEE5628850A41F0C51471656AAFB2492EA08, Standard Edition)
- Install depot tools, run ‘gclient’, and add depot_tools to the path
- Install VS 2015 Professional Update 3 - must have been installed after July 12th to get the latest fixes - with these settings and nothing else:
 * Visual C++ and make sure the three nodes underneath are also selected
 * Under Universal Windows App Development Tools make sure the "Tools (1.4.1) and Windows 10 SDK (10.0.14393)" and the "Windows 10 SDK (10.0.10586)" nodes are selected and nothing else
- Copy the *Debugger and Tools*.msi installers from another machine and run them. The 10.0.14393 versions (Anniversary Edition) should be used.
- Then run:
    python depot_tools\win_toolchain\package_from_installed.py 2015

It is also possible to package the 14393 SDK by running this command,
but that change is being saved for later:
    python depot_tools\win_toolchain\package_from_installed.py 2015 -w 10.0.14393.0

BUG= 627216 , 636468 , 427616 , 637456 

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

[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/get_landmines.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/vs_toolchain.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/win/copy_cdb_to_output.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/tools/perf/chrome_telemetry_build/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 8 2016

Labels: merge-merged-2854
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02343c661ee0482cb73ad0cf861999f2c57eeb11

commit 02343c661ee0482cb73ad0cf861999f2c57eeb11
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Sep 08 01:53:00 2016

Switch to VS 2015 Update 3

VS 2015 Update 3 offers numerous improvements such as improved
language conformance, fixes to avoid linker crashes, and
improved code-gen for faster performance. Update 3 also resolves
some problems with incremental linking silently falling back to
a full link.

It also fixes an internal compiler error in /analyze builds.

The landmines change is needed because otherwise things like the
out\Release\cdb directory do not get regenerated.

The process for creating this package is:
- Create a clean Windows VM running Windows Server 2012 R2 (en_windows_server_2012_r2_vl_with_update_x64_dvd_6052766.iso, SHA1: 247EAEE5628850A41F0C51471656AAFB2492EA08, Standard Edition)
- Install depot tools, run ‘gclient’, and add depot_tools to the path
- Install VS 2015 Professional Update 3 - must have been installed after July 12th to get the latest fixes - with these settings and nothing else:
 * Visual C++ and make sure the three nodes underneath are also selected
 * Under Universal Windows App Development Tools make sure the "Tools (1.4.1) and Windows 10 SDK (10.0.14393)" and the "Windows 10 SDK (10.0.10586)" nodes are selected and nothing else
- Copy the *Debugger and Tools*.msi installers from another machine and run them. The 10.0.14393 versions (Anniversary Edition) should be used.
- Then run:
    python depot_tools\win_toolchain\package_from_installed.py 2015

It is also possible to package the 14393 SDK by running this command,
but that change is being saved for later:
    python depot_tools\win_toolchain\package_from_installed.py 2015 -w 10.0.14393.0

BUG= 627216 , 636468 , 427616 , 637456 

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

[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/get_landmines.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/vs_toolchain.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/build/win/copy_cdb_to_output.py
[modify] https://crrev.com/02343c661ee0482cb73ad0cf861999f2c57eeb11/tools/perf/chrome_telemetry_build/BUILD.gn

Status: Fixed (was: Started)

Sign in to add a comment