Add media.tough_video_cases_reduced

Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect

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

[Ting-Yuan (Leo) Huang, 2016-01-22 02:50:22]

Description was changed from

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
==========

to

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect
==========

[Ting-Yuan (Leo) Huang, 2016-01-22 02:55:56]

Description was changed from

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect
==========

to

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect
==========

[Ting-Yuan (Leo) Huang, 2016-01-22 02:55:56]

laszio@chromium.org changed reviewers:
+ achuith@chromium.org

[Ting-Yuan (Leo) Huang, 2016-01-22 03:02:21]

laszio@chromium.org changed reviewers:
+ cmtice@chromium.org

[Ting-Yuan (Leo) Huang, 2016-01-22 03:02:22]

This CL reduces 30% of time required by media.tough_video_cases.

Because some of the nightly tests only look at time_to_play,
media.tough_video_cases_reduced is a subset of media.tough_video_cases and it
only contains cases that reports time_to_play.

[achuithb, 2016-01-22 19:21:01]

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

[achuithb, 2016-01-22 19:21:01]

Ned, does this seem ok?

[nednguyen, 2016-01-22 19:25:18]

On 2016/01/22 19:21:01, achuithb wrote:
> Ned, does this seem ok?

This seems like a waste of bot time to me. Why do create a new benchmark that
just have a subset of pages from an existing benchmark?

[cmtice1, 2016-01-22 21:15:16]

On 2016/01/22 19:25:18, nednguyen wrote:
> On 2016/01/22 19:21:01, achuithb wrote:
> > Ned, does this seem ok?
> 
> This seems like a waste of bot time to me. Why do create a new benchmark that
> just have a subset of pages from an existing benchmark?

Because the subset is the only part that returns values we are interested in and
using the subset only takes half the time to run that using the complete set
does, which matters a lot to us, because we run many iterations with multiple
images.

[nednguyen, 2016-01-22 21:16:19]

On 2016/01/22 21:15:16, cmtice1 wrote:
> On 2016/01/22 19:25:18, nednguyen wrote:
> > On 2016/01/22 19:21:01, achuithb wrote:
> > > Ned, does this seem ok?
> > 
> > This seems like a waste of bot time to me. Why do create a new benchmark
that
> > just have a subset of pages from an existing benchmark?
> 
> Because the subset is the only part that returns values we are interested in
and
> using the subset only takes half the time to run that using the complete set
> does, which matters a lot to us, because we run many iterations with multiple
> images.

Can you remove this subset from original benchmark?

[cmtice1, 2016-01-22 21:23:28]

On 2016/01/22 21:16:19, nednguyen wrote:
> On 2016/01/22 21:15:16, cmtice1 wrote:
> > On 2016/01/22 19:25:18, nednguyen wrote:
> > > On 2016/01/22 19:21:01, achuithb wrote:
> > > > Ned, does this seem ok?
> > > 
> > > This seems like a waste of bot time to me. Why do create a new benchmark
> that
> > > just have a subset of pages from an existing benchmark?
> > 
> > Because the subset is the only part that returns values we are interested in
> and
> > using the subset only takes half the time to run that using the complete set
> > does, which matters a lot to us, because we run many iterations with
multiple
> > images.
> 
> Can you remove this subset from original benchmark?

Isn't that what this CL does?  I'm not sure I understand your comment.

[nednguyen, 2016-01-22 21:25:32]

On 2016/01/22 21:23:28, cmtice1 wrote:
> On 2016/01/22 21:16:19, nednguyen wrote:
> > On 2016/01/22 21:15:16, cmtice1 wrote:
> > > On 2016/01/22 19:25:18, nednguyen wrote:
> > > > On 2016/01/22 19:21:01, achuithb wrote:
> > > > > Ned, does this seem ok?
> > > > 
> > > > This seems like a waste of bot time to me. Why do create a new benchmark
> > that
> > > > just have a subset of pages from an existing benchmark?
> > > 
> > > Because the subset is the only part that returns values we are interested
in
> > and
> > > using the subset only takes half the time to run that using the complete
set
> > > does, which matters a lot to us, because we run many iterations with
> multiple
> > > images.
> > 
> > Can you remove this subset from original benchmark?
> 
> Isn't that what this CL does?  I'm not sure I understand your comment.

