Use a single benchmark for WebRTC telemetry tests.

Use a single benchmark for WebRTC telemetry tests.

BUG=chromium:632295
R=nednguyen@google.com

[ehmaldonado_chromium, 2017-03-30 12:25:38]

Patchset #1 (id:1) has been deleted

[ehmaldonado_chromium, 2017-03-30 12:25:53]

Patchset #1 (id:20001) has been deleted

[ehmaldonado_chromium, 2017-03-30 12:26:26]

Please take a look.
Is this what you had in mind?

[nednguyen, 2017-03-30 13:14:08]

On 2017/03/30 12:26:26, ehmaldonado_chromium wrote:
> Please take a look.
> Is this what you had in mind?

I am excited by this CL :-) 

Though I want to check: how long would it take to run this whole benchmark?

[ehmaldonado_chromium, 2017-03-30 15:24:00]

On 2017/03/30 13:14:08, nednguyen wrote:
> On 2017/03/30 12:26:26, ehmaldonado_chromium wrote:
> > Please take a look.
> > Is this what you had in mind?
> 
> I am excited by this CL :-) 
> 
> Though I want to check: how long would it take to run this whole benchmark?

Now that I run it.
Took 17 minutes, and failed for the MultiplePeerConnections and DataChannel
pages with this error:

INFO:root:Chrome version: 2987
INFO:root:Trace sizes in bytes: {'traceEvents': 308193068, 'telemetry': 37720,
'tabIds': 40}
[ RUN      ] /tmp/tmp3I0jlr.html
TraceImportError: Invalid string length
    at Array.join (native)
    at Object.f [as string]
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:20494)
    at Object.c.transformTo
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:22247)
    at Object.c.transformTo
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:6414)
    at Function.GzipImporter.transformToString
(/tracing/extras/importer/gzip_importer.html:143:26)
    at Function.GzipImporter.inflateGzipData_
(/tracing/extras/importer/gzip_importer.html:118:31)
    at GzipImporter.extractSubtraces
(/tracing/extras/importer/gzip_importer.html:180:36)
    at addImportStage (/tracing/importer/import.html:162:40)
    at Task.run (/tracing/base/task.html:80:50)
    at Function.Task.RunSynchronously (/tracing/base/task.html:164:25)
[  FAILED  ] /tmp/tmp3I0jlr.html (14073 ms)
Traceback (most recent call last):
  File
"/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/telemetry/telemetry/value/failure.py",
line 41, in _GetExcInfoFromMessage
    raise Exception(message)
Exception: TraceImportError: Invalid string length
    at Array.join (native)
    at Object.f [as string]
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:20494)
    at Object.c.transformTo
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:22247)
    at Object.c.transformTo
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:6414)
    at Function.GzipImporter.transformToString
(/tracing/extras/importer/gzip_importer.html:143:26)
    at Function.GzipImporter.inflateGzipData_
(/tracing/extras/importer/gzip_importer.html:118:31)
    at GzipImporter.extractSubtraces
(/tracing/extras/importer/gzip_importer.html:180:36)
    at addImportStage (/tracing/importer/import.html:162:40)
    at Task.run (/tracing/base/task.html:80:50)
    at Function.Task.RunSynchronously (/tracing/base/task.html:164:25)

[  FAILED  ] multiple_peerconnections (207052 ms)

[ehmaldonado_chromium, 2017-03-30 15:24:14]

