Add sizes test to chromium.perf builders.

Add sizes test to chromium.perf builders.

Remove the test from the perf try configs, as those configs copy the perf waterfall configs.


BUG=570147
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300299

[dtu, 2016-04-27 14:41:28]

Description was changed from

==========
Add sizes to chromium.perf builders.


BUG=570147
==========

to

==========
Add sizes test to chromium.perf builders.

Remove some code that appears to be unused.


BUG=570147
==========

[dtu, 2016-04-27 14:41:29]

dtu@chromium.org changed reviewers:
+ aiolos@chromium.org, hans@chromium.org, prasadv@chromium.org

[hans, 2016-04-27 15:49:42]

There seems to be multiple things happening in this patch: enabling sizes,
remove some dead code, pointing _bisector -> _bisect (or is that part of the
dead code you refer to?)

Please break it up into separate patches.

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/tryserver_chromium_perf.py
(left):

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/tryserver_chromium_perf.py:111: },
Is this the dead code you're referring to in the description? If so, can you
just remove it in a separate patch instead?

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisect.py (right):

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/b...
scripts/slave/recipes/bisect.py:38: buildername='linux_perf_bisect')
This also seems unrelated to enabling sizes?

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
File
scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Builder.json
(right):

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Builder.json:784:
"RECIPE_PACKAGE_REPO[build]/scripts/slave/chromium/sizes.py",
Will this work on Android?

Maybe runtest.py expects the bot to have devices attached?

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
File scripts/slave/recipes/chromium.py (left):

https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
scripts/slave/recipes/chromium.py:143: test +=
api.bisect_tester(tempfile='/tmp/dummy')
This also seems unrelated to sizes.

[dtu, 2016-04-27 20:01:50]

On 2016/04/27 at 15:49:42, hans wrote:
> There seems to be multiple things happening in this patch: enabling sizes,
remove some dead code, pointing _bisector -> _bisect (or is that part of the
dead code you refer to?)
> 
> Please break it up into separate patches.
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipe_mo...
> File scripts/slave/recipe_modules/chromium_tests/tryserver_chromium_perf.py
(left):
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/chromium_tests/tryserver_chromium_perf.py:111: },
> Is this the dead code you're referring to in the description? If so, can you
just remove it in a separate patch instead?
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/b...
> File scripts/slave/recipes/bisect.py (right):
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/b...
> scripts/slave/recipes/bisect.py:38: buildername='linux_perf_bisect')
> This also seems unrelated to enabling sizes?
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
> File
scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Builder.json
(right):
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
>
scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Builder.json:784:
"RECIPE_PACKAGE_REPO[build]/scripts/slave/chromium/sizes.py",
> Will this work on Android?
> 
> Maybe runtest.py expects the bot to have devices attached?
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
> File scripts/slave/recipes/chromium.py (left):
> 
>
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
> scripts/slave/recipes/chromium.py:143: test +=
api.bisect_tester(tempfile='/tmp/dummy')
> This also seems unrelated to sizes.


Ok, uploaded a separate CL for the pre-work of removing the dead code.
https://codereview.chromium.org/1929613003

[dtu, 2016-04-27 21:30:02]

On 2016/04/27 at 20:01:50, dtu wrote:
> On 2016/04/27 at 15:49:42, hans wrote:
> > There seems to be multiple things happening in this patch: enabling sizes,
remove some dead code, pointing _bisector -> _bisect (or is that part of the
dead code you refer to?)
> > 
> > Please break it up into separate patches.
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipe_mo...
> > File scripts/slave/recipe_modules/chromium_tests/tryserver_chromium_perf.py
(left):
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipe_mo...
> > scripts/slave/recipe_modules/chromium_tests/tryserver_chromium_perf.py:111:
},
> > Is this the dead code you're referring to in the description? If so, can you
just remove it in a separate patch instead?
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/b...
> > File scripts/slave/recipes/bisect.py (right):
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/b...
> > scripts/slave/recipes/bisect.py:38: buildername='linux_perf_bisect')
> > This also seems unrelated to enabling sizes?
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
> > File
scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Builder.json
(right):
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
> >
scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Builder.json:784:
"RECIPE_PACKAGE_REPO[build]/scripts/slave/chromium/sizes.py",
> > Will this work on Android?
> > 
> > Maybe runtest.py expects the bot to have devices attached?
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
> > File scripts/slave/recipes/chromium.py (left):
> > 
> >
https://codereview.chromium.org/1920863007/diff/20001/scripts/slave/recipes/c...
> > scripts/slave/recipes/chromium.py:143: test +=
api.bisect_tester(tempfile='/tmp/dummy')
> > This also seems unrelated to sizes.
> 
> 
> Ok, uploaded a separate CL for the pre-work of removing the dead code.
> https://codereview.chromium.org/1929613003

