Move CT pages / page sets / benchmarks into Telemetry repo

Move CT pages / page sets / benchmarks into Telemetry repo.

Context:
In https://codereview.chromium.org/730033005/#msg15 I was convinced to create a ct_run_benchmark and my own
copy of benchmarks in the CT repository ( https://skia.googlesource.com/buildbot/+/master/ct/py/ct_run_benchmark
and https://skia.googlesource.com/buildbot/+/master/ct/py/benchmarks/ ).

In retrospect this was not a good idea, because:
When there are upstream changes only benchmarks in telemetry repo will be updated, CT benchmarks will break.
Moving this into telemetry repo makes things much more maintainable.

BUG=skia:4435
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:mac_10_10_perf_bisect

Committed: https://crrev.com/6b539126337ade3771ec901867a1f2af58662f04
Cr-Commit-Position: refs/heads/master@{#354285}

[rmistry, 2015-10-07 17:18:34]

Patchset #1 (id:1) has been deleted

[rmistry, 2015-10-07 17:33:13]

rmistry@google.com changed reviewers:
+ nednguyen@google.com

[nednguyen, 2015-10-07 17:42:25]

Hi Ravi, why can't your benchmark in cluster telemetry just subclass
_RasterizeAndRecordMicro to specify the page set themselves?

https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
File tools/perf/benchmarks/rasterize_and_record_micro.py (right):

https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
tools/perf/benchmarks/rasterize_and_record_micro.py:123: return page_set_class()
This restrict yourself to only be able to create page_set with empty
constructor. Is this what you want given we wouldn't want to generate the python
page set files dynmically moving forward?

[rmistry, 2015-10-07 19:44:58]

On 2015/10/07 17:42:25, nednguyen wrote:
> Hi Ravi, why can't your benchmark in cluster telemetry just subclass
> _RasterizeAndRecordMicro to specify the page set themselves?

They could, but then it would be a similar situation as today, changes to the
core framework would still end up breaking CT because CT's benchmarks would be
in another repository and would rely on the core structure.

> 
>
https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
> File tools/perf/benchmarks/rasterize_and_record_micro.py (right):
> 
>
https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
> tools/perf/benchmarks/rasterize_and_record_micro.py:123: return
page_set_class()
> This restrict yourself to only be able to create page_set with empty
> constructor. Is this what you want given we wouldn't want to generate the
python
> page set files dynmically moving forward?

"we wouldn't want to generate the python page set files dynamically moving
forward?"
Could you elaborate on this? I remember hearing about this a while back but do
not remember what the details where.

[rmistry, 2015-10-08 13:11:50]

On 2015/10/07 19:44:58, rmistry wrote:
> On 2015/10/07 17:42:25, nednguyen wrote:
> > Hi Ravi, why can't your benchmark in cluster telemetry just subclass
> > _RasterizeAndRecordMicro to specify the page set themselves?
> 
> They could, but then it would be a similar situation as today, changes to the
> core framework would still end up breaking CT because CT's benchmarks would be
> in another repository and would rely on the core structure.
> 
> > 
> >
>
https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
> > File tools/perf/benchmarks/rasterize_and_record_micro.py (right):
> > 
> >
>
https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
> > tools/perf/benchmarks/rasterize_and_record_micro.py:123: return
> page_set_class()
> > This restrict yourself to only be able to create page_set with empty
> > constructor. Is this what you want given we wouldn't want to generate the
> python
> > page set files dynmically moving forward?
> 
> "we wouldn't want to generate the python page set files dynamically moving
> forward?"
> Could you elaborate on this? I remember hearing about this a while back but do
> not remember what the details where.

About page sets with empty constructor: maybe that can be the restriction to use
the custom version of benchmarks, I can document this better if you prefer.

My renewed motivation for moving all benchmarks out of the CT repository is to
then allow users to run unlanded benchmarks on the cluster (feature request is
in https://code.google.com/p/skia/issues/detail?id=4173 ).

[nednguyen, 2015-10-08 20:27:25]

On 2015/10/08 13:11:50, rmistry wrote:
> On 2015/10/07 19:44:58, rmistry wrote:
> > On 2015/10/07 17:42:25, nednguyen wrote:
> > > Hi Ravi, why can't your benchmark in cluster telemetry just subclass
> > > _RasterizeAndRecordMicro to specify the page set themselves?
> > 
> > They could, but then it would be a similar situation as today, changes to
the
> > core framework would still end up breaking CT because CT's benchmarks would
be
> > in another repository and would rely on the core structure.
> > 
> > > 
> > >
> >
>
https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
> > > File tools/perf/benchmarks/rasterize_and_record_micro.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1393023002/diff/40001/tools/perf/benchmarks/r...
> > > tools/perf/benchmarks/rasterize_and_record_micro.py:123: return
> > page_set_class()
> > > This restrict yourself to only be able to create page_set with empty
> > > constructor. Is this what you want given we wouldn't want to generate the
> > python
> > > page set files dynmically moving forward?
> > 
> > "we wouldn't want to generate the python page set files dynamically moving
> > forward?"
> > Could you elaborate on this? I remember hearing about this a while back but
do
> > not remember what the details where.
> 
> About page sets with empty constructor: maybe that can be the restriction to
use
> the custom version of benchmarks, I can document this better if you prefer.
> 
> My renewed motivation for moving all benchmarks out of the CT repository is to
> then allow users to run unlanded benchmarks on the cluster (feature request is
> in https://code.google.com/p/skia/issues/detail?id=4173 ).

I am not sure that this helps much because if page/story_set API change, the
benchmark will still be broken. I think I need better understanding of your test
cases. What pages do you have in your page sets? If all your page behave in the
same way & only different in term of URLs, we can design something in tools perf
that you can pass in a list of URLs to run the benchmarks.

[rmistry, 2015-10-13 12:45:50]

Patchset #3 (id:60001) has been deleted

[rmistry, 2015-10-13 12:48:59]

Ned, I updated this CL based on our email conversation. WDYT?

I only made the change to rr, if this looks ok then I can make the same changes
to repaint and skpicture_printer.

[nednguyen, 2015-10-13 13:22:51]

On 2015/10/13 12:48:59, rmistry wrote:
> Ned, I updated this CL based on our email conversation. WDYT?
> 
> I only made the change to rr, if this looks ok then I can make the same
changes
> to repaint and skpicture_printer.

Awesome, this is lg2me. Can you try adding a smoke test for the
RasterizeAndRecordMicroCustomPageSet benchmark?
To set the option object, you can copy the code in
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...

[rmistry, 2015-10-13 17:21:56]

The CQ bit was checked by rmistry@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-13 17:38:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1393023002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1393023002/80001

[commit-bot: I haz the power, 2015-10-13 17:38:20]

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

[commit-bot: I haz the power, 2015-10-13 17:38:22]

Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are
accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[rmistry, 2015-10-13 18:48:33]

On 2015/10/13 13:22:51, nednguyen (SLOW REVIEW ooo) wrote:
> On 2015/10/13 12:48:59, rmistry wrote:
> > Ned, I updated this CL based on our email conversation. WDYT?
> > 
> > I only made the change to rr, if this looks ok then I can make the same
> changes
> > to repaint and skpicture_printer.
> 
> Awesome, this is lg2me. Can you try adding a smoke test for the
> RasterizeAndRecordMicroCustomPageSet benchmark?
> To set the option object, you can copy the code in
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...

I added a unittest (not sure if this is what you had in mind) PTAL.
This is only for RR have not done Repaint / SkPicturePrinter yet.

[nednguyen, 2015-10-13 19:01:55]

On 2015/10/13 18:48:33, rmistry wrote:
> On 2015/10/13 13:22:51, nednguyen (SLOW REVIEW ooo) wrote:
> > On 2015/10/13 12:48:59, rmistry wrote:
> > > Ned, I updated this CL based on our email conversation. WDYT?
> > > 
> > > I only made the change to rr, if this looks ok then I can make the same
> > changes
> > > to repaint and skpicture_printer.
> > 
> > Awesome, this is lg2me. Can you try adding a smoke test for the
> > RasterizeAndRecordMicroCustomPageSet benchmark?
> > To set the option object, you can copy the code in
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...
> 
> I added a unittest (not sure if this is what you had in mind) PTAL.
> This is only for RR have not done Repaint / SkPicturePrinter yet.

Can you raise some exception in  CTPage.RunPageInteractions & veirfy that
./tools/perf/run_tests ct_benchmarks_unittest fail? Besides that, this is lg2me
& you can go ahead with adding similar functions to  Repaint & SkPicturePrinter

[nednguyen, 2015-10-13 19:02:27]

nednguyen@google.com changed reviewers:
+ eakuefner@chromium.org

[nednguyen, 2015-10-13 20:38:15]

nednguyen@google.com changed reviewers:
+ aiolos@chromium.org

[rmistry, 2015-10-14 11:43:13]

On 2015/10/13 19:01:55, nednguyen (SLOW REVIEW ooo) wrote:
> On 2015/10/13 18:48:33, rmistry wrote:
> > On 2015/10/13 13:22:51, nednguyen (SLOW REVIEW ooo) wrote:
> > > On 2015/10/13 12:48:59, rmistry wrote:
> > > > Ned, I updated this CL based on our email conversation. WDYT?
> > > > 
> > > > I only made the change to rr, if this looks ok then I can make the same
> > > changes
> > > > to repaint and skpicture_printer.
> > > 
> > > Awesome, this is lg2me. Can you try adding a smoke test for the
> > > RasterizeAndRecordMicroCustomPageSet benchmark?
> > > To set the option object, you can copy the code in
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...
> > 
> > I added a unittest (not sure if this is what you had in mind) PTAL.
> > This is only for RR have not done Repaint / SkPicturePrinter yet.
> 
> Can you raise some exception in  CTPage.RunPageInteractions & veirfy that
> ./tools/perf/run_tests ct_benchmarks_unittest fail? Besides that, this is
lg2me
> & you can go ahead with adding similar functions to  Repaint &
SkPicturePrinter

Done. Also made changes to repaint and skpicture_printer.

[rmistry, 2015-10-14 11:43:22]

On 2015/10/14 11:43:13, rmistry wrote:
> On 2015/10/13 19:01:55, nednguyen (SLOW REVIEW ooo) wrote:
> > On 2015/10/13 18:48:33, rmistry wrote:
> > > On 2015/10/13 13:22:51, nednguyen (SLOW REVIEW ooo) wrote:
> > > > On 2015/10/13 12:48:59, rmistry wrote:
> > > > > Ned, I updated this CL based on our email conversation. WDYT?
> > > > > 
> > > > > I only made the change to rr, if this looks ok then I can make the
same
> > > > changes
> > > > > to repaint and skpicture_printer.
> > > > 
> > > > Awesome, this is lg2me. Can you try adding a smoke test for the
> > > > RasterizeAndRecordMicroCustomPageSet benchmark?
> > > > To set the option object, you can copy the code in
> > > >
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...
> > > 
> > > I added a unittest (not sure if this is what you had in mind) PTAL.
> > > This is only for RR have not done Repaint / SkPicturePrinter yet.
> > 
> > Can you raise some exception in  CTPage.RunPageInteractions & veirfy that
> > ./tools/perf/run_tests ct_benchmarks_unittest fail? Besides that, this is
> lg2me
> > & you can go ahead with adding similar functions to  Repaint &
> SkPicturePrinter
> 
> Done. Also made changes to repaint and skpicture_printer.

PTAL.

[rmistry, 2015-10-15 11:01:03]

On 2015/10/14 11:43:22, rmistry wrote:
> On 2015/10/14 11:43:13, rmistry wrote:
> > On 2015/10/13 19:01:55, nednguyen (SLOW REVIEW ooo) wrote:
> > > On 2015/10/13 18:48:33, rmistry wrote:
> > > > On 2015/10/13 13:22:51, nednguyen (SLOW REVIEW ooo) wrote:
> > > > > On 2015/10/13 12:48:59, rmistry wrote:
> > > > > > Ned, I updated this CL based on our email conversation. WDYT?
> > > > > > 
> > > > > > I only made the change to rr, if this looks ok then I can make the
> same
> > > > > changes
> > > > > > to repaint and skpicture_printer.
> > > > > 
> > > > > Awesome, this is lg2me. Can you try adding a smoke test for the
> > > > > RasterizeAndRecordMicroCustomPageSet benchmark?
> > > > > To set the option object, you can copy the code in
> > > > >
> > > >
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...
> > > > 
> > > > I added a unittest (not sure if this is what you had in mind) PTAL.
> > > > This is only for RR have not done Repaint / SkPicturePrinter yet.
> > > 
> > > Can you raise some exception in  CTPage.RunPageInteractions & veirfy that
> > > ./tools/perf/run_tests ct_benchmarks_unittest fail? Besides that, this is
> > lg2me
> > > & you can go ahead with adding similar functions to  Repaint &
> > SkPicturePrinter
> > 
> > Done. Also made changes to repaint and skpicture_printer.
> 
> PTAL.

Friendly ping. I would like to get this in soonish so that I can test stuff over
the coming weekend.

[nednguyen, 2015-10-15 12:51:19]

lgtm with nits

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
File tools/perf/benchmarks/ct_benchmarks_unittest.py (right):

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
tools/perf/benchmarks/ct_benchmarks_unittest.py:117: if __name__ == '__main__':
nits: remove this

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
File tools/perf/benchmarks/rasterize_and_record_micro.py (right):

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
tools/perf/benchmarks/rasterize_and_record_micro.py:109: def
AddBenchmarkCommandLineArgs(cls, parser):
Since this logic is the same for the three benchmarks, can you factor this to a
helper method?

You can add s.t to tools/perf/ct_benchmarks_util.py

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
tools/perf/benchmarks/rasterize_and_record_micro.py:122: def
ProcessCommandLineArgs(cls, parser, args):
Same for this. 

ct_benchmarks_util.ValidateCommandlineArgs

[rmistry, 2015-10-15 13:24:32]

Thanks Ned!

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
File tools/perf/benchmarks/ct_benchmarks_unittest.py (right):

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
tools/perf/benchmarks/ct_benchmarks_unittest.py:117: if __name__ == '__main__':
On 2015/10/15 12:51:19, nednguyen (SLOW REVIEW ooo) wrote:
> nits: remove this

Done.

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
File tools/perf/benchmarks/rasterize_and_record_micro.py (right):

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
tools/perf/benchmarks/rasterize_and_record_micro.py:109: def
AddBenchmarkCommandLineArgs(cls, parser):
On 2015/10/15 12:51:19, nednguyen (SLOW REVIEW ooo) wrote:
> Since this logic is the same for the three benchmarks, can you factor this to
a
> helper method?
> 
> You can add s.t to tools/perf/ct_benchmarks_util.py

Done.

https://codereview.chromium.org/1393023002/diff/160001/tools/perf/benchmarks/...
tools/perf/benchmarks/rasterize_and_record_micro.py:122: def
ProcessCommandLineArgs(cls, parser, args):
On 2015/10/15 12:51:19, nednguyen (SLOW REVIEW ooo) wrote:
> Same for this. 
> 
> ct_benchmarks_util.ValidateCommandlineArgs

Done.

[rmistry, 2015-10-15 13:24:53]

The CQ bit was checked by rmistry@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-15 13:25:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1393023002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1393023002/200001

[commit-bot: I haz the power, 2015-10-15 13:33:42]

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

[commit-bot: I haz the power, 2015-10-15 13:33:42]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[rmistry, 2015-10-15 13:36:26]

The CQ bit was checked by rmistry@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-15 13:36:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1393023002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1393023002/220001

[eakuefner, 2015-10-15 14:01:31]

lgtm % couple of nits

I'm really glad to see this moving into tools/perf.

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
File tools/perf/benchmarks/ct_benchmarks_unittest.py (right):

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
tools/perf/benchmarks/ct_benchmarks_unittest.py:5: """Test Cluster Telemetry
benchmarks and page sets."""
nit: no need for this comment

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
File tools/perf/benchmarks/rasterize_and_record_micro.py (right):

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
tools/perf/benchmarks/rasterize_and_record_micro.py:101: @benchmark.Disabled
Usually Disabled usages are expected to be accompanied by a bug ID that
specifies why it is that the benchmarks are disabled -- in this case it looks
like you plan on not running these on the perf waterfall indefinitely, so could
you just write a comment to this effect?

[rmistry, 2015-10-15 14:10:37]

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
File tools/perf/benchmarks/ct_benchmarks_unittest.py (right):

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
tools/perf/benchmarks/ct_benchmarks_unittest.py:5: """Test Cluster Telemetry
benchmarks and page sets."""
On 2015/10/15 14:01:31, eakuefner wrote:
> nit: no need for this comment

Done.

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
File tools/perf/benchmarks/rasterize_and_record_micro.py (right):

https://codereview.chromium.org/1393023002/diff/220001/tools/perf/benchmarks/...
tools/perf/benchmarks/rasterize_and_record_micro.py:101: @benchmark.Disabled
On 2015/10/15 14:01:31, eakuefner wrote:
> Usually Disabled usages are expected to be accompanied by a bug ID that
> specifies why it is that the benchmarks are disabled -- in this case it looks
> like you plan on not running these on the perf waterfall indefinitely, so
could
> you just write a comment to this effect?

Done.

[rmistry, 2015-10-15 14:10:43]

The CQ bit was checked by rmistry@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-15 14:11:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1393023002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1393023002/240001

[commit-bot: I haz the power, 2015-10-15 16:12:13]

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

[commit-bot: I haz the power, 2015-10-15 16:12:13]

Dry run: Try jobs failed on following builders:
  android_nexus5_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no build
URL)
  linux_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)

[rmistry, 2015-10-15 16:13:57]

On 2015/10/15 16:12:13, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   android_nexus5_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no
build
> URL)
>   linux_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)