On 2017/03/30 15:24:00, ehmaldonado_chromium wrote:
> On 2017/03/30 13:14:08, nednguyen wrote:
> > On 2017/03/30 12:26:26, ehmaldonado_chromium wrote:
> > > Please take a look.
> > > Is this what you had in mind?
> > 
> > I am excited by this CL :-) 
> > 
> > Though I want to check: how long would it take to run this whole benchmark?
> 
> Now that I run it.
> Took 17 minutes, and failed for the MultiplePeerConnections and DataChannel
> pages with this error:
> 
> INFO:root:Chrome version: 2987
> INFO:root:Trace sizes in bytes: {'traceEvents': 308193068, 'telemetry': 37720,
> 'tabIds': 40}
> [ RUN      ] /tmp/tmp3I0jlr.html
> TraceImportError: Invalid string length
>     at Array.join (native)
>     at Object.f [as string]
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:20494)
>     at Object.c.transformTo
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:22247)
>     at Object.c.transformTo
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:6414)
>     at Function.GzipImporter.transformToString
> (/tracing/extras/importer/gzip_importer.html:143:26)
>     at Function.GzipImporter.inflateGzipData_
> (/tracing/extras/importer/gzip_importer.html:118:31)
>     at GzipImporter.extractSubtraces
> (/tracing/extras/importer/gzip_importer.html:180:36)
>     at addImportStage (/tracing/importer/import.html:162:40)
>     at Task.run (/tracing/base/task.html:80:50)
>     at Function.Task.RunSynchronously (/tracing/base/task.html:164:25)
> [  FAILED  ] /tmp/tmp3I0jlr.html (14073 ms)
> Traceback (most recent call last):
>   File
>
"/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/telemetry/telemetry/value/failure.py",
> line 41, in _GetExcInfoFromMessage
>     raise Exception(message)
> Exception: TraceImportError: Invalid string length
>     at Array.join (native)
>     at Object.f [as string]
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:20494)
>     at Object.c.transformTo
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:22247)
>     at Object.c.transformTo
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:6414)
>     at Function.GzipImporter.transformToString
> (/tracing/extras/importer/gzip_importer.html:143:26)
>     at Function.GzipImporter.inflateGzipData_
> (/tracing/extras/importer/gzip_importer.html:118:31)
>     at GzipImporter.extractSubtraces
> (/tracing/extras/importer/gzip_importer.html:180:36)
>     at addImportStage (/tracing/importer/import.html:162:40)
>     at Task.run (/tracing/base/task.html:80:50)
>     at Function.Task.RunSynchronously (/tracing/base/task.html:164:25)
> 
> [  FAILED  ] multiple_peerconnections (207052 ms)

I'm trying again.

[nednguyen, 2017-03-30 15:26:17]

On 2017/03/30 15:24:14, ehmaldonado_chromium wrote:
> On 2017/03/30 15:24:00, ehmaldonado_chromium wrote:
> > On 2017/03/30 13:14:08, nednguyen wrote:
> > > On 2017/03/30 12:26:26, ehmaldonado_chromium wrote:
> > > > Please take a look.
> > > > Is this what you had in mind?
> > > 
> > > I am excited by this CL :-) 
> > > 
> > > Though I want to check: how long would it take to run this whole
benchmark?
> > 
> > Now that I run it.
> > Took 17 minutes, and failed for the MultiplePeerConnections and DataChannel
> > pages with this error:
> > 
> > INFO:root:Chrome version: 2987
> > INFO:root:Trace sizes in bytes: {'traceEvents': 308193068, 'telemetry':
37720,
> > 'tabIds': 40}
> > [ RUN      ] /tmp/tmp3I0jlr.html
> > TraceImportError: Invalid string length
> >     at Array.join (native)
> >     at Object.f [as string]
> >
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:20494)
> >     at Object.c.transformTo
> >
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:22247)
> >     at Object.c.transformTo
> >
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:6414)
> >     at Function.GzipImporter.transformToString
> > (/tracing/extras/importer/gzip_importer.html:143:26)
> >     at Function.GzipImporter.inflateGzipData_
> > (/tracing/extras/importer/gzip_importer.html:118:31)
> >     at GzipImporter.extractSubtraces
> > (/tracing/extras/importer/gzip_importer.html:180:36)
> >     at addImportStage (/tracing/importer/import.html:162:40)
> >     at Task.run (/tracing/base/task.html:80:50)
> >     at Function.Task.RunSynchronously (/tracing/base/task.html:164:25)
> > [  FAILED  ] /tmp/tmp3I0jlr.html (14073 ms)
> > Traceback (most recent call last):
> >   File
> >
>
"/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/telemetry/telemetry/value/failure.py",
> > line 41, in _GetExcInfoFromMessage
> >     raise Exception(message)
> > Exception: TraceImportError: Invalid string length
> >     at Array.join (native)
> >     at Object.f [as string]
> >
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:20494)
> >     at Object.c.transformTo
> >
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:22247)
> >     at Object.c.transformTo
> >
>
(/usr/local/google/home/ehmaldonado/chromium/src/third_party/catapult/tracing/third_party/jszip/jszip.min.js:12:6414)
> >     at Function.GzipImporter.transformToString
> > (/tracing/extras/importer/gzip_importer.html:143:26)
> >     at Function.GzipImporter.inflateGzipData_
> > (/tracing/extras/importer/gzip_importer.html:118:31)
> >     at GzipImporter.extractSubtraces
> > (/tracing/extras/importer/gzip_importer.html:180:36)
> >     at addImportStage (/tracing/importer/import.html:162:40)
> >     at Task.run (/tracing/base/task.html:80:50)
> >     at Function.Task.RunSynchronously (/tracing/base/task.html:164:25)
> > 
> > [  FAILED  ] multiple_peerconnections (207052 ms)
> 
> I'm trying again.

