build: Enable optimize_for_size unconditionally.

build: Enable optimize_for_size unconditionally.

This change causes us to favor size over speed on Linux and Mac,
which aligns the build config for those platforms with that of the
other supported platforms, and should reduce the binary size impact
of enabling ThinLTO. This change is expected to reduce binary size
for Linux official builds by about 15%.

There may be unacceptable perf regressions associated with this
change, but the perf bots should at least let us know where those
regressions are. I plan to monitor the Linux and Mac perf bots once
it lands.

BUG=660216
R=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2864383003
Cr-Commit-Position: refs/heads/master@{#470606}
Committed: https://chromium.googlesource.com/chromium/src/+/c1269ce7fec8568a1789e07b2b5bb3b630a67f5f

[pcc1, 2017-05-08 22:32:20]

Let me know if there are any perf teams that need to be FYI'd here.

[pcc1, 2017-05-08 22:34:12]

The CQ bit was checked by pcc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-08 22:36:48]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 00:20:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-09 00:20:53]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[pcc1, 2017-05-09 00:41:29]

The cast_shell_linux trybot failures appear to be legitimate; taking a look.

[pcc1, 2017-05-09 01:31:45]

Looks like the root cause is crbug.com/576197. If I disable ICF or if I link
with lld (which has a fix for the same bug: https://reviews.llvm.org/D23732) the
problem goes away. 

I imagine that the same thing that's blocking us from rolling clang would also
block us from rolling binutils (?), but I'll see if I can fix this in our copy
of binutils.

[Nico, 2017-05-09 02:18:56]

On 2017/05/09 01:31:45, pcc1 wrote:
> Looks like the root cause is crbug.com/576197. If I disable ICF or if I link
> with lld (which has a fix for the same bug: https://reviews.llvm.org/D23732)
the
> problem goes away. 

Ivan was working on switching us to lld, if you want you can do that. I think
it's already bundled, we just need to gather some link time numbers, flip the
switch, and monitor.

> I imagine that the same thing that's blocking us from rolling clang would also
> block us from rolling binutils (?), but I'll see if I can fix this in our copy
> of binutils.

I think Hans said goma team is back this week. I don't remember if binutils
needs to be pushed to goma, but it's possible.

This change here lgtm once the section alignment icf thing is figured out one
way or the other.

[pcc1, 2017-05-09 02:29:09]

On 2017/05/09 02:18:56, Nico wrote:
> On 2017/05/09 01:31:45, pcc1 wrote:
> > Looks like the root cause is crbug.com/576197. If I disable ICF or if I link
> > with lld (which has a fix for the same bug: https://reviews.llvm.org/D23732)
> the
> > problem goes away. 
> 
> Ivan was working on switching us to lld, if you want you can do that. I think
> it's already bundled, we just need to gather some link time numbers, flip the
> switch, and monitor.

I was planning to do that once https://codereview.chromium.org/2848113002/
lands. I also considered doing lld first (for non-LTO builds), but that doesn't
help us flip the optimize_for_size flag for LTO builds, and I wanted to avoid
having too many serial dependencies.

> > I imagine that the same thing that's blocking us from rolling clang would
also
> > block us from rolling binutils (?), but I'll see if I can fix this in our
copy
> > of binutils.
> 
> I think Hans said goma team is back this week. I don't remember if binutils
> needs to be pushed to goma, but it's possible.

I believe so, see end of third_party/binutils/upload.sh

[pcc1, 2017-05-09 17:36:58]

The CQ bit was checked by pcc@chromium.org

[commit-bot: I haz the power, 2017-05-09 17:37:55]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pcc1, 2017-05-09 17:38:25]

On 2017/05/09 02:18:56, Nico wrote:
> This change here lgtm once the section alignment icf thing is figured out one
> way or the other.

For the record, I landed a fix for the gold bug in
https://codereview.chromium.org/2870673004/

[commit-bot: I haz the power, 2017-05-10 00:49:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 00:49:34]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[pcc1, 2017-05-10 04:37:15]

The CQ bit was checked by pcc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 04:37:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 08:36:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 08:36:56]

Dry run: This issue passed the CQ dry run.

[pcc1, 2017-05-10 16:31:54]

The CQ bit was checked by pcc@chromium.org

[commit-bot: I haz the power, 2017-05-10 16:34:37]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 16:41:35]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494433914062780, "parent_rev":
"ea6cba18fb2e07477a7992cc027677f37a9b772e", "commit_rev":
"c1269ce7fec8568a1789e07b2b5bb3b630a67f5f"}

[commit-bot: I haz the power, 2017-05-10 16:41:44]

Description was changed from

==========
build: Enable optimize_for_size unconditionally.

This change causes us to favor size over speed on Linux and Mac,
which aligns the build config for those platforms with that of the
other supported platforms, and should reduce the binary size impact
of enabling ThinLTO. This change is expected to reduce binary size
for Linux official builds by about 15%.

There may be unacceptable perf regressions associated with this
change, but the perf bots should at least let us know where those
regressions are. I plan to monitor the Linux and Mac perf bots once
it lands.

BUG=660216
R=thakis@chromium.org
==========

to

==========
build: Enable optimize_for_size unconditionally.

This change causes us to favor size over speed on Linux and Mac,
which aligns the build config for those platforms with that of the
other supported platforms, and should reduce the binary size impact
of enabling ThinLTO. This change is expected to reduce binary size
for Linux official builds by about 15%.

There may be unacceptable perf regressions associated with this
change, but the perf bots should at least let us know where those
regressions are. I plan to monitor the Linux and Mac perf bots once
it lands.

BUG=660216
R=thakis@chromium.org

Review-Url: https://codereview.chromium.org/2864383003
Cr-Commit-Position: refs/heads/master@{#470606}
Committed:
https://chromium.googlesource.com/chromium/src/+/c1269ce7fec8568a1789e07b2b5b...
==========

[commit-bot: I haz the power, 2017-05-10 16:41:45]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/c1269ce7fec8568a1789e07b2b5b...

[yhirano, 2017-05-11 03:20:43]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2881503002/ by yhirano@chromium.org.

The reason for reverting is: Causes failures on a MSAN bot.
https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20Chrom...
.