The reason for the pre-work is that the bisect bots copy the perf bot configs.
This means, if I add sizes to the perf builders, it will automatically run on
the bisect bots. I need to remove the dead code so it's safe to modify the
bisect test configs.

[dtu, 2016-04-27 21:38:52]

Patchset #3 (id:40001) has been deleted

[dtu, 2016-04-27 22:02:52]

PTAL! This is dependent on https://codereview.chromium.org/1929613003 .

[hans, 2016-04-27 22:22:28]

lgtm, thanks!

"Remove some code that appears to be unused." from the CL description probably
doesn't apply anymore.

[dtu, 2016-04-28 01:19:53]

Description was changed from

==========
Add sizes test to chromium.perf builders.

Remove some code that appears to be unused.


BUG=570147
==========

to

==========
Add sizes test to chromium.perf builders.

Remove the test from the perf try configs, as those configs copy the perf
waterfall configs.


BUG=570147
==========

[dtu, 2016-04-28 19:19:18]

dtu@chromium.org changed reviewers:
+ robertocn@chromium.org

[dtu, 2016-04-28 19:19:19]

PTAL: prasadv or robertocn for OWNERS

[RobertoCN, 2016-04-28 20:07:49]

lgtm

[dtu, 2016-04-28 20:33:32]

The CQ bit was checked by dtu@chromium.org

[commit-bot: I haz the power, 2016-04-28 20:33:39]

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

[commit-bot: I haz the power, 2016-04-28 20:39:04]

Description was changed from

==========
Add sizes test to chromium.perf builders.

Remove the test from the perf try configs, as those configs copy the perf
waterfall configs.


BUG=570147
==========

to

==========
Add sizes test to chromium.perf builders.

Remove the test from the perf try configs, as those configs copy the perf
waterfall configs.


BUG=570147

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300299
==========

[commit-bot: I haz the power, 2016-04-28 20:39:06]

Committed patchset #3 (id:60001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=300299

[hans, 2016-05-10 16:39:10]

It seems this broke the size step on
win_perf_bisect_builder:win_perf_bisect_builder

I rely on that for tracking down Clang size regressions. Can we put it back?

[hans, 2016-05-10 16:39:42]

On 2016/05/10 16:39:10, hans wrote:
> It seems this broke the size step on
> win_perf_bisect_builder:win_perf_bisect_builder

Copy-paste fail; I meant to point to this:
https://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect...

[dtu, 2016-05-10 20:36:03]

On 2016/05/10 16:39:42, hans wrote:
> On 2016/05/10 16:39:10, hans wrote:
> > It seems this broke the size step on
> > win_perf_bisect_builder:win_perf_bisect_builder
> 
> Copy-paste fail; I meant to point to this:
>
https://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect...

I'm a little confused. How are you using that?

[hans, 2016-05-10 20:38:38]

On 2016/05/10 20:36:03, dtu wrote:
> On 2016/05/10 16:39:42, hans wrote:
> > On 2016/05/10 16:39:10, hans wrote:
> > > It seems this broke the size step on
> > > win_perf_bisect_builder:win_perf_bisect_builder
> > 
> > Copy-paste fail; I meant to point to this:
> >
>
https://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect...
> 
> I'm a little confused. How are you using that?

I just send tryjobs to it:

git cl try -m tryserver.chromium.perf -b win_perf_bisect_builder -r nnnn