That error seems like those pages are producing too big traces. Ehsan is working
on making the tracing import algorithm allows bigger trace size in
https://github.com/catapult-project/catapult/issues/2826

Though the fact that it didn't fail before & only fail now with your refactoring
seems a bit suspicious to me

[ehmaldonado_chromium, 2017-03-30 15:35:22]

> That error seems like those pages are producing too big traces. Ehsan is
working
> on making the tracing import algorithm allows bigger trace size in
> https://github.com/catapult-project/catapult/issues/2826
> 
> Though the fact that it didn't fail before & only fail now with your
refactoring
> seems a bit suspicious to me

They were not running on TBMv2 before.

[ehmaldonado_chromium, 2017-03-30 15:52:22]

On 2017/03/30 15:35:22, ehmaldonado_chromium wrote:
> > That error seems like those pages are producing too big traces. Ehsan is
> working
> > on making the tracing import algorithm allows bigger trace size in
> > https://github.com/catapult-project/catapult/issues/2826
> > 
> > Though the fact that it didn't fail before & only fail now with your
> refactoring
> > seems a bit suspicious to me
> 
> They were not running on TBMv2 before.

What about having two benchmarks? One for TBMv2 and another for legacy.

[ehmaldonado_chromium, 2017-03-30 15:52:24]

On 2017/03/30 15:35:22, ehmaldonado_chromium wrote:
> > That error seems like those pages are producing too big traces. Ehsan is
> working
> > on making the tracing import algorithm allows bigger trace size in
> > https://github.com/catapult-project/catapult/issues/2826
> > 
> > Though the fact that it didn't fail before & only fail now with your
> refactoring
> > seems a bit suspicious to me
> 
> They were not running on TBMv2 before.

What about having two benchmarks? One for TBMv2 and another for legacy.

[nednguyen, 2017-05-12 02:14:53]

On 2017/03/30 15:52:24, ehmaldonado_chromium wrote:
> On 2017/03/30 15:35:22, ehmaldonado_chromium wrote:
> > > That error seems like those pages are producing too big traces. Ehsan is
> > working
> > > on making the tracing import algorithm allows bigger trace size in
> > > https://github.com/catapult-project/catapult/issues/2826
> > > 
> > > Though the fact that it didn't fail before & only fail now with your
> > refactoring
> > > seems a bit suspicious to me
> > 
> > They were not running on TBMv2 before.
> 
> What about having two benchmarks? One for TBMv2 and another for legacy.

This CL is probably ready to land now?

[ehlesmes, 2017-05-12 06:09:00]

On 2017/05/12 02:14:53, nednguyen wrote:
> On 2017/03/30 15:52:24, ehmaldonado_chromium wrote:
> > On 2017/03/30 15:35:22, ehmaldonado_chromium wrote:
> > > > That error seems like those pages are producing too big traces. Ehsan is
> > > working
> > > > on making the tracing import algorithm allows bigger trace size in
> > > > https://github.com/catapult-project/catapult/issues/2826
> > > > 
> > > > Though the fact that it didn't fail before & only fail now with your
> > > refactoring
> > > > seems a bit suspicious to me
> > > 
> > > They were not running on TBMv2 before.
> > 
> > What about having two benchmarks? One for TBMv2 and another for legacy.
> 
> This CL is probably ready to land now?

I tried running it yesterday and the test browser crashed. Will try again today.

[ehmaldonado_chromium, 2017-05-12 07:24:08]

On 2017/05/12 06:09:00, ehlesmes wrote:
> On 2017/05/12 02:14:53, nednguyen wrote:
> > On 2017/03/30 15:52:24, ehmaldonado_chromium wrote:
> > > On 2017/03/30 15:35:22, ehmaldonado_chromium wrote:
> > > > > That error seems like those pages are producing too big traces. Ehsan
is
> > > > working
> > > > > on making the tracing import algorithm allows bigger trace size in
> > > > > https://github.com/catapult-project/catapult/issues/2826
> > > > > 
> > > > > Though the fact that it didn't fail before & only fail now with your
> > > > refactoring
> > > > > seems a bit suspicious to me
> > > > 
> > > > They were not running on TBMv2 before.
> > > 
> > > What about having two benchmarks? One for TBMv2 and another for legacy.
> > 
> > This CL is probably ready to land now?
> 
> I tried running it yesterday and the test browser crashed. Will try again
today.

Sorry, wrong account.

[ehmaldonado_chromium, 2017-05-15 14:25:13]

Patchset #3 (id:70001) has been deleted

[ehmaldonado_chromium, 2017-05-15 15:47:10]

