Telemetry: Add mean-frame-time and tasks-per-frame

Telemetry: Add mean-frame-time and tasks-per-frame

Mean frame time is in other benchmarks, but it's a good
sanity check for understanding performance changes. So is
tasks-per-frame on each thread, so that is also added in this
patch.

BUG=None

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

[epenner, 2014-10-10 23:01:28]

epenner@chromium.org changed reviewers:
+ ernstm@chromium.org, tonyg@chromium.org, vmiura@chromium.org

[epenner, 2014-10-10 23:02:17]

Ptal.

... Tests coming shortly.

[vmiura, 2014-10-11 00:19:52]

Did you want to add .gitmodules?

[ernstm, 2014-10-11 00:24:22]

I only see mean frame time in the patch, but not frames per second. Should the
title of the patch be changed?

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
File tools/perf/metrics/timeline.py (right):

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:88: # report different frame-rates by adding
their associated trace here.
Why add this if we don't use it? You could instead use FrameTraceThreadName and
FrameTraceName directly, no? If you want to keep it, I suggest changing variable
names, because having ThreadFrameTraceName, FrameTraceName and
FrameTraceThreadName is very confusing. This one could be called
FrameTraceMapping, becuase it maps FrameTraceThreadName to FrameTraceName.

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:112: def Rate(numerator, denominator):
We have telmetry.util.statistics.DivideIfPossibleOrZero for this.

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:139: # This name sucks but I don't want to change
it and lose continuity
Metrics can be migrated to new names on the dashboard, if that helps.

[epenner, 2014-10-13 22:20:32]

Ptal. Added tests also. This is pretty basic extra data so the tests are just
verifying the data is there and correct.

Regarding .gitmodules, it's not showing locally in git, so I'll dig into that
and get it removed.

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
File tools/perf/metrics/timeline.py (right):

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:88: # report different frame-rates by adding
their associated trace here.
Ahh, yeah my original goal was to remove those other ones and report multiple
frame-rates etc. but I decided not to do that. I've removed this mapping as we
can always add it in the future.

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:112: def Rate(numerator, denominator):
On 2014/10/11 00:24:22, ernstm wrote:
> We have telmetry.util.statistics.DivideIfPossibleOrZero for this.

That doesn't cast to float. If you think I can change that to cast to float I'll
use it, otherwise I prefer keeping this.

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:139: # This name sucks but I don't want to change
it and lose continuity
On 2014/10/11 00:24:22, ernstm wrote:
> Metrics can be migrated to new names on the dashboard, if that helps.

That sounds cool. Got some documentation on that? How about I do it in a
separate patch. It will change many pagesets/benchmarks so I want to do it
carefully.

[epenner, 2014-10-13 22:32:06]

> Regarding .gitmodules, it's not showing locally in git, so I'll dig into that
> and get it removed.

Cherry-picking to a new branch seems to have removed it. Ready to go I think.

[ernstm, 2014-10-14 17:06:12]

lgtm % nits

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
File tools/perf/metrics/timeline.py (right):

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:112: def Rate(numerator, denominator):
On 2014/10/13 22:20:32, epenner wrote:
> On 2014/10/11 00:24:22, ernstm wrote:
> > We have telmetry.util.statistics.DivideIfPossibleOrZero for this.
> 
> That doesn't cast to float. If you think I can change that to cast to float
I'll
> use it, otherwise I prefer keeping this.

Please change DivideIfPossibleOrZero to cast to float. I only round one other
call site and that should definitely cast to float.

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:139: # This name sucks but I don't want to change
it and lose continuity
On 2014/10/13 22:20:32, epenner wrote:
> On 2014/10/11 00:24:22, ernstm wrote:
> > Metrics can be migrated to new names on the dashboard, if that helps.
> 
> That sounds cool. Got some documentation on that? How about I do it in a
> separate patch. It will change many pagesets/benchmarks so I want to do it
> carefully.

qyearsley can help with the data migration. It is a good idea to do it as a
separate patch.

https://codereview.chromium.org/643343002/diff/180001/tools/perf/metrics/time...
File tools/perf/metrics/timeline.py (right):

https://codereview.chromium.org/643343002/diff/180001/tools/perf/metrics/time...
tools/perf/metrics/timeline.py:139: def
ThreadMeanFrameResultName(thread_category):
I would rename this to ThreadMeanFrameTimeResultName.

https://codereview.chromium.org/643343002/diff/180001/tools/perf/metrics/time...
tools/perf/metrics/timeline.py:209: mean_frame = Rate(self.all_action_time,
num_frames)
mean_frame -> mean_frame_time

[epenner, 2014-10-18 01:02:30]

Addressed feedback.

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
File tools/perf/metrics/timeline.py (right):

https://codereview.chromium.org/643343002/diff/50001/tools/perf/metrics/timel...
tools/perf/metrics/timeline.py:139: # This name sucks but I don't want to change
it and lose continuity
On 2014/10/14 17:06:11, ernstm wrote:
> On 2014/10/13 22:20:32, epenner wrote:
> > On 2014/10/11 00:24:22, ernstm wrote:
> > > Metrics can be migrated to new names on the dashboard, if that helps.
> > 
> > That sounds cool. Got some documentation on that? How about I do it in a
> > separate patch. It will change many pagesets/benchmarks so I want to do it
> > carefully.
> 
> qyearsley can help with the data migration. It is a good idea to do it as a
> separate patch.

Acknowledged.

https://codereview.chromium.org/643343002/diff/180001/tools/perf/metrics/time...
File tools/perf/metrics/timeline.py (right):

https://codereview.chromium.org/643343002/diff/180001/tools/perf/metrics/time...
tools/perf/metrics/timeline.py:139: def
ThreadMeanFrameResultName(thread_category):
On 2014/10/14 17:06:11, ernstm wrote:
> I would rename this to ThreadMeanFrameTimeResultName. 

Done.

https://codereview.chromium.org/643343002/diff/180001/tools/perf/metrics/time...
tools/perf/metrics/timeline.py:209: mean_frame = Rate(self.all_action_time,
num_frames)
On 2014/10/14 17:06:11, ernstm wrote:
> mean_frame -> mean_frame_time

Done.

[epennerAtGoogle, 2014-10-20 21:16:43]

The CQ bit was checked by epenner@google.com

[commit-bot: I haz the power, 2014-10-20 21:18:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/643343002/250001

[commit-bot: I haz the power, 2014-10-20 23:09:01]

Committed patchset #9 (id:250001)

[commit-bot: I haz the power, 2014-10-20 23:09:52]

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