Port memory.long_running_idle_gmail benchmarks to TBMv2.

Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2
and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2

BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq

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

[ulan, 2016-04-27 15:58:27]

Description was changed from

==========
Port memory.long_running_idle_gmail* benchmarks to TBMv2.

BUG=chromium:593793
==========

to

==========
Port memory.long_running_idle_gmail* benchmarks to TBMv2.

BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

[ulan, 2016-04-27 16:02:59]

Description was changed from

==========
Port memory.long_running_idle_gmail* benchmarks to TBMv2.

BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

to

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with memory.long_running_idle_gmail_tbmv2

and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

[ulan, 2016-04-27 16:03:11]

Description was changed from

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with memory.long_running_idle_gmail_tbmv2

and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

to

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2

and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

[ulan, 2016-04-27 16:03:19]

Description was changed from

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2

and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

to

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2
and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

[ulan, 2016-04-27 16:04:05]

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

[ulan, 2016-04-27 16:04:06]

PTAL

[nednguyen, 2016-04-27 16:05:45]

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

[nednguyen, 2016-04-27 16:05:46]

lgtm but Petr may have some comment about how you set up the dumps.

[petrcermak, 2016-04-27 16:22:17]

LGTM with a few comments, but please wait until
https://github.com/catapult-project/catapult/commit/c3f191e1307c73e31ad29a2f9...
rolls into Chromium (should happen within the next few hours). Otherwise, your
patch will most likely break the bots (see
https://bugs.chromium.org/p/chromium/issues/detail?id=606698).

Thanks,
Petr

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

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:122: v8_categories = [
v8_categories and memory_categories could be class constants.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:125: all_categories = v8_categories +
memory_categories
1) Note that this will contain 'blink.console' twice. To avoid this, I recommend
that |v8_categories| and |memory_categories| are sets instead.
2) You probably want to make sure that everything else is disabled (see
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...).
To do this, I suggest you do: ','.join(['-*'] + list(v8_categories +
memory_categories)).
3) I would inline |all_categories| because it just seems to add unnecessary
indirection (feel free to ignore).

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:133: 
nit: add one more blank line here (there should be 2 blank lines between
top-level definitions)

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:147: 
ditto: add extra blank line

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/page_sets/lo...
File tools/perf/page_sets/long_running_idle_google_cases.py (right):

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/page_sets/lo...
tools/perf/page_sets/long_running_idle_google_cases.py:14: steps =
IDLE_TIME_IN_SECONDS / SAMPLING_INTERVAL_IN_SECONDS
why not make this a global constant as well?

[petrcermak, 2016-04-27 16:22:35]

petrcermak@chromium.org changed reviewers:
+ perezju@chromium.org

[petrcermak, 2016-04-27 16:22:35]

+cc perezju

[perezju, 2016-04-27 16:29:43]

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

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:131: def
SetupBenchmarkDefaultTraceRerunOptions(self, tbm_options):
we're getting rid of these TraceRerun* methods, just move this line of code to
the bottom of CreateTimelineBasedMeasurementOptions.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:137: # For 'android' see
http://crbug.com/579546.
That bug shows that the (non-tbm) benchmark was re-enabled on Android. Could we
also try to re-enable this one? (could be a follow up CL)

[ulan, 2016-04-28 14:34:35]

Thanks!

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

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:122: v8_categories = [
On 2016/04/27 16:22:17, petrcermak wrote:
> v8_categories and memory_categories could be class constants.

Since they are not reused, I'd prefer to keep them in the smallest possible
scope.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:125: all_categories = v8_categories +
memory_categories
On 2016/04/27 16:22:17, petrcermak wrote:
> 1) Note that this will contain 'blink.console' twice. To avoid this, I
recommend
> that |v8_categories| and |memory_categories| are sets instead.
> 2) You probably want to make sure that everything else is disabled (see
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchma...).
> To do this, I suggest you do: ','.join(['-*'] + list(v8_categories +
> memory_categories)).
> 3) I would inline |all_categories| because it just seems to add unnecessary
> indirection (feel free to ignore).

2, 3 - done. TracingCategoryFilter can handle duplicates.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:131: def
SetupBenchmarkDefaultTraceRerunOptions(self, tbm_options):
On 2016/04/27 16:29:42, perezju wrote:
> we're getting rid of these TraceRerun* methods, just move this line of code to
> the bottom of CreateTimelineBasedMeasurementOptions.

I tried to that, but then _MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions
gets invoked and fails.

I'd rather not remove _MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions in
this CL, so I am keeping this function for now.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:133: 
On 2016/04/27 16:22:17, petrcermak wrote:
> nit: add one more blank line here (there should be 2 blank lines between
> top-level definitions)

Done.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:137: # For 'android' see
http://crbug.com/579546.
On 2016/04/27 16:29:43, perezju wrote:
> That bug shows that the (non-tbm) benchmark was re-enabled on Android. Could
we
> also try to re-enable this one? (could be a follow up CL)

Acknowledged. I will re-enable in a different CL.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:147: 
On 2016/04/27 16:22:17, petrcermak wrote:
> ditto: add extra blank line

Done.

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/page_sets/lo...
File tools/perf/page_sets/long_running_idle_google_cases.py (right):

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/page_sets/lo...
tools/perf/page_sets/long_running_idle_google_cases.py:14: steps =
IDLE_TIME_IN_SECONDS / SAMPLING_INTERVAL_IN_SECONDS
On 2016/04/27 16:22:17, petrcermak wrote:
> why not make this a global constant as well?