How do you say we move forward with this?

[nednguyen, 2017-05-15 16:18:38]

On 2017/05/15 15:47:10, ehmaldonado_chromium wrote:
> How do you say we move forward with this?

Just land this the bot to see which test fails & disable the failing tests later
& try to reenable them.

[ehmaldonado_chromium, 2017-05-15 16:30:41]

The CQ bit was checked by ehmaldonado@chromium.org

[commit-bot: I haz the power, 2017-05-15 16:31:38]

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, 2017-05-15 16:31:39]

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

[commit-bot: I haz the power, 2017-05-15 16:31:40]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[ehmaldonado_chromium, 2017-05-15 16:33:26]

On 2017/05/15 16:31:40, commit-bot: I haz the power wrote:
> No L-G-T-M from a valid reviewer yet.
> CQ run can only be started once the patch has received an L-G-T-M from a full
> committer.
> Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
> full super star committer.
> Committers are members of the group "project-chromium-committers".
> Note that this has nothing to do with OWNERS files.

Right.
Can you LGTM then, please?

[nednguyen, 2017-05-15 16:55:00]

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
File tools/perf/benchmarks/webrtc.py (right):

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
tools/perf/benchmarks/webrtc.py:22: return 'webrtc.perf_benchmark'
Just name this webrtc. The 'perf_benchmark' part doesn't add much, imo

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
tools/perf/benchmarks/webrtc.py:32: # 'toplevel',
Without 'toplevel', cpuTimeMetric doesn't work

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
tools/perf/benchmarks/webrtc.py:42: # 'expectedQueueingTimeMetric',
Add comment linking to bug to enable this

[ehmaldonado_chromium, 2017-05-16 08:36:28]

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
File tools/perf/benchmarks/webrtc.py (right):

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
tools/perf/benchmarks/webrtc.py:22: return 'webrtc.perf_benchmark'
On 2017/05/15 16:55:00, nednguyen wrote:
> Just name this webrtc. The 'perf_benchmark' part doesn't add much, imo

Done.

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
tools/perf/benchmarks/webrtc.py:32: # 'toplevel',
On 2017/05/15 16:55:00, nednguyen wrote:
> Without 'toplevel', cpuTimeMetric doesn't work

Acknowledged.

https://codereview.chromium.org/2790553003/diff/80001/tools/perf/benchmarks/w...
tools/perf/benchmarks/webrtc.py:42: # 'expectedQueueingTimeMetric',
On 2017/05/15 16:55:00, nednguyen wrote:
> Add comment linking to bug to enable this

The bug seems to have been fixed in https://codereview.chromium.org/2871573006
but I haven't verified, so I'd rather delete these and add them later. They're
still part of the roadmap.

[nednguyen, 2017-05-16 12:42:22]

lgtm

[ehmaldonado_chromium, 2017-05-16 12:42:57]

The CQ bit was checked by ehmaldonado@chromium.org

[commit-bot: I haz the power, 2017-05-16 12:43:25]

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, 2017-05-16 12:55:08]

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

[commit-bot: I haz the power, 2017-05-16 12:55:10]

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

[ehmaldonado_chromium, 2017-05-16 13:23:56]

Patchset #5 (id:100001) has been deleted

[ehmaldonado_chromium, 2017-05-16 13:24:12]

The CQ bit was checked by ehmaldonado@chromium.org

[ehmaldonado_chromium, 2017-05-16 13:24:13]

The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2790553003/#ps110001
(title: "Ran tools/perf/generate_perf_data")

[commit-bot: I haz the power, 2017-05-16 13:24:42]

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, 2017-05-16 13:34:19]

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

[commit-bot: I haz the power, 2017-05-16 13:34:20]

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

[ehmaldonado_chromium, 2017-05-16 13:56:10]

On 2017/05/16 13:34:20, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

I keep getting this error saying I should run tools/perf/generate_perf_data.
When I do, nothing changes.
But if I create another branch and patch the CL and run it again, it does
change.
Do you know what might be wrong?

[nednguyen, 2017-05-16 14:01:05]

On 2017/05/16 13:56:10, ehmaldonado_chromium wrote:
> On 2017/05/16 13:34:20, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
> 
> I keep getting this error saying I should run tools/perf/generate_perf_data.
> When I do, nothing changes.
> But if I create another branch and patch the CL and run it again, it does
> change.
> Do you know what might be wrong?

I think this is because rietveld cannot deal with the size of the CL. You will
need to upload the CL again using "--gerrit" flag (I filed
https://bugs.chromium.org/p/chromium/issues/detail?id=720156 last week about
this)