Add /Gw to the compiler command line to reduce .dll sizes by 0.85%

Add /Gw to the compiler command line to reduce .dll sizes by 0.85%

/Gw is a new command added in VS 2013 Update 2 that puts individual
global variables in their own COMDATs so that /opt:ref can more
efficiently throw them out. Size savings with buildtype=Official are
shown below:

DLL Name            Old size     New size  Bytes Saved  Percentage
chrome.dll         30,918,656   30,766,592  152,064       0.49%
chrome_child.dll   35,204,096   34,797,056  407,040       1.16%
chrome_elf.dll        125,952      124,928    1,024       0.81%
chrome_watcher.dll     216,576     213,504    3,072       1.42%
pdf.dll              9,200,128   9,162,240   37,888       0.41%
Total               75,665,408  75,064,320  601,088       0.79%

Some of the saving is from getting rid of unused duplicate copies
of const globals declared in header files. twoPiDouble and piDouble,
for instance, were showing up 2,051 and 1,408 times respectively.

This fix covers most or all of the problems covered by bug 441988
as well as other wastage.

BUG=441988
Committed: https://crrev.com/145c9c07b8fe42e0eb6cf997c67070fc62dd241e
Cr-Commit-Position: refs/heads/master@{#309067}

[brucedawson, 2014-12-18 02:02:04]

brucedawson@chromium.org changed reviewers:
+ scottmg@chromium.org

[brucedawson, 2014-12-18 02:02:04]

Scott, this looks like an easy change to shave off 0.85% (550 KiB) of size from
the chrome*.dll binaries. It requires VS 2013 Update 2.

Currently the patch has the change made in two places, with the common.gypi
location being slightly more effective. Any thoughts? Presumably the other
location (release_defaults.gypi) missed a few.cpp files -- I want them all.

[scottmg, 2014-12-18 03:05:38]

Cool! I haven't used this flag before. My only concern would be if there's any
drawback in link times, or substantial increase for regular Release (i.e.
non-LTCG builds).

https://codereview.chromium.org/812263002/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/812263002/diff/1/build/common.gypi#newcode3392
build/common.gypi:3392: '/Gw',  # Put data in separate COMDATs, new in VS 2013
Update 2
You can drop the ", new in VS 2013 Update 2", and put a '.' at the end of
comments.

[brucedawson, 2014-12-18 17:58:07]

As a bonus, this flag mostly resolves the issues mentioned in bug
https://code.google.com/p/chromium/issues/detail?id=441988. Without /gW there
are ~2,000 copies of twoPiDouble and twoPiFloat (among others) but after this
change there are just five copies of twoPiDouble and zero? of twoPiFloat.

Tests show that the change to common.gypi gives the full benefit (at least for
chrome.dll) so I am going to remove the change from release_defaults.gypi. I
don't know why release_defaults.gypi doesn't hit all object files, but oh well.

Note that pdf.dll is shrunk by 37 KiB, bring the total savings to 587 KiB.

I don't anticipate a slowdown in normal release-build linking. I could do some
overnight measurements.

[brucedawson, 2014-12-18 18:13:13]

I fixed the comment, simplified the change, and added measurements for pdf.dll.

Let me know if you think I should try measuring the link times for normal
builds.

[scottmg, 2014-12-18 18:21:05]

On 2014/12/18 18:13:13, brucedawson wrote:
> I fixed the comment, simplified the change, and added measurements for
pdf.dll.
> 
> Let me know if you think I should try measuring the link times for normal
> builds.

LGTM. Just keep an eye on some of the cycle times (e.g.
http://build.chromium.org/p/chromium/stats/Win under 'compile') after it lands
to make sure there's no upward trend.

Thanks!

[scottmg, 2014-12-18 18:21:19]

Also, was there a BUG= to tag this with?

[brucedawson, 2014-12-18 18:58:43]

Duplicate static/const variable bug added.

[brucedawson, 2014-12-18 18:59:54]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2014-12-18 19:00:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/812263002/20001

[commit-bot: I haz the power, 2014-12-18 20:36:14]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-12-18 20:36:58]

Patchset 2 (id:??) landed as
https://crrev.com/145c9c07b8fe42e0eb6cf997c67070fc62dd241e
Cr-Commit-Position: refs/heads/master@{#309067}

[Nico, 2014-12-29 19:47:15]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2014-12-29 19:47:16]

(Belated nit: I always think it's somewhat dishonest to express size changes in
terms of percentages. Since chrome is so chubby, percentages almost always look
pretty small, even if the change is fairly noticeable in magnitude. Most people
who use percentages to try to convince me that their binary size increase isn't
so bad though, and don't use it to undersell their size savings ;-) )

[brucedawson, 2014-12-29 19:49:45]

Interesting perspective Nico. I'm a huge fan of using percentages to describe
improvements since it avoids overselling, but I hadn't considered that the other
side of that coin is using them to downplay regressions.