[WIP] [DEFINITELY NOT READY TO LAND] Cpu time metric implementation

This has the full implementation of the metric. The metric will be landed as a series of much smaller CLs - this patch is to sort out high level design issues.

[dproy, 2017-04-07 16:59:00]

Description was changed from

==========
[WIP] [DEFINITELY NOT READY TO LAND] Cpu time metric implementation
==========

to

==========
This has the full implementation of the metric. The metric will be landed as a
series of much smaller CLs - this patch is to sort out high level design issues.
==========

[benjhayden, 2017-04-07 18:05:16]

benjhayden@chromium.org changed reviewers:
+ benjhayden@chromium.org

[benjhayden, 2017-04-07 18:05:17]

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
File tracing/tracing/extras/chrome/cpu_time.html (right):

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time.html:22:
tr.b.math.iterateOverIntersectingIntervals(
High-level design question: associatedEvents?

Way back in early RAIL and perf-insights days, we tried to collect the slices
that were actually associated with the UEs (fka IRs), instead of whatever slices
happened to overlap them.
This kind of fidelity could make the per-UE metrics a lot more stable and
meaningful.

associatedEvents currently sometimes almost work well enough. :-/

The ideal is complete coverage: all slices would be associated with the UE that
caused them.

We started out using flow events to connect input latency async slices with all
of the slices caused by the user gesture.
Flow events can be very verbose and often blew up the trace file.

There are some kinds of UEs that currently never get associatedEvents but could
with a bit of work, like CSS animations.

Loads and Idles currently vacuum up all overlapping slices.
We could probably find a principled solution for Loads using FrameBlamer that
I'll describe more below.

Idles are defined as a timeline subtraction, so there is probably no principled
way to define which slices should be associated with "the user isn't doing
anything", except maybe requestIdleCallback, but some sites use alternative APIs
to schedule "idle" work like timers, but some timers are actually animations, so
we'd have to fix https://github.com/catapult-project/catapult/issues/1146 but
there are other kinds of idle work that might be triggered by crazy non-APIs
like the network...
I could keep going along that line, but there are other lines of thought that
are relevant.

The biggest problem with associatedEvents currently is probably that there are
lots of events that don't get associated with any UEs, so we had an 'other'
category that was terribly suspiciously huge.

So skyostil did FrameBlamer, which does a great job with coverage without
blowing up the trace, but it trades in some of the strong fidelity guarantees
provided by flow events. FrameBlamer annotates slices with the DOM frame or
scheduler that caused them, then we'd do a windowing algorithm like you're doing
here, but at least we could filter out overlapping events that pertain to frames
that the user was not interacting with.

RAIL was deprioritized before I could integrate FrameBlamer into the UserModel,
but it would be a huge improvement.

The practical thing to do with this information is probably to keep it in the
back of your mind until cpuTimeMetric launches as you've currently defined it
without associatedEvents.
IIUC, benchmarking team will help you evaluate the noise and stability of the
metric on the dashboard.
If you find that the per-UE measurements are too noisy, and you dig in to
compare traces and find that large tasks are moving in and out of UEs, before
you throw out UEs, please just come to me and I'll help you evaluate your
options for filtering associatedEvents. The situation is not as gloomy as I made
it seem, I'm sure we can find ways to use some of these ideas to improve
fidelity and stability in targeted ways without a moonshot.

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time.html:100: if (initiatorType) {
I think all UEs should have initiatorType, right?

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time.html:134: segmentToBounds.set(segment,
Drop empty intersections?

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
File tracing/tracing/extras/chrome/cpu_time_test_utils.html (right):

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time_test_utils.html:152: 
no blank lines at starts or ends of blocks

[dproy, 2017-04-07 18:35:10]

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
File tracing/tracing/extras/chrome/cpu_time.html (right):

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time.html:22:
tr.b.math.iterateOverIntersectingIntervals(
On 2017/04/07 18:05:17, benjhayden wrote:

> If you find that the per-UE measurements are too noisy, and you dig in to
> compare traces and find that large tasks are moving in and out of UEs, before
> you throw out UEs, please just come to me and I'll help you evaluate your
> options for filtering associatedEvents. The situation is not as gloomy as I
made
> it seem, I'm sure we can find ways to use some of these ideas to improve
> fidelity and stability in targeted ways without a moonshot.

Thank you so much for providing the context here! I will definitely keep these
ideas in mind as I evaluate the metric more.

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time.html:100: if (initiatorType) {
On 2017/04/07 18:05:17, benjhayden wrote:
> I think all UEs should have initiatorType, right?

Idle Expectation doesn't.

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time.html:134: segmentToBounds.set(segment,
On 2017/04/07 18:05:17, benjhayden wrote:
> Drop empty intersections?

Yes - I missed that. Will update.

[dproy, 2017-04-07 18:53:49]

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
File tracing/tracing/extras/chrome/cpu_time.html (right):

https://codereview.chromium.org/2804043003/diff/20001/tracing/tracing/extras/...
tracing/tracing/extras/chrome/cpu_time.html:134: segmentToBounds.set(segment,
On 2017/04/07 18:35:10, dproy wrote:
> On 2017/04/07 18:05:17, benjhayden wrote:
> > Drop empty intersections?
> 
> Yes - I missed that. Will update. 

Actually I changed my mind. |segments| is already filtered when this function is
called, and (1) I don't want to redo the intersection check, and (2) having an
entry for each segment in |segments| is a logically pleasing property of have
even if theoretically some of the ranges could be empty. I added a comment
making this decision explicit.