Add duration estimation data to RenderingStats.

cc/debug/rendering_stats.cc

Index: cc/debug/rendering_stats.cc
diff --git a/cc/debug/rendering_stats.cc b/cc/debug/rendering_stats.cc
index 3e123b667c774656f88f421225e5f1963ad64f69..de1da8573c225ebd4a1db82fcc6c1fae5d710c11 100644
--- a/cc/debug/rendering_stats.cc
+++ b/cc/debug/rendering_stats.cc
@@ -47,6 +47,19 @@ ImplThreadRenderingStats::AsTraceableData() const {
   record_data->SetInteger("visible_content_area", visible_content_area);
   record_data->SetInteger("approximated_visible_content_area",
                           approximated_visible_content_area);
+  record_data->SetDouble("draw_duration", draw_duration.InMillisecondsF());
+  record_data->SetDouble("draw_duration_estimate",
+                         draw_duration_estimate.InMillisecondsF());
+  record_data->SetDouble("begin_main_frame_to_commit_duration",
+                         begin_main_frame_to_commit_duration.InMillisecondsF());
+  record_data->SetDouble(
+      "begin_main_frame_to_commit_duration_estimate",
+      begin_main_frame_to_commit_duration_estimate.InMillisecondsF());
+  record_data->SetDouble("commit_to_activate_duration",
+                         commit_to_activate_duration.InMillisecondsF());
+  record_data->SetDouble(
+      "commit_to_activate_duration_estimate",

[brianderson, 2014/07/02 22:39:28]

Postfix labels with _ms so units are easy to figure out?

[Dominik Grewe, 2014/07/03 13:16:38]

Done.

[ernstm, 2014/07/08 15:01:36]

I'd prefer consistent units for all time values. I know it's complicated
(reference builds on perf bots!), but could we convert all time values in
RenderingStats to milliseconds?

+      commit_to_activate_duration_estimate.InMillisecondsF());
   return TracedValue::FromValue(record_data.release());
 }
 
@@ -57,6 +70,17 @@ void ImplThreadRenderingStats::Add(const ImplThreadRenderingStats& other) {
   rasterized_pixel_count += other.rasterized_pixel_count;
   visible_content_area += other.visible_content_area;
   approximated_visible_content_area += other.approximated_visible_content_area;
+
+  // There should only ever be one sample of these durations per frame.

[Dominik Grewe, 2014/07/02 14:43:22]

Adding up values doesn't really make sense. We could use a list instead. See my
comment in RenderingStatsInstrumentation below.

Afaict, this method is actually never called. So what's the use case for it?

[brianderson, 2014/07/02 22:39:28]

Yeah, it doesn't make sense. Definitely get rid of the logic below. Not a big
deal, but maybe create a separate ImplThreadRenderingStatsAccumulated struct for
purity?
I think it's used at the end of LTHI::DrawLayers with the call to
AccumulateAndClearImplThreadStats.

[ernstm, 2014/07/08 15:01:36]

I think we can get rid of the accumulated stats entirely, because they are now
used only to update the PaintTimeCounter in LayerTreeHostImpl, where they are
converted back to per-frame stats. I believe this patch could be simplified
considerably by first making this change.

[Dominik Grewe, 2014/07/08 15:10:41]

I don't think it would simplify this patch a lot. If I understand correctly, we
still need to use lists of values because we cannot guarantee that we don't call
Add* twice on the same RenderingStats objects (see Brian's comment in
RenderingStatsInstrumentation::AddDrawDuration).

+  draw_duration += other.draw_duration;
+  draw_duration_estimate += other.draw_duration_estimate;
+  begin_main_frame_to_commit_duration +=
+      other.begin_main_frame_to_commit_duration;
+  begin_main_frame_to_commit_duration_estimate +=
+      other.begin_main_frame_to_commit_duration_estimate;
+  commit_to_activate_duration += other.commit_to_activate_duration;
+  commit_to_activate_duration_estimate +=
+      other.commit_to_activate_duration_estimate;
 }
 
 void RenderingStats::Add(const RenderingStats& other) {