cc: Remove main thread rendering stats

cc: Remove main thread rendering stats

Removed the main thread rendering stats + plumbing and merged the
impl-side stats into RenderingStats.

BUG=

Committed: https://crrev.com/3f7c234021771a4bca46be9ce72911e070ad8c06
Cr-Commit-Position: refs/heads/master@{#303349}

[hendrikw, 2014-11-06 22:28:39]

hendrikw@chromium.org changed reviewers:
+ ernstm@chromium.org, vmpstr@chromium.org

[hendrikw, 2014-11-06 22:28:40]

Removed the main thread stats, and merge the impl side stats into
Rendering stats.

[hendrikw, 2014-11-06 22:33:22]

On 2014/11/06 22:28:40, hendrikw wrote:
> Removed the main thread stats, and merge the impl side stats into
> Rendering stats.

Btw, I merged the impl-side stats into rendering stats, but the event name is
still "BenchmarkInstrumentation::ImplThreadRenderingStats", I'm not sure how
this should work, but I think I could address this in a follow-up.

There are also a couple of html and js files that refer to
"BenchmarkInstrumentation::MainThreadRenderingStats", I'll remove that in a
follow-up.

[ernstm, 2014-11-07 06:42:05]

Nice clean up. LGTM % nit.

It's fine to do the renaming of the event in a follow up patch. It should be
called BenchmarkInstrumentation::RenderingStats. This will require handling both
the old and the new event name on the telemetry side, because we run an older
Chrome version as the reference build with ToT telemetry. I'm not familiar with
the js files you mentioned. Nat would probably know how to migrate those.

https://codereview.chromium.org/707963003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/web_perf/metrics/rendering_stats_unittest.py
(right):

https://codereview.chromium.org/707963003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/web_perf/metrics/rendering_stats_unittest.py:39: def
Advance(self, low=0, high=1):
Please also set the default value for 'low' to 0.1 (same as below).

[hendrikw, 2014-11-07 15:34:21]

Cool, I've updated the default low value.

As for the implthread name, maybe we should just keep the name as is then, there
doesn't seem to be much harm in that?

[ernstm, 2014-11-07 17:53:12]

On 2014/11/07 15:34:21, hendrikw wrote:
> Cool, I've updated the default low value.
> 
> As for the implthread name, maybe we should just keep the name as is then,
there
> doesn't seem to be much harm in that?

The name ImplThreadRenderingStats is a bit clunky and it's not consistent with
the class name, that's why I would like to rename it. But it's not urgent. You
can open a low-priority bug for that and assign it to me.

The update of the js files should still happen (removing references to
MainThreadRenderingStats).

[vmpstr, 2014-11-07 19:57:32]

https://codereview.chromium.org/707963003/diff/40001/cc/debug/rendering_stats.h
File cc/debug/rendering_stats.h (right):

https://codereview.chromium.org/707963003/diff/40001/cc/debug/rendering_stats...
cc/debug/rendering_stats.h:40: int64 frame_count = 0;
Is this allowed?

[hendrikw, 2014-11-07 20:33:28]

No, it's not allowed yet :(

Done PTAL

[vmpstr, 2014-11-07 22:41:19]

lgtm, assuming all of the relevant telemetry tests work as before

https://codereview.chromium.org/707963003/diff/60001/cc/debug/rendering_stats.h
File cc/debug/rendering_stats.h (right):

https://codereview.chromium.org/707963003/diff/60001/cc/debug/rendering_stats...
cc/debug/rendering_stats.h:31: std::vector<base::TimeDelta> values;
Consider making this type of a change a separate patch (I think this is fine,
but just for future reference)

https://codereview.chromium.org/707963003/diff/60001/cc/layers/picture_layer.cc
File cc/layers/picture_layer.cc (right):

https://codereview.chromium.org/707963003/diff/60001/cc/layers/picture_layer....
cc/layers/picture_layer.cc:128: client_, &pile_invalidation_,
SafeOpaqueBackgroundColor(),
:\

https://codereview.chromium.org/707963003/diff/60001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/web_perf/metrics/rendering_stats_unittest.py
(right):

https://codereview.chromium.org/707963003/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/web_perf/metrics/rendering_stats_unittest.py:78:
timestamp = mock_timer.AdvanceAndGet()
This also seems like a somewhat unrelated change?

[hendrikw, 2014-11-07 22:54:26]

The CQ bit was checked by hendrikw@chromium.org

[commit-bot: I haz the power, 2014-11-07 22:55:54]

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

[commit-bot: I haz the power, 2014-11-07 23:04:14]

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

[commit-bot: I haz the power, 2014-11-07 23:04:15]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hendrikw, 2014-11-08 00:26:02]

The CQ bit was checked by hendrikw@chromium.org

[commit-bot: I haz the power, 2014-11-08 00:28:50]

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

[commit-bot: I haz the power, 2014-11-08 01:59:36]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-11-08 02:00:15]

Patchset 5 (id:??) landed as
https://crrev.com/3f7c234021771a4bca46be9ce72911e070ad8c06
Cr-Commit-Position: refs/heads/master@{#303349}