Add new multipage_skpicture_printer benchmark and measurement

Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

Committed: https://crrev.com/b40b6e680dcca6b9a7bbfe31455d3e05d9b0c9ab
Cr-Commit-Position: refs/heads/master@{#415115}

[rmistry, 2016-08-25 17:53:01]

Description was changed from

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
==========

to

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

[rmistry, 2016-08-25 17:58:17]

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

[rmistry, 2016-08-25 17:58:56]

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

[commit-bot: I haz the power, 2016-08-25 17:59:15]

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

[rmistry, 2016-08-26 11:43:53]

rmistry@google.com changed reviewers:
+ halcanary@chromium.org

[nednguyen, 2016-08-26 15:06:35]

On 2016/08/26 11:43:54, rmistry wrote:

Hi Ravi, sorry for the delay on this. I am ooo at the moment and will get back
to this on monday.

From a high level view, can you create a folder benchmarks/cluster_benchmarks/ &
put all the benchmarks related to ct to that folder?

[hal.canary, 2016-08-26 15:10:33]

halcanary@google.com changed reviewers:
+ halcanary@google.com

[hal.canary, 2016-08-26 15:10:34]

lgtm

[rmistry, 2016-08-29 12:49:51]

On 2016/08/26 15:06:35, nednguyen (ooo til 8-29) wrote:
> On 2016/08/26 11:43:54, rmistry wrote:
> 
> Hi Ravi, sorry for the delay on this. I am ooo at the moment and will get back
> to this on monday.
> 
> From a high level view, can you create a folder benchmarks/cluster_benchmarks/
&
> put all the benchmarks related to ct to that folder?

I would prefer if these benchmarks stay in the top level benchmarks/ dir
because:
* They are not really only CT benchmarks. skpicture_printer (and soon
multipage_skpicture_printer) is actually used by a recipe in our waterfall to
create SKPs weekly. That flow has nothing to do with CT.
* There are no existing benchmarks that are only CT benchmarks. A lot of
existing benchmarks can support being run on CT by having a separate CT suffixed
class with calls to ct_benchmarks_util.

[nednguyen, 2016-08-29 13:42:38]

On 2016/08/29 12:49:51, rmistry wrote:
> On 2016/08/26 15:06:35, nednguyen (ooo til 8-29) wrote:
> > On 2016/08/26 11:43:54, rmistry wrote:
> > 
> > Hi Ravi, sorry for the delay on this. I am ooo at the moment and will get
back
> > to this on monday.
> > 
> > From a high level view, can you create a folder
benchmarks/cluster_benchmarks/
> &
> > put all the benchmarks related to ct to that folder?
> 
> I would prefer if these benchmarks stay in the top level benchmarks/ dir
> because:
> * They are not really only CT benchmarks. skpicture_printer (and soon
> multipage_skpicture_printer) is actually used by a recipe in our waterfall to
> create SKPs weekly. That flow has nothing to do with CT.
> * There are no existing benchmarks that are only CT benchmarks. A lot of
> existing benchmarks can support being run on CT by having a separate CT
suffixed
> class with calls to ct_benchmarks_util.

I see. From a high level point of view, can you explain how is this different
from  skpicture_printer? I also see that the new code is almost the same as the
existing ones, except for the javascript to be evaluated. Can we parameterize
the existing SkpicturePrinter measurement's contructor with multi_page param &
move the new MultipageSkpicturePrinter( benchmarks to the same
skpicture_printer.py file?

[rmistry, 2016-08-29 13:54:58]

On 2016/08/29 13:42:38, nednguyen wrote:
> On 2016/08/29 12:49:51, rmistry wrote:
> > On 2016/08/26 15:06:35, nednguyen (ooo til 8-29) wrote:
> > > On 2016/08/26 11:43:54, rmistry wrote:
> > > 
> > > Hi Ravi, sorry for the delay on this. I am ooo at the moment and will get
> back
> > > to this on monday.
> > > 
> > > From a high level view, can you create a folder
> benchmarks/cluster_benchmarks/
> > &
> > > put all the benchmarks related to ct to that folder?
> > 
> > I would prefer if these benchmarks stay in the top level benchmarks/ dir
> > because:
> > * They are not really only CT benchmarks. skpicture_printer (and soon
> > multipage_skpicture_printer) is actually used by a recipe in our waterfall
to
> > create SKPs weekly. That flow has nothing to do with CT.
> > * There are no existing benchmarks that are only CT benchmarks. A lot of
> > existing benchmarks can support being run on CT by having a separate CT
> suffixed
> > class with calls to ct_benchmarks_util.
> 
> I see. From a high level point of view, can you explain how is this different
> from  skpicture_printer? I also see that the new code is almost the same as
the
> existing ones, except for the javascript to be evaluated. Can we parameterize
> the existing SkpicturePrinter measurement's contructor with multi_page param &
> move the new MultipageSkpicturePrinter( benchmarks to the same
> skpicture_printer.py file?

Right, javascript is different and the output is different.
I started out parameterizing the existing measurement but then went back to
keeping things separate because things may further diverge in some way. I was
thinking I would clean things up when we have a better idea of what we use case
for the new measurement is.

[nednguyen, 2016-08-29 14:00:41]

lgtm

https://codereview.chromium.org/2278653004/diff/40001/tools/perf/benchmarks/m...
File tools/perf/benchmarks/multipage_skpicture_printer.py (right):

https://codereview.chromium.org/2278653004/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/multipage_skpicture_printer.py:25:
@benchmark.Disabled('all')
nits: can you add documentation explaining why we disable this?

[rmistry, 2016-08-29 14:24:55]

https://codereview.chromium.org/2278653004/diff/40001/tools/perf/benchmarks/m...
File tools/perf/benchmarks/multipage_skpicture_printer.py (right):

https://codereview.chromium.org/2278653004/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/multipage_skpicture_printer.py:25:
@benchmark.Disabled('all')
On 2016/08/29 14:00:41, nednguyen wrote:
> nits: can you add documentation explaining why we disable this?

Done. Also made change to skpicture_printer.py

[rmistry, 2016-08-29 15:20:52]

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

[commit-bot: I haz the power, 2016-08-29 15:21:39]

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

[rmistry, 2016-08-29 15:28:05]

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

[commit-bot: I haz the power, 2016-08-29 15:28:24]

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, 2016-08-29 17:01:05]

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

[commit-bot: I haz the power, 2016-08-29 17:01:06]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[rmistry, 2016-08-29 17:04:48]

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

[commit-bot: I haz the power, 2016-08-29 17:15:58]

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, 2016-08-29 18:00:19]

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

[commit-bot: I haz the power, 2016-08-29 18:00:20]

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

[rmistry, 2016-08-29 18:12:37]

The CQ bit was checked by rmistry@google.com

[rmistry, 2016-08-29 18:12:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, halcanary@google.com
Link to the patchset: https://codereview.chromium.org/2278653004/#ps80001
(title: "Remove unused imports")

[commit-bot: I haz the power, 2016-08-29 18:25:55]

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

[rmistry, 2016-08-29 20:06:57]

On 2016/08/29 18:25:55, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

If there are no objections I am going to skip android_s5_perf_cq since it does
not seem to be picking up any jobs.

[nednguyen, 2016-08-29 20:18:01]

On 2016/08/29 20:06:57, rmistry wrote:
> On 2016/08/29 18:25:55, commit-bot: I haz the power wrote:
> > CQ is trying da patch. Follow status at
> >  
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> 
> If there are no objections I am going to skip android_s5_perf_cq since it does
> not seem to be picking up any jobs.

lgtm
Feel free to ignore that bot

[rmistry, 2016-08-29 20:24:23]

Description was changed from

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

to

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

[rmistry, 2016-08-29 20:24:50]

The CQ bit was unchecked by rmistry@google.com

[rmistry, 2016-08-29 20:24:50]

The CQ bit was checked by rmistry@google.com

[commit-bot: I haz the power, 2016-08-29 20:47:13]

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-08-30 04:48:13]

Description was changed from

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

to

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

[commit-bot: I haz the power, 2016-08-30 04:48:14]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-30 04:50:48]

Description was changed from

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

to

==========
Add new multipage_skpicture_printer benchmark and measurement

BUG=chromium:641003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

Committed: https://crrev.com/b40b6e680dcca6b9a7bbfe31455d3e05d9b0c9ab
Cr-Commit-Position: refs/heads/master@{#415115}
==========

[commit-bot: I haz the power, 2016-08-30 04:51:09]

Patchset 5 (id:??) landed as
https://crrev.com/b40b6e680dcca6b9a7bbfe31455d3e05d9b0c9ab
Cr-Commit-Position: refs/heads/master@{#415115}