Revert of Enable LTO on Linux for the official builds.

Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of https://codereview.chromium.org/2003733004/ )

Reason for revert:
Breaks android x64 builds.
https://bugs.chromium.org/p/chromium/issues/detail?id=615039

Original issue's description:
> Enable LTO on Linux for the official builds.
>
> This CL turns on Link-Time Optimization for official
> builds, which makes the resulting binaries to run faster.
>
> On the dark side, the change increases the link time by 3x-5x,
> and is also very memory-hungry. All official bots were
> upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> were also accordingly increased.
>
> This CL might cause timeouts / OOMs, if some slaves still
> have not enough RAM. Please, revert the CL with pointing
> out to the slaves / bots broken by this change.
>
> BUG=453195
>
> Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> Cr-Commit-Position: refs/heads/master@{#395365}

TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=453195, 615039
Committed: https://crrev.com/3ec265f1c1a76a6f5dfee6d860cf1a4b6e9ba79a
Cr-Commit-Position: refs/heads/master@{#396175}

[agrieve, 2016-05-26 15:14:33]

Created Revert of Enable LTO on Linux for the official builds.

[commit-bot: I haz the power, 2016-05-26 15:14:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2015703004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2015703004/1

[chromium-reviews, 2016-05-26 15:16:19]

Are you sure it breaks android x64?
The intent of the CL was to only enable LTO on Linux, not Andorid.

On Thu, May 26, 2016 at 8:14 AM, <agrieve@chromium.org> wrote:

> Reviewers: Dirk Pranke (slow), Lei Zhang, krasin
> CL: https://codereview.chromium.org/2015703004/
>
> Message:
> Created Revert of Enable LTO on Linux for the official builds.
>
> Description:
> Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of
> https://codereview.chromium.org/2003733004/ )
>
> Reason for revert:
> Breaks android x64 builds.
> https://bugs.chromium.org/p/chromium/issues/detail?id=615039
>
> Original issue's description:
> > Enable LTO on Linux for the official builds.
> >
> > This CL turns on Link-Time Optimization for official
> > builds, which makes the resulting binaries to run faster.
> >
> > On the dark side, the change increases the link time by 3x-5x,
> > and is also very memory-hungry. All official bots were
> > upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> > were also accordingly increased.
> >
> > This CL might cause timeouts / OOMs, if some slaves still
> > have not enough RAM. Please, revert the CL with pointing
> > out to the slaves / bots broken by this change.
> >
> > BUG=453195
> >
> > Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> > Cr-Commit-Position: refs/heads/master@{#395365}
>
> TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=453195
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+2, -11 lines):
> M build/common.gypi
> M build/toolchain/toolchain.gni
>
>
> Index: build/common.gypi
> diff --git a/build/common.gypi b/build/common.gypi
> index
>
23eab59c9b8b534e442f036a659eee086986ef1f..67348d42460aba5a1b62fa69ff2fff9ec61c3bcd
> 100644
> --- a/build/common.gypi
> +++ b/build/common.gypi
> @@ -825,13 +825,6 @@
>
> ['buildtype=="Official"', {
> 'enable_prod_wallet_service%': 1,
> - }],
> -
> - # Enable Link Time Optimization for the official Linux Chrome.
> - # This requires LLVM Gold plugin to be downloaded.
> - # See src/tools/clang/scripts/update.py
> - ['OS=="linux" and target_arch=="x64" and buildtype=="Official" and
> branding=="Chrome" and chromeos==0', {
> - 'use_lto%': 1,
> }],
>
> # Enable hotwording on Chrome-branded ChromeOS builds.
> Index: build/toolchain/toolchain.gni
> diff --git a/build/toolchain/toolchain.gni b/build/toolchain/toolchain.gni
> index
>
1eae20159efd302d31de93d7d4df4cb42eabb67f..4ab0270f902a7b8dfd9b3114b0281e787d70265e
> 100644
> --- a/build/toolchain/toolchain.gni
> +++ b/build/toolchain/toolchain.gni
> @@ -5,14 +5,12 @@
> # Toolchain-related configuration that may be needed outside the context
> of the
> # toolchain() rules themselves.
>
> -import("//build/config/chrome_build.gni")
> -
> declare_args() {
> # Enable Link Time Optimization in optimized builds (output programs run
> # faster, but linking is up to 5-20x slower).
> #
> - allow_posix_link_time_opt = is_linux && !is_chromeos && target_cpu ==
> "x64" &&
> - is_chrome_branded && is_official_build
> + # TODO(pcc): Remove this flag if/when LTO is enabled in official builds.
> + allow_posix_link_time_opt = false
>
> # Set to true to use lld, the LLVM linker. This flag may be used on Windows
> # with the shipped LLVM toolchain, or on Linux with a self-built
> top-of-tree
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2016-05-26 15:17:22]

The CQ bit was unchecked by thakis@chromium.org

[agrieve, 2016-05-26 15:17:35]

The CQ bit was unchecked by agrieve@chromium.org

[agrieve, 2016-05-26 15:18:08]

Description was changed from

==========
Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of
https://codereview.chromium.org/2003733004/ )

Reason for revert:
Breaks android x64 builds.
https://bugs.chromium.org/p/chromium/issues/detail?id=615039

Original issue's description:
> Enable LTO on Linux for the official builds.
> 
> This CL turns on Link-Time Optimization for official
> builds, which makes the resulting binaries to run faster.
> 
> On the dark side, the change increases the link time by 3x-5x,
> and is also very memory-hungry. All official bots were
> upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> were also accordingly increased.
> 
> This CL might cause timeouts / OOMs, if some slaves still
> have not enough RAM. Please, revert the CL with pointing
> out to the slaves / bots broken by this change.
> 
> BUG=453195
> 
> Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> Cr-Commit-Position: refs/heads/master@{#395365}

TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=453195
==========

to

==========
Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of
https://codereview.chromium.org/2003733004/ )

Reason for revert:
Breaks android x64 builds.
https://bugs.chromium.org/p/chromium/issues/detail?id=615039

Original issue's description:
> Enable LTO on Linux for the official builds.
> 
> This CL turns on Link-Time Optimization for official
> builds, which makes the resulting binaries to run faster.
> 
> On the dark side, the change increases the link time by 3x-5x,
> and is also very memory-hungry. All official bots were
> upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> were also accordingly increased.
> 
> This CL might cause timeouts / OOMs, if some slaves still
> have not enough RAM. Please, revert the CL with pointing
> out to the slaves / bots broken by this change.
> 
> BUG=453195
> 
> Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> Cr-Commit-Position: refs/heads/master@{#395365}

TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=453195,615039
==========

[agrieve, 2016-05-26 15:18:15]

The CQ bit was checked by agrieve@chromium.org

[Nico, 2016-05-26 15:18:24]

Let's try to fix this instead of reverting. The intent is that this shouldn't
trigger for Android, and from looking at the diff it's not clear to me why it
does. It's probably a one-line fix though.

[commit-bot: I haz the power, 2016-05-26 15:18:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2015703004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2015703004/1

[commit-bot: I haz the power, 2016-05-26 15:19:06]

Description was changed from

==========
Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of
https://codereview.chromium.org/2003733004/ )

Reason for revert:
Breaks android x64 builds.
https://bugs.chromium.org/p/chromium/issues/detail?id=615039

Original issue's description:
> Enable LTO on Linux for the official builds.
> 
> This CL turns on Link-Time Optimization for official
> builds, which makes the resulting binaries to run faster.
> 
> On the dark side, the change increases the link time by 3x-5x,
> and is also very memory-hungry. All official bots were
> upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> were also accordingly increased.
> 
> This CL might cause timeouts / OOMs, if some slaves still
> have not enough RAM. Please, revert the CL with pointing
> out to the slaves / bots broken by this change.
> 
> BUG=453195
> 
> Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> Cr-Commit-Position: refs/heads/master@{#395365}

TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=453195,615039
==========

to

==========
Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of
https://codereview.chromium.org/2003733004/ )

Reason for revert:
Breaks android x64 builds.
https://bugs.chromium.org/p/chromium/issues/detail?id=615039

Original issue's description:
> Enable LTO on Linux for the official builds.
> 
> This CL turns on Link-Time Optimization for official
> builds, which makes the resulting binaries to run faster.
> 
> On the dark side, the change increases the link time by 3x-5x,
> and is also very memory-hungry. All official bots were
> upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> were also accordingly increased.
> 
> This CL might cause timeouts / OOMs, if some slaves still
> have not enough RAM. Please, revert the CL with pointing
> out to the slaves / bots broken by this change.
> 
> BUG=453195
> 
> Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> Cr-Commit-Position: refs/heads/master@{#395365}

TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=453195,615039
==========

[commit-bot: I haz the power, 2016-05-26 15:19:07]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-26 15:20:24]

Description was changed from

==========
Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of
https://codereview.chromium.org/2003733004/ )

Reason for revert:
Breaks android x64 builds.
https://bugs.chromium.org/p/chromium/issues/detail?id=615039

Original issue's description:
> Enable LTO on Linux for the official builds.
> 
> This CL turns on Link-Time Optimization for official
> builds, which makes the resulting binaries to run faster.
> 
> On the dark side, the change increases the link time by 3x-5x,
> and is also very memory-hungry. All official bots were
> upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> were also accordingly increased.
> 
> This CL might cause timeouts / OOMs, if some slaves still
> have not enough RAM. Please, revert the CL with pointing
> out to the slaves / bots broken by this change.
> 
> BUG=453195
> 
> Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> Cr-Commit-Position: refs/heads/master@{#395365}

TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=453195,615039
==========

to

==========
Revert of Enable LTO on Linux for the official builds. (patchset #1 id:1 of
https://codereview.chromium.org/2003733004/ )

Reason for revert:
Breaks android x64 builds.
https://bugs.chromium.org/p/chromium/issues/detail?id=615039

Original issue's description:
> Enable LTO on Linux for the official builds.
>
> This CL turns on Link-Time Optimization for official
> builds, which makes the resulting binaries to run faster.
>
> On the dark side, the change increases the link time by 3x-5x,
> and is also very memory-hungry. All official bots were
> upgraded to have 200+ GB RAM to handle LTO builds. Timeouts
> were also accordingly increased.
>
> This CL might cause timeouts / OOMs, if some slaves still
> have not enough RAM. Please, revert the CL with pointing
> out to the slaves / bots broken by this change.
>
> BUG=453195
>
> Committed: https://crrev.com/7c163521444b7277f2c33bc7bc05667673db7d48
> Cr-Commit-Position: refs/heads/master@{#395365}

TBR=dpranke@chromium.org,thestig@chromium.org,krasin@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=453195,615039

Committed: https://crrev.com/3ec265f1c1a76a6f5dfee6d860cf1a4b6e9ba79a
Cr-Commit-Position: refs/heads/master@{#396175}
==========

[commit-bot: I haz the power, 2016-05-26 15:20:25]

Patchset 1 (id:??) landed as
https://crrev.com/3ec265f1c1a76a6f5dfee6d860cf1a4b6e9ba79a
Cr-Commit-Position: refs/heads/master@{#396175}

[Nico, 2016-05-26 15:20:50]

On 2016/05/26 15:19:07, commit-bot: I haz the power wrote:
> Committed patchset #1 (id:1)

...

[Nico, 2016-05-26 15:21:50]

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

[Nico, 2016-05-26 15:21:51]

https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.gni
File build/toolchain/toolchain.gni (left):

https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.g...
build/toolchain/toolchain.gni:14: allow_posix_link_time_opt = is_linux &&
!is_chromeos && target_cpu == "x64" &&
I'm not sure why you rechecked cq after I unchecked it, but I think replacing
"is_linux" with 'target_os == "linux"' here should be sufficient to fix your
problem.

[chromium-reviews, 2016-05-26 15:24:26]

is this already reverted?
Can we un-revert and then fix, as thakis suggests?

On Thu, May 26, 2016 at 8:21 AM, <thakis@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.gni
> File build/toolchain/toolchain.gni (left):
>
>
>
https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.g...
> build/toolchain/toolchain.gni:14: allow_posix_link_time_opt = is_linux
> && !is_chromeos && target_cpu == "x64" &&
> I'm not sure why you rechecked cq after I unchecked it, but I think
> replacing "is_linux" with 'target_os == "linux"' here should be
> sufficient to fix your problem.
>
> https://codereview.chromium.org/2015703004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[agrieve, 2016-05-26 15:33:02]

On 2016/05/26 15:24:26, chromium-reviews wrote:
> is this already reverted?
> Can we un-revert and then fix, as thakis suggests?
> 
> On Thu, May 26, 2016 at 8:21 AM, <mailto:thakis@chromium.org> wrote:
> 
> >
> >
> >
>
https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.gni
> > File build/toolchain/toolchain.gni (left):
> >
> >
> >
>
https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.g...
> > build/toolchain/toolchain.gni:14: allow_posix_link_time_opt = is_linux
> > && !is_chromeos && target_cpu == "x64" &&
> > I'm not sure why you rechecked cq after I unchecked it, but I think
> > replacing "is_linux" with 'target_os == "linux"' here should be
> > sufficient to fix your problem.
> >
> > https://codereview.chromium.org/2015703004/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Sorry, didn't see that you had unchecked it. Tried the fix locally and it
worked. I'll reland with the fix.

[agrieve, 2016-05-26 15:33:03]

On 2016/05/26 15:24:26, chromium-reviews wrote:
> is this already reverted?
> Can we un-revert and then fix, as thakis suggests?
> 
> On Thu, May 26, 2016 at 8:21 AM, <mailto:thakis@chromium.org> wrote:
> 
> >
> >
> >
>
https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.gni
> > File build/toolchain/toolchain.gni (left):
> >
> >
> >
>
https://codereview.chromium.org/2015703004/diff/1/build/toolchain/toolchain.g...
> > build/toolchain/toolchain.gni:14: allow_posix_link_time_opt = is_linux
> > && !is_chromeos && target_cpu == "x64" &&
> > I'm not sure why you rechecked cq after I unchecked it, but I think
> > replacing "is_linux" with 'target_os == "linux"' here should be
> > sufficient to fix your problem.
> >
> > https://codereview.chromium.org/2015703004/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Sorry, didn't see that you had unchecked it. Tried the fix locally and it
worked. I'll reland with the fix.

[agrieve, 2016-05-26 15:33:50]

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

The reason for reverting is: Will re-land with Nico's suggested fix..