scheduler: Add support for tracing scheduler state

content/renderer/scheduler/renderer_scheduler_selector.h

Index: content/renderer/scheduler/renderer_scheduler_selector.h
diff --git a/content/renderer/scheduler/renderer_scheduler_selector.h b/content/renderer/scheduler/renderer_scheduler_selector.h
index 7f661308505b283fdae760cfd3dd557b753dc2fd..b768494c844f7056ae234055cfb2db59fcebe04c 100644
--- a/content/renderer/scheduler/renderer_scheduler_selector.h
+++ b/content/renderer/scheduler/renderer_scheduler_selector.h
@@ -7,9 +7,17 @@
 
 #include <set>
 
+#include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
 #include "content/renderer/scheduler/task_queue_selector.h"
 
+namespace base {
+namespace debug {
+class ConvertableToTraceFormat;
+class TracedValue;
+}
+}
+
 namespace content {
 
 // A RendererSchedulerSelector is a TaskQueueSelector which is used by the
@@ -54,6 +62,13 @@ class RendererSchedulerSelector : public TaskQueueSelector {
       const std::vector<const base::TaskQueue*>& work_queues) override;
   bool SelectWorkQueueToService(size_t* out_queue_index) override;
 
+  // Serialize the selector state into the given state dictionary for tracing.
+  void AsValueInto(base::debug::TracedValue* state) const;
+

[picksi1, 2014/10/28 11:01:40]

"AsValueInto" means nothing to me! Is there some idiom I am unaware of?

[Sami, 2014/10/28 12:57:47]

Agreed that it isn't very descriptive, but it's pretty common tracing vernacular
so I'll follow the pattern:
https://code.google.com/p/chromium/codesearch#search/&q=asvalueinto&sq=packag...

+  // Set the name |queue_index| for tracing purposes. |name| must be a pointer
+  // to a static string.
+  void SetQueueName(size_t queue_index, const char* name);
+
  private:
   // Returns true if queueA contains an older task than queueB.
   static bool IsOlder(const base::TaskQueue* queueA,
@@ -72,13 +87,26 @@ class RendererSchedulerSelector : public TaskQueueSelector {
   bool QueueEnabledWithPriority(size_t queue_index,
                                 QueuePriority priority) const;
 
+  void DidSelectQueue(size_t selected_queue, QueuePriority priority);

[rmcilroy, 2014/10/27 17:51:28]

nit - comment when to call this.

[Sami, 2014/10/28 12:57:47]

Done.

+

[picksi1, 2014/10/28 11:01:40]

Possibly the ship has sailed, but it should the selected_queue index be a
typedef (e.g. "QueueIndex") or something other than a size_t?

[Sami, 2014/10/28 12:57:47]

I think we went back and forth with this a couple of times but ended up with
size_t since it is commonly used with indices. Unfortunately a typedef doesn't
really buy you much in terms of type correctness because it will get implicitly
converted to and from a size_t.

(btw, typedefs are now discouraged for new code; "using" is the new hotness.)

+  scoped_refptr<base::debug::ConvertableToTraceFormat> AsValueWithSelectedQueue(
+      size_t selected_queue) const;
+  static const char* PriorityToString(QueuePriority priority);
+
   // Number of high priority tasks which can be run before a normal priority
   // task should be selected to prevent starvation.
   // TODO(rmcilroy): Check if this is a good value.
   static const size_t kMaxStarvationTasks = 5;
 
+  struct Queue {
+    Queue();
+
+    const base::TaskQueue* work_queue;
+    const char* name;
+  };

[picksi1, 2014/10/28 11:01:40]

Does this structure need to hold a work_queue pointer, as this adds an extra
indirection to every queue use? Could we just keep the name here as a parallel
data structure to the original task queue list?

[Sami, 2014/10/28 12:57:47]

Yes, the work queues are not owned by this class so we need to hold a pointer.
It was the same before this patch. There really shouldn't be extra indirection
here -- just the element stride changes. I don't like having two parallel arrays
because you have to make sure their sizes are in sync.

Anyway, this is all academic because this struct is no more :)

+
   base::ThreadChecker main_thread_checker_;
-  std::vector<const base::TaskQueue*> work_queues_;
+  std::vector<Queue> queues_;
   std::set<size_t> queue_priorities_[kQueuePriorityCount];
   size_t starvation_count_;
 };