Done.

[petrcermak, 2016-04-28 14:38:45]

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

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:122: v8_categories = [
On 2016/04/28 14:34:35, ulan wrote:
> On 2016/04/27 16:22:17, petrcermak wrote:
> > v8_categories and memory_categories could be class constants.
> 
> Since they are not reused, I'd prefer to keep them in the smallest possible
> scope

if you prefix them with an underscore, they will technically be private to the
class.

[perezju, 2016-04-28 14:45:09]

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

https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
tools/perf/benchmarks/memory_infra.py:131: def
SetupBenchmarkDefaultTraceRerunOptions(self, tbm_options):
On 2016/04/28 14:34:35, ulan wrote:
> On 2016/04/27 16:29:42, perezju wrote:
> > we're getting rid of these TraceRerun* methods, just move this line of code
to
> > the bottom of CreateTimelineBasedMeasurementOptions.
> 
> I tried to that, but then _MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions
> gets invoked and fails.
> 
> I'd rather not remove _MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions in
> this CL, so I am keeping this function for now.

This is no longer true after Petr's CL
https://codereview.chromium.org/1920363003/

It's probably a good idea to rebase against that and test again, because I guess
if you leave this as is then it's going to break.

[ulan, 2016-04-28 14:50:08]

On 2016/04/28 14:45:09, perezju wrote:
>
https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
> File tools/perf/benchmarks/memory_infra.py (right):
> 
>
https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
> tools/perf/benchmarks/memory_infra.py:131: def
> SetupBenchmarkDefaultTraceRerunOptions(self, tbm_options):
> On 2016/04/28 14:34:35, ulan wrote:
> > On 2016/04/27 16:29:42, perezju wrote:
> > > we're getting rid of these TraceRerun* methods, just move this line of
code
> to
> > > the bottom of CreateTimelineBasedMeasurementOptions.
> > 
> > I tried to that, but then
_MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions
> > gets invoked and fails.
> > 
> > I'd rather not remove _MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions in
> > this CL, so I am keeping this function for now.
> 
> This is no longer true after Petr's CL
> https://codereview.chromium.org/1920363003/
> 
> It's probably a good idea to rebase against that and test again, because I
guess
> if you leave this as is then it's going to break.

Thanks! Will do.

> if you prefix them with an underscore, they will technically be private to the
> class.
OK, what is the advantage of moving them to the class level?

[petrcermak, 2016-04-28 14:57:17]

On 2016/04/28 14:50:08, ulan wrote:
> On 2016/04/28 14:45:09, perezju wrote:
> >
>
https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
> > File tools/perf/benchmarks/memory_infra.py (right):
> > 
> >
>
https://codereview.chromium.org/1931533002/diff/40001/tools/perf/benchmarks/m...
> > tools/perf/benchmarks/memory_infra.py:131: def
> > SetupBenchmarkDefaultTraceRerunOptions(self, tbm_options):
> > On 2016/04/28 14:34:35, ulan wrote:
> > > On 2016/04/27 16:29:42, perezju wrote:
> > > > we're getting rid of these TraceRerun* methods, just move this line of
> code
> > to
> > > > the bottom of CreateTimelineBasedMeasurementOptions.
> > > 
> > > I tried to that, but then
> _MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions
> > > gets invoked and fails.
> > > 
> > > I'd rather not remove _MemoryInfra.SetupBenchmarkDefaultTraceRerunOptions
in
> > > this CL, so I am keeping this function for now.
> > 
> > This is no longer true after Petr's CL
> > https://codereview.chromium.org/1920363003/
> > 
> > It's probably a good idea to rebase against that and test again, because I
> guess
> > if you leave this as is then it's going to break.
> 
> Thanks! Will do.
> 
> > if you prefix them with an underscore, they will technically be private to
the
> > class.
> OK, what is the advantage of moving them to the class level?

I personally find it better to store constants as constants (rather than
temporary local variables). However, it's just a matter of personal taste, so
feel free to ignore this.

[ulan, 2016-04-28 15:27:47]

> I personally find it better to store constants as constants (rather than
> temporary local variables). However, it's just a matter of personal taste, so
> feel free to ignore this.
Thanks, I see your point.

[ulan, 2016-04-28 15:54:35]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-04-28 15:54:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
petrcermak@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/1931533002/#ps100001
(title: "Rebase and remove deprecated method")

[commit-bot: I haz the power, 2016-04-28 15:54:49]

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

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

Description was changed from

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2
and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

to

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2
and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

[commit-bot: I haz the power, 2016-04-28 19:06:21]

Committed patchset #6 (id:100001)

[shatch, 2016-04-29 15:31:27]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1935493003/ by simonhatch@chromium.org.

The reason for reverting is: All the windows bots going purple, suspect this CL.

BUG=607922.

[commit-bot: I haz the power, 2016-04-30 17:20:05]

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

[commit-bot: I haz the power, 2016-04-30 17:56:50]

Description was changed from

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2
and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2


BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq
==========

to

==========
Port memory.long_running_idle_gmail benchmarks to TBMv2.

This replaces:
memory.long_running_idle_gmail_tbm with
memory.long_running_idle_gmail_tbmv2
and
memory.long_running_idle_gmail_background with
memory.long_running_idle_gmail_background_tbmv2

BUG=chromium:593793
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:winx64_10_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq;tryserver.chromium.perf:linux_perf_cq

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