[Telemetry] Change default BattOr benchmark chrome tracing categories to be rail.

[Telemetry] Change default BattOr benchmark chrome tracing categories to be rail.

BUG=637158
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

Committed: https://crrev.com/2cbfbe277ed9b712ac9b28763cd1e247bc5e89f8
Cr-Commit-Position: refs/heads/master@{#413874}

[rnephew (Reviews Here), 2016-08-22 14:00:44]

Description was changed from

==========
[Telemetry] Change default BattOr benchmark chrome tracing categories to be
rail.

BUG=637158
==========

to

==========
[Telemetry] Change default BattOr benchmark chrome tracing categories to be
rail.

BUG=637158
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
==========

[rnephew (Reviews Here), 2016-08-22 14:01:39]

rnephew@chromium.org changed reviewers:
+ alexandermont@chromium.org, nednguyen@google.com

[rnephew (Reviews Here), 2016-08-22 18:05:12]

On 2016/08/22 14:01:40, rnephew (Reviews Here) wrote:

Note: 
After https://codereview.chromium.org/2261713003/ lands I will need to make
changes to this CL to reflect that 'rail' category will be default for all
battor tests.

[alexandermont, 2016-08-22 18:12:58]

lgtm

[nednguyen, 2016-08-22 18:30:24]

lgtm

[rnephew (Reviews Here), 2016-08-22 19:11:46]

On 2016/08/22 18:30:24, nednguyen wrote:
> lgtm

Landing this without the change mentioned before so that it will land faster
(since the other hasn't landed yet). Will have a follow up CL getting rid of the
repetitive code.

[rnephew (Reviews Here), 2016-08-22 19:11:52]

The CQ bit was checked by rnephew@chromium.org

[commit-bot: I haz the power, 2016-08-22 19:12:52]

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-22 21:00:14]

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

[commit-bot: I haz the power, 2016-08-22 21:00:14]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[nednguyen, 2016-08-22 21:07:07]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2016-08-22 21:08:57]

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-22 21:15:53]

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

[commit-bot: I haz the power, 2016-08-22 21:15:54]

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

[rnephew (Reviews Here), 2016-08-22 21:17:01]

The CQ bit was checked by rnephew@chromium.org

[commit-bot: I haz the power, 2016-08-22 21:18:12]

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-22 23:20:02]

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

[commit-bot: I haz the power, 2016-08-22 23:20:03]

Exceeded global retry quota

[rnephew (Reviews Here), 2016-08-22 23:22:52]

The CQ bit was checked by rnephew@chromium.org

[commit-bot: I haz the power, 2016-08-22 23:23:32]

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-23 03:56:36]

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

[commit-bot: I haz the power, 2016-08-23 03:56:37]

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

[rnephew (Reviews Here), 2016-08-23 14:00:25]

The CQ bit was checked by rnephew@chromium.org

[commit-bot: I haz the power, 2016-08-23 14:00:40]

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

[rnephew (Reviews Here), 2016-08-23 16:21:04]

On 2016/08/23 14:00:40, 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...

This doesn't appear to get rid of the OOM problem. Alex, is it correctly
enabling rail-only traces?

[nednguyen, 2016-08-23 17:19:28]

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

[nednguyen, 2016-08-23 17:19:28]

Zhen, halppp!

https://codereview.chromium.org/2265083002/diff/1/tools/perf/benchmarks/batto...
File tools/perf/benchmarks/battor.py (right):

https://codereview.chromium.org/2265083002/diff/1/tools/perf/benchmarks/batto...
tools/perf/benchmarks/battor.py:18:
chrome_trace_category_filter.ChromeTraceCategoryFilter())
Oh, this may mean all the things by default.

+Zhen will know the most up to date way to specify "only use rail" category.

[alexandermont, 2016-08-23 17:20:20]

On 2016/08/23 at 16:21:04, rnephew wrote:
> On 2016/08/23 14:00:40, 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...
> 
> This doesn't appear to get rid of the OOM problem. Alex, is it correctly
enabling rail-only traces?

It looks like this is the correct syntax for rail-only traces. (Alternatively
you can do:

    options.config.chrome_trace_config.SetCategoryFilter(
      chrome_trace_category_filter.ChromeTraceCategoryFilter('rail')))

but the way you did it should be fine.

I don't see any OOM errors in the logs from the failed tryjobs here though. I
just see some "Invalid ProtoExpectation" errors. Maybe that is the problem? Are
you sure the problem is OOM? (If so, can you point me to where in the log the
OOM error appears? Maybe there's an OOM problem somewhere else in the code as
well.)

[rnephew (Reviews Here), 2016-08-23 17:25:54]

> It looks like this is the correct syntax for rail-only traces. (Alternatively
> you can do:
> 
>     options.config.chrome_trace_config.SetCategoryFilter(
>       chrome_trace_category_filter.ChromeTraceCategoryFilter('rail')))
> 
> but the way you did it should be fine.
> 
> I don't see any OOM errors in the logs from the failed tryjobs here though. I
> just see some "Invalid ProtoExpectation" errors. Maybe that is the problem?
Are
> you sure the problem is OOM? (If so, can you point me to where in the log the
> OOM error appears? Maybe there's an OOM problem somewhere else in the code as
> well.)

https://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf...

[  FAILED  ] /tmp/tmpYeTbzO.html (47445 ms)
Traceback (most recent call last):
  File
