[not for review] Add Draw entries to window Performance Timeline

cc/debug/performance_draw_timing_counter.h

Index: cc/debug/performance_draw_timing_counter.h
diff --git a/cc/debug/performance_draw_timing_counter.h b/cc/debug/performance_draw_timing_counter.h
new file mode 100644
index 0000000000000000000000000000000000000000..766e5cf3414c2fb5739185261d0ce72baf5169d1
--- /dev/null
+++ b/cc/debug/performance_draw_timing_counter.h
@@ -0,0 +1,73 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CC_DEBUG_PERFORMANCE_DRAW_TIMING_COUNTER_H_
+#define CC_DEBUG_PERFORMANCE_DRAW_TIMING_COUNTER_H_
+
+#include <set>
+#include <utility>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/containers/hash_tables.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/time/time.h"
+#include "cc/debug/ring_buffer.h"
+#include "cc/trees/layer_tree_host_common.h"
+
+namespace cc {
+
+// This class maintains a history of timestamps, and provides functionality to
+// intelligently compute average frames per second.

[enne (OOO), 2014/07/22 20:44:25]

Where does it compute average fps?

[Mike B, 2014/07/25 23:09:40]

It does not. That comment was copied over when I cribbed from
frame_rate_counter.h. I'll clean up those vestiges!

On 2014/07/22 20:44:25, enne wrote:

+class PerformanceDrawTimingCounter {

[enne (OOO), 2014/07/22 20:44:25]

bikeshedding: The word "performance" here is a little bit wordy.  You could
probably just call this DrawTimingCounter to go with DrawTimingSet and
DrawTimingInfo.

[Mike B, 2014/07/25 23:09:40]

Done.

+ public:
+  struct DrawEvent {
+    int frame_id_;
+    int64_t rect_id_;
+    base::TimeTicks timestamp_;
+    DrawEvent(int frame_id, int64_t rect_id, base::TimeTicks timestamp) :
+        frame_id_(frame_id), rect_id_(rect_id), timestamp_(timestamp) {}
+    DrawEvent() {}
+    DrawEvent(const DrawEvent& rhs) :
+        frame_id_(rhs.frame_id_),
+        rect_id_(rhs.rect_id_),
+        timestamp_(rhs.timestamp_)
+    {}
+    DrawEvent& operator=(const DrawEvent& rhs) {
+      frame_id_ = rhs.frame_id_;
+      rect_id_ = rhs.rect_id_;
+      timestamp_ = rhs.timestamp_;
+      return *this;
+    }
+  };
+
+  static scoped_ptr<PerformanceDrawTimingCounter> Create();
+
+  scoped_ptr<DrawTimingSet> getDrawTimingCounts();

[enne (OOO), 2014/07/22 20:44:25]

style nit: get_draw_timing_counts

Also, "get" is a bit innocuous of a name for something that seems to do a bunch
of work.  Maybe "calculate" instead of "get"?

You could also skip the allocation and just make this void
calculate_draw_timing_counts(DrawTimingSet* output)

[Mike B, 2014/07/25 23:09:40]

I think you're right and "get" is probably not appropriate. 
That would suggest it should be CalculateDrawTimingCounts though.


On 2014/07/22 20:44:25, enne wrote:

+
+  void clearEvents() { ring_buffer_.Clear(); }

[enne (OOO), 2014/07/22 20:44:25]

style nit: clear_events

[Mike B, 2014/07/25 23:09:40]

Done.

+
+  int current_frame_number() const { return ring_buffer_.CurrentIndex(); }
+  size_t time_stamp_history_size() const { return ring_buffer_.BufferSize(); }
+
+  void SaveTimeStamp(int frame_id, int64_t rect_id, base::TimeTicks timestamp);

[enne (OOO), 2014/07/22 20:44:25]

Is this function used?

[Mike B, 2014/07/25 23:09:40]

Nope. Removed.
On 2014/07/22 20:44:25, enne wrote:

+
+  void SaveTimeStamps(base::TimeTicks timestamp,
+                      const std::vector<std::pair<int, int64_t> > &frame_ids);

[enne (OOO), 2014/07/22 20:44:25]

Did git cl upload complain at you about needing to run git cl format? It should
have fixed this.

[Mike B, 2014/07/25 23:09:40]

git cl upload complained about a fair number of style issues, but didn't catch
this one.

On 2014/07/22 20:44:25, enne wrote:

+
+  typedef RingBuffer<struct DrawEvent, 4096> RingBufferType;

[enne (OOO), 2014/07/22 20:44:25]

What happens when this overflows?

[Mike B, 2014/07/25 23:09:40]

New stuff fails to get appended and drops.
On 2014/07/22 20:44:25, enne wrote:

+  RingBufferType::Iterator begin() const { return ring_buffer_.Begin(); }
+  RingBufferType::Iterator end() const { return ring_buffer_.End(); }
+
+ private:
+  explicit PerformanceDrawTimingCounter();

[enne (OOO), 2014/07/22 20:44:25]

style nit: explicit only on single-arg constructors

[Mike B, 2014/07/25 23:09:40]

Done.

+
+  RingBufferType ring_buffer_;
+
+  DISALLOW_COPY_AND_ASSIGN(PerformanceDrawTimingCounter);
+};
+
+}  // namespace cc
+
+#endif  // CC_DEBUG_PERFORMANCE_DRAW_TIMING_COUNTER_H_