Removed a few extra trybots because the jobs timed out.

[rmistry, 2015-10-15 16:14:03]

On 2015/10/15 16:12:13, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   android_nexus5_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no
build
> URL)
>   linux_perf_bisect on tryserver.chromium.perf (JOB_TIMED_OUT, no build URL)

Removed a few extra trybots because the jobs timed out.

[rmistry, 2015-10-15 16:14:19]

The CQ bit was checked by rmistry@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-15 16:15:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1393023002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1393023002/240001

[commit-bot: I haz the power, 2015-10-15 16:19:54]

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

[commit-bot: I haz the power, 2015-10-15 16:19:55]

Dry run: This issue passed the CQ dry run.

[rmistry, 2015-10-15 16:36:40]

The CQ bit was checked by rmistry@google.com

[rmistry, 2015-10-15 16:36:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, eakuefner@chromium.org
Link to the patchset: https://codereview.chromium.org/1393023002/#ps240001
(title: "Address comments")

[commit-bot: I haz the power, 2015-10-15 16:36:49]

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

[commit-bot: I haz the power, 2015-10-15 16:53:55]

Committed patchset #11 (id:240001)

[commit-bot: I haz the power, 2015-10-15 16:54:35]

Patchset 11 (id:??) landed as
https://crrev.com/6b539126337ade3771ec901867a1f2af58662f04
Cr-Commit-Position: refs/heads/master@{#354285}