"/b/c/b/mac_retina_perf_cq/src/third_party/catapult/telemetry/telemetry/value/failure.py",
line 41, in _GetExcInfoFromMessage
    raise Exception(message)
Exception: Unknown stack


This is how th eOOM error manifests, correct?

[alexandermont, 2016-08-23 17:25:57]

On 2016/08/23 at 17:20:20, alexandermont wrote:
> On 2016/08/23 at 16:21:04, rnephew wrote:
> > On 2016/08/23 14:00:40, 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...
> > 
> > This doesn't appear to get rid of the OOM problem. Alex, is it correctly
enabling rail-only traces?
> 
> It looks like this is the correct syntax for rail-only traces. (Alternatively
you can do:
> 
>     options.config.chrome_trace_config.SetCategoryFilter(
>       chrome_trace_category_filter.ChromeTraceCategoryFilter('rail')))
> 
> but the way you did it should be fine.
> 
> I don't see any OOM errors in the logs from the failed tryjobs here though. I
just see some "Invalid ProtoExpectation" errors. Maybe that is the problem? Are
you sure the problem is OOM? (If so, can you point me to where in the log the
OOM error appears? Maybe there's an OOM problem somewhere else in the code as
well.)

Actually, never mind that. I see exactly what the problem is. The problem is the
line a few lines below this,
options.config.chrome_trace_config.SetDefaultOverheadFilter(), which wipes out
what you just set above and instead sets it to the default tracing categories.
You want to keep what you have but get rid of the line
options.config.chrome_trace_config.SetDefaultOverheadFilter()

[rnephew (Reviews Here), 2016-08-23 17:29:54]

On 2016/08/23 17:25:57, alexandermont wrote:
> On 2016/08/23 at 17:20:20, alexandermont wrote:
> > On 2016/08/23 at 16:21:04, rnephew wrote:
> > > On 2016/08/23 14:00:40, 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...
> > > 
> > > This doesn't appear to get rid of the OOM problem. Alex, is it correctly
> enabling rail-only traces?
> > 
> > It looks like this is the correct syntax for rail-only traces.
(Alternatively
> you can do:
> > 
> >     options.config.chrome_trace_config.SetCategoryFilter(
> >       chrome_trace_category_filter.ChromeTraceCategoryFilter('rail')))
> > 
> > but the way you did it should be fine.
> > 
> > I don't see any OOM errors in the logs from the failed tryjobs here though.
I
> just see some "Invalid ProtoExpectation" errors. Maybe that is the problem?
Are
> you sure the problem is OOM? (If so, can you point me to where in the log the
> OOM error appears? Maybe there's an OOM problem somewhere else in the code as
> well.)
> 
> Actually, never mind that. I see exactly what the problem is. The problem is
the
> line a few lines below this,
> options.config.chrome_trace_config.SetDefaultOverheadFilter(), which wipes out
> what you just set above and instead sets it to the default tracing categories.
> You want to keep what you have but get rid of the line
> options.config.chrome_trace_config.SetDefaultOverheadFilter()

Tested locally, trace is much smaller. Thanks for catching that!

[rnephew (Reviews Here), 2016-08-23 17:30:04]

The CQ bit was checked by rnephew@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 17:30:31]

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

[Zhen Wang, 2016-08-23 17:39:40]

> It looks like this is the correct syntax for rail-only traces. (Alternatively
> you can do:
> 
>     options.config.chrome_trace_config.SetCategoryFilter(
>       chrome_trace_category_filter.ChromeTraceCategoryFilter('rail')))
> 
> but the way you did it should be fine.

I agree with Alex. Looks like the problem is solved?

[rnephew (Reviews Here), 2016-08-23 17:40:59]

On 2016/08/23 17:39:40, Zhen Wang wrote:
> > It looks like this is the correct syntax for rail-only traces.
(Alternatively
> > you can do:
> > 
> >     options.config.chrome_trace_config.SetCategoryFilter(
> >       chrome_trace_category_filter.ChromeTraceCategoryFilter('rail')))
> > 
> > but the way you did it should be fine.
> 
> I agree with Alex. Looks like the problem is solved?

Yep, should be. Running git cl try now. Will commit if those all pass, since the
change since last LGTM is small.

[commit-bot: I haz the power, 2016-08-23 19:32:39]

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

[commit-bot: I haz the power, 2016-08-23 19:32:39]

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

[rnephew (Reviews Here), 2016-08-23 22:39:46]

The CQ bit was checked by rnephew@chromium.org

[rnephew (Reviews Here), 2016-08-23 22:39:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, alexandermont@chromium.org
Link to the patchset: https://codereview.chromium.org/2265083002/#ps20001
(title: "get rid of filter override")

[commit-bot: I haz the power, 2016-08-23 22:40:46]

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-23 22:47:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-23 22:48:39]

Description was changed from

==========
[Telemetry] Change default BattOr benchmark chrome tracing categories to be
rail.

BUG=637158
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

==========
[Telemetry] Change default BattOr benchmark chrome tracing categories to be
rail.

BUG=637158
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

Committed: https://crrev.com/2cbfbe277ed9b712ac9b28763cd1e247bc5e89f8
Cr-Commit-Position: refs/heads/master@{#413874}
==========

[commit-bot: I haz the power, 2016-08-23 22:48:40]

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