telemetry: Add new metrics to smoothness benchmark.

Added discrepancy and inverse RMS frame time to smoothness benchmark.

Discrepancy is a measure for the largest area of jank in a series of time stamps.

Inverse RMS frame time is a generalization of frames/second that penalizes longer frame times. We can switch to the inverse of another generalized mean with a higher exponent if we want to penalize long frame times even more.

R=nduca@chromium.org, tonyg@chromium.org, klobag@chromium.org, vangelis@chromium.org
BUG=280627, 281544
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221782

[nduca, 2013-09-05 17:57:41]

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
File tools/perf/metrics/smoothness.py (right):

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
tools/perf/metrics/smoothness.py:126: def CalcResults(benchmark_stats, results):
you're saying screen frame throughout here but as far as i can tell this is impl
thread frames no? how do you reconcile these names with reality as we go toward
ubercomp etc?

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
tools/perf/metrics/smoothness.py:129: frame_times = []
screen_frame_xxx here and elsewhere

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
tools/perf/metrics/smoothness.py:140: results.Add('relative_frame_discrepancy',
'',
people are going to be very confused about the difference between absolute and
relative. can we pick one and stick with it?

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
tools/perf/metrics/smoothness.py:144: round(DivideIfPossibleOrZero(1000.0,
this is the key metric, no? can we make this less about the way we compute it
and more about it being the frame time people should pay attention to?
frame_time perhaps.

And, can we deprecate mean_frame_time?

[ernstm, 2013-09-05 18:38:07]

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
File tools/perf/metrics/smoothness.py (right):

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
tools/perf/metrics/smoothness.py:126: def CalcResults(benchmark_stats, results):
On 2013/09/05 17:57:41, nduca wrote:
> you're saying screen frame throughout here but as far as i can tell this is
impl
> thread frames no? how do you reconcile these names with reality as we go
toward
> ubercomp etc?

The name originates from the numFramesSentToScreen field in the stats returned
by GpuBenchmarkingExtension, used in the old smoothness implementation. It can
represent impl thread frames or frames submitted in
RenderWidget::DoDeferredUpdated in legacy software mode. Maybe we should just
drop the 'screen' and put some documentation into the code that explains what
exactly the time stamps, etc. represent?

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
tools/perf/metrics/smoothness.py:140: results.Add('relative_frame_discrepancy',
'',
On 2013/09/05 17:57:41, nduca wrote:
> people are going to be very confused about the difference between absolute and
> relative. can we pick one and stick with it?

I've put in both, so that we can evaluate which one works best in practice. I
personally believe we should use the absolute discrepancy in combination with
the inverse RMS frame time. Absolute discrepancy quantifies the worst jank
(independent of the  number of good frames) and inverse RMS frame time
characterizes the average experience during the entire scroll. If we just use
the absolute discrepancy, we can drop the 'absolute' from the name.

https://codereview.chromium.org/23506030/diff/8001/tools/perf/metrics/smoothn...
tools/perf/metrics/smoothness.py:144: round(DivideIfPossibleOrZero(1000.0,
On 2013/09/05 17:57:41, nduca wrote:
> this is the key metric, no? can we make this less about the way we compute it
> and more about it being the frame time people should pay attention to?
> frame_time perhaps.
> 
> And, can we deprecate mean_frame_time?

The thing about this metric is that the exponent in the generalized mean allows
us to blend between the average frame time and the maximum frame time. The
inverse is similar to frame rate, but it is not the same (except when all frame
times are equal). We could call it generalized_frame_rate, or power_frame_rate
(generalized mean is also called power mean).

[nduca, 2013-09-05 21:43:57]

Great points.

So recapping, can we do

ToplevelFrameTime
ToplevelJank

ImplThreadFrameTime
ImplThreadJank

And then move everything else to

Experimental_SomethingWeMightWantToSomedayMakeOfficial
Experimental_SomethingElseWeFIndInteresting


Also, can you add a median or 75%'ile to the mix so we can see that on the bots.
Again, that could be Experimental_BlahlBlah.

LGTM in case you want to land as a checkpoint on the direction toward this
stuff..

[tonyg, 2013-09-06 02:37:31]

I'll totally defer to Nat on reviewing this, but just wanted to point out that
we currently only have alerts set up on the mean_frame_time trace for the
smoothness measurement. If we want to monitor other metrics, please let me know
and I'll add them.

[ernstm, 2013-09-06 18:16:02]

I removed relative_frame_discrepancy, renamed absolute_frame_discrepancy to
experimental_jank, and renamed inverse_rms_frame_time to
experimental_mean_frame_time. The new names emphasis the kind of metrics instead
of how they are computed. We can remove the 'experimental' tag once we are
confident that these are the right metrics. I also added an
experimental_median_frame_time.

I did not address the impl_thread vs. top level issue in this patch. Will put up
a separate patch for that and also fix the somewhat misleading 'screen_frame_*'
naming.

[commit-bot: I haz the power, 2013-09-06 18:21:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ernstm@chromium.org/23506030/3001

[commit-bot: I haz the power, 2013-09-06 18:21:41]

Failed to apply patch for tools/perf/measurements/smoothness.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file tools/perf/measurements/smoothness.py
  Hunk #1 FAILED at 35.
  1 out of 1 hunk FAILED -- saving rejects to file
tools/perf/measurements/smoothness.py.rej

Patch:       tools/perf/measurements/smoothness.py
Index: tools/perf/measurements/smoothness.py
diff --git a/tools/perf/measurements/smoothness.py
b/tools/perf/measurements/smoothness.py
index
47a7802b7af51579fa5a3a4b95b17da47f804739..fc5980b8501a1fcb93f12fafe8320e2df709b8e0
100644
--- a/tools/perf/measurements/smoothness.py
+++ b/tools/perf/measurements/smoothness.py
@@ -35,7 +35,7 @@ class Smoothness(page_measurement.PageMeasurement):
     return hasattr(page, 'smoothness')
 
   def WillRunAction(self, page, tab, action):
-    tab.browser.StartTracing('webkit,cc,benchmark', 60)
+    tab.browser.StartTracing('webkit,benchmark', 60)
     if tab.browser.platform.IsRawDisplayFrameRateSupported():
       tab.browser.platform.StartRawDisplayFrameRateMeasurement()
     self._metrics = smoothness.SmoothnessMetrics(tab)

[commit-bot: I haz the power, 2013-09-06 18:33:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ernstm@chromium.org/23506030/10001

[commit-bot: I haz the power, 2013-09-06 21:16:18]

Change committed as 221782