I mean removing these duplicate pages from ToughVideoCasesPageSet. If
ToughVideoCasesReducedPageSet already give you the data for those pages, there
is no need to repeat them in ToughVideoCasesPageSet.

Also consider renaming the two pagesets

[cmtice1, 2016-01-22 21:29:05]

On 2016/01/22 21:25:32, nednguyen wrote:
> On 2016/01/22 21:23:28, cmtice1 wrote:
> > On 2016/01/22 21:16:19, nednguyen wrote:
> > > On 2016/01/22 21:15:16, cmtice1 wrote:
> > > > On 2016/01/22 19:25:18, nednguyen wrote:
> > > > > On 2016/01/22 19:21:01, achuithb wrote:
> > > > > > Ned, does this seem ok?
> > > > > 
> > > > > This seems like a waste of bot time to me. Why do create a new
benchmark
> > > that
> > > > > just have a subset of pages from an existing benchmark?
> > > > 
> > > > Because the subset is the only part that returns values we are
interested
> in
> > > and
> > > > using the subset only takes half the time to run that using the complete
> set
> > > > does, which matters a lot to us, because we run many iterations with
> > multiple
> > > > images.
> > > 
> > > Can you remove this subset from original benchmark?
> > 
> > Isn't that what this CL does?  I'm not sure I understand your comment.
> 
> I mean removing these duplicate pages from ToughVideoCasesPageSet. If
> ToughVideoCasesReducedPageSet already give you the data for those pages, there
> is no need to repeat them in ToughVideoCasesPageSet.
> 
> Also consider renaming the two pagesets

So if I understand what you are suggesting, we should break
media.tough_video_cases into two  tests, one of which contains the part we are
interested in and the other contains the rest, rather than just creating a
subset as we currently have done?

[nednguyen, 2016-01-22 21:30:25]

On 2016/01/22 21:29:05, cmtice1 wrote:
> On 2016/01/22 21:25:32, nednguyen wrote:
> > On 2016/01/22 21:23:28, cmtice1 wrote:
> > > On 2016/01/22 21:16:19, nednguyen wrote:
> > > > On 2016/01/22 21:15:16, cmtice1 wrote:
> > > > > On 2016/01/22 19:25:18, nednguyen wrote:
> > > > > > On 2016/01/22 19:21:01, achuithb wrote:
> > > > > > > Ned, does this seem ok?
> > > > > > 
> > > > > > This seems like a waste of bot time to me. Why do create a new
> benchmark
> > > > that
> > > > > > just have a subset of pages from an existing benchmark?
> > > > > 
> > > > > Because the subset is the only part that returns values we are
> interested
> > in
> > > > and
> > > > > using the subset only takes half the time to run that using the
complete
> > set
> > > > > does, which matters a lot to us, because we run many iterations with
> > > multiple
> > > > > images.
> > > > 
> > > > Can you remove this subset from original benchmark?
> > > 
> > > Isn't that what this CL does?  I'm not sure I understand your comment.
> > 
> > I mean removing these duplicate pages from ToughVideoCasesPageSet. If
> > ToughVideoCasesReducedPageSet already give you the data for those pages,
there
> > is no need to repeat them in ToughVideoCasesPageSet.
> > 
> > Also consider renaming the two pagesets
> 
> So if I understand what you are suggesting, we should break
> media.tough_video_cases into two  tests, one of which contains the part we are
> interested in and the other contains the rest, rather than just creating a
> subset as we currently have done?

Correct.

[Ting-Yuan (Leo) Huang, 2016-01-25 07:53:32]

Done. In patch set 2, tests that don't report time_to_play are moved into
media.tough_video_cases_extra.

[nednguyen, 2016-01-25 16:11:36]

On 2016/01/25 07:53:32, Ting-Yuan (Leo) Huang wrote:
> Done. In patch set 2, tests that don't report time_to_play are moved into
> media.tough_video_cases_extra.

lgtm

But can you make a subsequent patch to rename the pages? Pages in
ToughVideoCasesPageSet & ToughVideoCasesExtraPageSet should be rename to be
contiguous.

