Teach mojo_benchmark to produce chart_data for the perf dashboard.

Teach mojo_benchmark to produce chart_data for the perf dashboard.

This patch adds the ability to encode the benchmark results in the 'chart_data'
format understood by Chromium Performance Benchmark.

Fixes domokit/devtools#52.

R=viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/64736fae560422778d97e5e747458e34697cd24e

[ppi, 2015-10-15 21:57:36]

ppi@chromium.org changed reviewers:
+ viettrungluu@chromium.org

[ppi, 2015-10-15 21:57:36]

Hi Trung,

ptal.

[viettrungluu, 2015-10-15 22:32:57]

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/de...
File mojo/devtools/common/devtoolslib/perf_dashboard_unittest.py (right):

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/de...
mojo/devtools/common/devtoolslib/perf_dashboard_unittest.py:36:
self.assertEquals(1, len(result))
This seems like a very long-winded way to test the contents of |result|. Can't
you just compare it with a dictionary?

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
File mojo/devtools/common/mojo_benchmark (right):

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:180: help='file to which chart data for the
performance '
Despite what snobs may think, "file to write chart data for the performance
dashboard to" is both shorter and clearer.

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:228: measurement_name =
measurement_spec.replace('/', '-')
Can't you align the way things are named/specified?

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:229: chart_data_recorder.record_scalar(name,
measurement_name, 'ms',
Are the results always milliseconds?

(Likewise here: It'd be nice if the measurement_results also contained the
units.)

[ppi, 2015-10-15 23:13:14]

Thanks, ptal.

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/de...
File mojo/devtools/common/devtoolslib/perf_dashboard_unittest.py (right):

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/de...
mojo/devtools/common/devtoolslib/perf_dashboard_unittest.py:36:
self.assertEquals(1, len(result))
On 2015/10/15 22:32:56, viettrungluu wrote:
> This seems like a very long-winded way to test the contents of |result|. Can't
> you just compare it with a dictionary?

Done.

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
File mojo/devtools/common/mojo_benchmark (right):

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:180: help='file to which chart data for the
performance '
On 2015/10/15 22:32:56, viettrungluu wrote:
> Despite what snobs may think, "file to write chart data for the performance
> dashboard to" is both shorter and clearer.

Done.

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:228: measurement_name =
measurement_spec.replace('/', '-')
On 2015/10/15 22:32:56, viettrungluu wrote:
> Can't you align the way things are named/specified?

Mixing undescores with dashes in the spec seems not palatable.

I think we will eventually want to have some better format to spec the
measurements - maybe in the function notation, ie
"time_until(category:event_name)",
time_between(category1:event1,category2:event2),
percentile_duration(category:event_name, percentile)", but  this would perhaps
better be addressed separately.

Wdyt?

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:229: chart_data_recorder.record_scalar(name,
measurement_name, 'ms',
On 2015/10/15 22:32:56, viettrungluu wrote:
> Are the results always milliseconds?
> 
> (Likewise here: It'd be nice if the measurement_results also contained the
> units.)

For now, yes. I think we will want to have the benchmark app report results
along with the units and propagate this here. I'd suggest to to this when/if we
grow first measurement type that does not return milliseconds.

[ppi, 2015-10-21 14:29:37]

Hi Trung, can you tal?

[viettrungluu, 2015-10-21 15:45:28]

lgtm

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
File mojo/devtools/common/mojo_benchmark (right):

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:228: measurement_name =
measurement_spec.replace('/', '-')
On 2015/10/15 23:13:14, ppi wrote:
> On 2015/10/15 22:32:56, viettrungluu wrote:
> > Can't you align the way things are named/specified?
> 
> Mixing undescores with dashes in the spec seems not palatable.
> 
> I think we will eventually want to have some better format to spec the
> measurements - maybe in the function notation, ie
> "time_until(category:event_name)",
> time_between(category1:event1,category2:event2),
> percentile_duration(category:event_name, percentile)", but  this would perhaps
> better be addressed separately.
> 
> Wdyt?

OK

https://codereview.chromium.org/1406063002/diff/20001/mojo/devtools/common/mo...
mojo/devtools/common/mojo_benchmark:229: chart_data_recorder.record_scalar(name,
measurement_name, 'ms',
On 2015/10/15 23:13:14, ppi wrote:
> On 2015/10/15 22:32:56, viettrungluu wrote:
> > Are the results always milliseconds?
> > 
> > (Likewise here: It'd be nice if the measurement_results also contained the
> > units.)
> 
> For now, yes. I think we will want to have the benchmark app report results
> along with the units and propagate this here. I'd suggest to to this when/if
we
> grow first measurement type that does not return milliseconds.

Sure.

[ppi, 2015-10-22 09:14:46]

Committed patchset #4 (id:60001) manually as
64736fae560422778d97e5e747458e34697cd24e (presubmit successful).