Make components source_sets on Android.

Make components source_sets on Android.

There was a perf regression reported when components were changed from source sets to static libraries in the non-component build. This patch switches back to source sets on Android only to see what happens to thread_times.key_silk_cases and the build performance.

BUG=619593
Committed: https://crrev.com/89248dc70996f9927292a7fee6911715464e1643
Cr-Commit-Position: refs/heads/master@{#405056}

[brettw, 2016-07-11 20:08:30]

Description was changed from

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593
==========

to

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593
TBR=dpranke
==========

[brettw, 2016-07-11 20:08:30]

brettw@chromium.org changed reviewers:
+ dpranke@chromium.org

[brettw, 2016-07-11 20:08:34]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-07-11 20:09:05]

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, 2016-07-11 21:45:18]

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

[commit-bot: I haz the power, 2016-07-11 21:45:22]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2016-07-11 21:47:50]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-07-11 21:48:29]

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

[Dirk Pranke, 2016-07-11 22:36:02]

lgtm

[brettw, 2016-07-11 22:44:15]

Description was changed from

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593
TBR=dpranke
==========

to

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593
==========

[commit-bot: I haz the power, 2016-07-12 01:52:28]

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

[commit-bot: I haz the power, 2016-07-12 01:52:29]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2016-07-12 18:10:44]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-07-12 18:12:14]

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, 2016-07-12 21:56:30]

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

[commit-bot: I haz the power, 2016-07-12 21:56:30]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2016-07-13 04:59:15]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-07-13 04:59:31]

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, 2016-07-13 06:25:39]

Description was changed from

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593
==========

to

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593
==========

[commit-bot: I haz the power, 2016-07-13 06:25:40]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-13 06:25:46]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 06:28:25]

Description was changed from

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593
==========

to

==========
Make components source_sets on Android.

There was a perf regression reported when components were changed from source
sets to static libraries in the non-component build. This patch switches back to
source sets on Android only to see what happens to thread_times.key_silk_cases
and the build performance.

BUG=619593

Committed: https://crrev.com/89248dc70996f9927292a7fee6911715464e1643
Cr-Commit-Position: refs/heads/master@{#405056}
==========

[commit-bot: I haz the power, 2016-07-13 06:28:26]

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

[Nico, 2016-07-13 15:05:16]

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

[Nico, 2016-07-13 15:05:16]

All clang tot bots are now failing generate_build_files with messages like this:

ERROR at build arg file (use "gn args <out_dir>" to edit):7:28: Assignment had
no effect.
llvm_force_head_revision = true
                           ^---
You set the variable "llvm_force_head_revision" here and it was unused before it
went
out of scope.
See //build/config/BUILDCONFIG.gn:462:3: which caused the file to be included.
  "//build/config/compiler:compiler",
  ^---------------------------------


https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin64?numbuilds=200
https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin64/builds/8939/...

https://build.chromium.org/p/chromium.fyi/builders/ClangToTLinux/builds/5721/...

(and many more)

This file did touch BUILDCONFIG.gn and it's the only one doing so in the
regression range, but it's not clear how it could cause that problem.

Since it's a small CL and it'll be a while until y'all are going to get in, I'll
speculatively revert it for now.

[Nico, 2016-07-13 15:05:52]

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

The reason for reverting is: Speculative, might've broke
llvm_force_head_revision=true builds (see comment on the original review)..

[Reid Kleckner, 2016-07-13 15:28:33]

On 2016/07/13 15:05:52, Nico wrote:
> A revert of this CL (patchset #1 id:1) has been created in
> https://codereview.chromium.org/2151523002/ by mailto:thakis@chromium.org.
> 
> The reason for reverting is: Speculative, might've broke
> llvm_force_head_revision=true builds (see comment on the original review)..

If this doesn't fix it, I suspect the gn roll:
https://codereview.chromium.org/2138013004/

[Reid Kleckner, 2016-07-13 15:30:22]

On 2016/07/13 15:28:33, Reid Kleckner wrote:
> On 2016/07/13 15:05:52, Nico wrote:
> > A revert of this CL (patchset #1 id:1) has been created in
> > https://codereview.chromium.org/2151523002/ by mailto:thakis@chromium.org.
> > 
> > The reason for reverting is: Speculative, might've broke
> > llvm_force_head_revision=true builds (see comment on the original review)..
> 
> If this doesn't fix it, I suspect the gn roll:
> https://codereview.chromium.org/2138013004/

This didn't fix it, here is a clang TOT build with the revert that is still red
for the same reason:
https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT/builds/3...

[Nico, 2016-07-13 15:32:04]

A'ight, I relanded this CL. Is reverting the gn roll safe, or does
something depend on the new binary already?

On Wed, Jul 13, 2016 at 11:30 AM, <rnk@chromium.org> wrote:

> On 2016/07/13 15:28:33, Reid Kleckner wrote:
> > On 2016/07/13 15:05:52, Nico wrote:
> > > A revert of this CL (patchset #1 id:1) has been created in
> > > https://codereview.chromium.org/2151523002/ by mailto:
> thakis@chromium.org.
> > >
> > > The reason for reverting is: Speculative, might've broke
> > > llvm_force_head_revision=true builds (see comment on the original
> review)..
> >
> > If this doesn't fix it, I suspect the gn roll:
> > https://codereview.chromium.org/2138013004/
>
> This didn't fix it, here is a clang TOT build with the revert that is
> still red
> for the same reason:
>
>
https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT/builds/3...
>
> https://codereview.chromium.org/2137023003/
>

-- 
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.

[Dirk Pranke, 2016-07-13 16:28:39]

Reverting the GN roll should be safe.

On Wed, Jul 13, 2016 at 8:32 AM, Nico Weber <thakis@chromium.org> wrote:

> A'ight, I relanded this CL. Is reverting the gn roll safe, or does
> something depend on the new binary already?
>
> On Wed, Jul 13, 2016 at 11:30 AM, <rnk@chromium.org> wrote:
>
>> On 2016/07/13 15:28:33, Reid Kleckner wrote:
>> > On 2016/07/13 15:05:52, Nico wrote:
>> > > A revert of this CL (patchset #1 id:1) has been created in
>> > > https://codereview.chromium.org/2151523002/ by mailto:
>> thakis@chromium.org.
>> > >
>> > > The reason for reverting is: Speculative, might've broke
>> > > llvm_force_head_revision=true builds (see comment on the original
>> review)..
>> >
>> > If this doesn't fix it, I suspect the gn roll:
>> > https://codereview.chromium.org/2138013004/
>>
>> This didn't fix it, here is a clang TOT build with the revert that is
>> still red
>> for the same reason:
>>
>>
https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT/builds/3...
>>
>> https://codereview.chromium.org/2137023003/
>>
>
>

-- 
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.