[Ting-Yuan (Leo) Huang, 2016-01-26 04:53:15]

On 2016/01/25 16:11:36, nednguyen wrote:
> On 2016/01/25 07:53:32, Ting-Yuan (Leo) Huang wrote:
> > Done. In patch set 2, tests that don't report time_to_play are moved into
> > media.tough_video_cases_extra.
> 
> lgtm
> 
> But can you make a subsequent patch to rename the pages? Pages in
> ToughVideoCasesPageSet & ToughVideoCasesExtraPageSet should be rename to be
> contiguous.

Done. Pages in ToughVideoCasesPageSet / ToughVideoCasesExtraPageSet are renamed
to Page[1-26] / Page[27-40] respectively. I also reordered them so that Page2
follows Page1 and so on in the source code. The patch may look a little bit
messy, though.

[nednguyen, 2016-01-26 15:45:24]

On 2016/01/26 04:53:15, Ting-Yuan (Leo) Huang wrote:
> On 2016/01/25 16:11:36, nednguyen wrote:
> > On 2016/01/25 07:53:32, Ting-Yuan (Leo) Huang wrote:
> > > Done. In patch set 2, tests that don't report time_to_play are moved into
> > > media.tough_video_cases_extra.
> > 
> > lgtm
> > 
> > But can you make a subsequent patch to rename the pages? Pages in
> > ToughVideoCasesPageSet & ToughVideoCasesExtraPageSet should be rename to be
> > contiguous.
> 
> Done. Pages in ToughVideoCasesPageSet / ToughVideoCasesExtraPageSet are
renamed
> to Page[1-26] / Page[27-40] respectively. I also reordered them so that Page2
> follows Page1 and so on in the source code. The patch may look a little bit
> messy, though.

Err, I would suggest land patch set 2, and do patch set 3 in a subsequent patch.
The more moving pieces in one patch, the riskier it's.

[Ting-Yuan (Leo) Huang, 2016-01-29 07:58:20]

Patchset #3 (id:40001) has been deleted

[Ting-Yuan (Leo) Huang, 2016-01-29 07:59:36]

The CQ bit was checked by laszio@chromium.org

[commit-bot: I haz the power, 2016-01-29 07:59:55]

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

[Ting-Yuan (Leo) Huang, 2016-01-29 08:02:18]

On 2016/01/26 15:45:24, nednguyen wrote:
> On 2016/01/26 04:53:15, Ting-Yuan (Leo) Huang wrote:
> > On 2016/01/25 16:11:36, nednguyen wrote:
> > > On 2016/01/25 07:53:32, Ting-Yuan (Leo) Huang wrote:
> > > > Done. In patch set 2, tests that don't report time_to_play are moved
into
> > > > media.tough_video_cases_extra.
> > > 
> > > lgtm
> > > 
> > > But can you make a subsequent patch to rename the pages? Pages in
> > > ToughVideoCasesPageSet & ToughVideoCasesExtraPageSet should be rename to
be
> > > contiguous.
> > 
> > Done. Pages in ToughVideoCasesPageSet / ToughVideoCasesExtraPageSet are
> renamed
> > to Page[1-26] / Page[27-40] respectively. I also reordered them so that
Page2
> > follows Page1 and so on in the source code. The patch may look a little bit
> > messy, though.
> 
> Err, I would suggest land patch set 2, and do patch set 3 in a subsequent
patch.
> The more moving pieces in one patch, the riskier it's.

Alright, I'll do a subsequent patch after patch set 2 is landed.

[commit-bot: I haz the power, 2016-01-29 10:14:15]

Description was changed from

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect
==========

to

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect
==========

[commit-bot: I haz the power, 2016-01-29 10:14:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-29 10:15:14]

Description was changed from

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect
==========

to

==========
Add media.tough_video_cases_reduced

media.tough_video_cases_reduced is a subset of media.tough_video_cases.
It contains only cases which report time_to_play.

BUG=crbug.com/565180
TEST=./tools/perf/run_benchmarks --browser=cros-chrome --remote=[dut] \
     media.tough_video_cases_reduced
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect

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

[commit-bot: I haz the power, 2016-01-29 10:15:15]

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