[scheduler] Add RendererScheduler.TaskDurationPerQueueType.

third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc

Index: third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc b/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
index 509b289113297bf853cb7faf50c16645809fbefd..7bff2880fa7417b7b1cb0f34b4a6efbaadc3e927 100644
--- a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
+++ b/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
@@ -8,6 +8,7 @@
 #include "base/debug/stack_trace.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/histogram.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -49,6 +50,9 @@ constexpr base::TimeDelta kThreadLoadTrackerWaitingPeriodBeforeReporting =
 // We do not throttle anything while audio is played and shortly after that.
 constexpr base::TimeDelta kThrottlingDelayAfterAudioIsPlayed =
     base::TimeDelta::FromSeconds(5);
+// Maximal bound on task duration for reporting.
+constexpr base::TimeDelta kMaxTaskDurationForReporting =
+    base::TimeDelta::FromMinutes(1);
 
 void ReportForegroundRendererTaskLoad(base::TimeTicks time, double load) {
   if (!blink::RuntimeEnabledFeatures::timerThrottlingForBackgroundTabsEnabled())
@@ -1778,13 +1782,39 @@ void RendererSchedulerImpl::didProcessTask(TaskQueue* task_queue,
       start_time_ticks, end_time_ticks);
   MainThreadOnly().background_main_thread_load_tracker.RecordTaskTime(
       start_time_ticks, end_time_ticks);
+
   // TODO(altimin): Per-page metrics should also be considered.
-  UMA_HISTOGRAM_CUSTOM_COUNTS(
-      "RendererScheduler.TaskTime",
-      (end_time_ticks - start_time_ticks).InMicroseconds(), 1, 1000000, 50);
+  RecordTaskMetrics(task_queue->GetQueueType(),

[haraken, 2017/03/17 11:59:51]

I'm just curious but wouldn't it be heavy to record histograms at every task?

[altimin, 2017/03/17 12:43:12]

We've started recording task lengths a while ago. This particular histogram is
fine because we just increment a value and report only when value was
sufficiently large (>1ms, but we might want to increase this threshold). 

+                    end_time_ticks - start_time_ticks);
+}
+
+void RendererSchedulerImpl::RecordTaskMetrics(TaskQueue::QueueType queue_type,
+                                              base::TimeDelta duration) {
+  duration = std::min(duration, kMaxTaskDurationForReporting);
+
+  UMA_HISTOGRAM_CUSTOM_COUNTS("RendererScheduler.TaskTime",
+                              duration.InMicroseconds(), 1, 1000 * 1000, 50);
+
   UMA_HISTOGRAM_ENUMERATION("RendererScheduler.NumberOfTasksPerQueueType",

[Sami, 2017/03/17 12:53:38]

Is this metric useful anymore with the new duration histogram?

[altimin, 2017/03/17 13:05:01]

Good question. Let's leave it for now (to see the relationship between the
number of tasks and the total duration) and revisit it when we have the data.

[Sami, 2017/05/16 10:45:07]

Ok, let's add a TODO to look into it though?

[altimin, 2017/05/16 11:56:14]

Done.

-                            static_cast<int>(task_queue->GetQueueType()),
+                            static_cast<int>(queue_type),
                             static_cast<int>(TaskQueue::QueueType::COUNT));
+
+  // Report only whole milliseconds to avoid overflow.
+  base::TimeDelta& total_duration =
+      MainThreadOnly()
+          .task_duration_per_queue_type[static_cast<int>(queue_type)];
+  total_duration += duration;
+  int64_t milliseconds = total_duration.InMilliseconds();
+  if (milliseconds > 0) {
+    total_duration -= base::TimeDelta::FromMilliseconds(milliseconds);
+    base::Histogram::FactoryGet(

[Sami, 2017/03/17 12:53:38]

Could we cache this getter?

[altimin, 2017/03/17 13:05:01]

Let's avoid doing this — all macros do invoke Histogram::FactoryGet and it
results in one hash map lookup.

[Ilya Sherman, 2017/03/17 20:27:36]

Actually, the macros define a static pointer.  That's pretty much the entire
reason that we have macros rather than functions -- so as to cache the pointer.

[altimin, 2017/05/16 11:56:14]

Done.

+        "RendererScheduler.TaskDurationPerQueueType", 1,
+        static_cast<int>(TaskQueue::QueueType::COUNT),
+        static_cast<int>(TaskQueue::QueueType::COUNT) + 1,
+        base::HistogramBase::kUmaTargetedHistogramFlag)
+        ->AddCount(static_cast<int>(queue_type),
+                   static_cast<int>(milliseconds));
+  }
 }
 
 void RendererSchedulerImpl::AddTaskTimeObserver(