[scheduler] Discard anomalously long tasks and bump metrics version.

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 b3812dc5780262f6b79143108f71885aee99d4b6..a4119f0ec830a77ce9e80d083928015d5b43209f 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
@@ -53,9 +53,11 @@ constexpr base::TimeDelta kThrottlingDelayAfterAudioIsPlayed =
     base::TimeDelta::FromSeconds(5);
 constexpr base::TimeDelta kQueueingTimeWindowDuration =
     base::TimeDelta::FromSeconds(1);
-// Maximal bound on task duration for reporting.
-constexpr base::TimeDelta kMaxTaskDurationForReporting =
-    base::TimeDelta::FromMinutes(1);
+// Threshold for discarding ultra-long tasks. It is assumed that ultra-long
+// tasks are reporting glitches (e.g. system falling asleep in the middle
+// of the task).

[Ilya Sherman, 2017/05/19 21:15:11]

Hmm, why are you making this assumption?  Could you specifically subtract out
sleep time, rather than just guessing?  I'm concerned that we might be blinding
ourselves to real egregious tasks by simply ignoring them.

[altimin, 2017/05/19 22:34:20]

The problem is that we can't reliably detect when system is sleeping (plus it's
very platform-specific).
We are already doing something like this for other metrics. 

It is a good idea to add one more histogram to track the number (and duration)
of discarded tasks? This way we can monitor and act if something goes wrong.
(The assumption is here that we have a very small number of bad tasks).

[Sami, 2017/05/22 13:57:39]

FWIW the inactive renderer dialog should already be triggering for very long
tasks like these and we get crash reports when that happens (if the user agrees
to send one), so I think we should be already covered here.

[Ilya Sherman, 2017/05/22 20:43:43]

Having some visibility into what data is being missed by this metric does seem
appropriate.  If there's already coverage for this, then LGTM; otherwise, yes,
please do add an additional histogram.

+constexpr base::TimeDelta kLongTaskDiscardingThreshold =
+    base::TimeDelta::FromSeconds(30);
 
 void ReportForegroundRendererTaskLoad(base::TimeTicks time, double load) {
   if (!blink::RuntimeEnabledFeatures::timerThrottlingForBackgroundTabsEnabled())
@@ -64,8 +66,8 @@ void ReportForegroundRendererTaskLoad(base::TimeTicks time, double load) {
   int load_percentage = static_cast<int>(load * 100);
   DCHECK_LE(load_percentage, 100);
 
-  UMA_HISTOGRAM_PERCENTAGE("RendererScheduler.ForegroundRendererMainThreadLoad",
-                           load_percentage);
+  UMA_HISTOGRAM_PERCENTAGE(
+      "RendererScheduler.ForegroundRendererMainThreadLoad2", load_percentage);
   TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"),
                  "RendererScheduler.ForegroundRendererLoad", load_percentage);
 }
@@ -77,8 +79,8 @@ void ReportBackgroundRendererTaskLoad(base::TimeTicks time, double load) {
   int load_percentage = static_cast<int>(load * 100);
   DCHECK_LE(load_percentage, 100);
 
-  UMA_HISTOGRAM_PERCENTAGE("RendererScheduler.BackgroundRendererMainThreadLoad",
-                           load_percentage);
+  UMA_HISTOGRAM_PERCENTAGE(
+      "RendererScheduler.BackgroundRendererMainThreadLoad2", load_percentage);
   TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"),
                  "RendererScheduler.BackgroundRendererLoad", load_percentage);
 }
@@ -220,7 +222,7 @@ RendererSchedulerImpl::MainThreadOnly::MainThreadOnly(
       rail_mode_observer(nullptr),
       wake_up_budget_pool(nullptr),
       task_duration_per_queue_type_histogram(base::Histogram::FactoryGet(
-          "RendererScheduler.TaskDurationPerQueueType",
+          "RendererScheduler.TaskDurationPerQueueType2",
           1,
           static_cast<int>(TaskQueue::QueueType::COUNT),
           static_cast<int>(TaskQueue::QueueType::COUNT) + 1,
@@ -1841,26 +1843,31 @@ void RendererSchedulerImpl::DidProcessTask(TaskQueue* task_queue,
   task_queue_throttler()->OnTaskRunTimeReported(task_queue, start_time_ticks,
                                                 end_time_ticks);
 
-  // We want to measure thread time here, but for efficiency reasons
-  // we stick with wall time.
-  GetMainThreadOnly().foreground_main_thread_load_tracker.RecordTaskTime(
-      start_time_ticks, end_time_ticks);
-  GetMainThreadOnly().background_main_thread_load_tracker.RecordTaskTime(
-      start_time_ticks, end_time_ticks);
-
   // TODO(altimin): Per-page metrics should also be considered.
-  RecordTaskMetrics(task_queue->GetQueueType(),
-                    end_time_ticks - start_time_ticks);
+  RecordTaskMetrics(task_queue->GetQueueType(), start_time_ticks,
+                    end_time_ticks);
 }
 
 void RendererSchedulerImpl::RecordTaskMetrics(TaskQueue::QueueType queue_type,
-                                              base::TimeDelta duration) {
-  UMA_HISTOGRAM_CUSTOM_COUNTS("RendererScheduler.TaskTime",
+                                              base::TimeTicks start_time,
+                                              base::TimeTicks end_time) {
+  base::TimeDelta duration = end_time - start_time;
+  if (duration > kLongTaskDiscardingThreshold)
+    return;
+
+  UMA_HISTOGRAM_CUSTOM_COUNTS("RendererScheduler.TaskTime2",
                               duration.InMicroseconds(), 1, 1000 * 1000, 50);
 
+  // We want to measure thread time here, but for efficiency reasons
+  // we stick with wall time.
+  GetMainThreadOnly().foreground_main_thread_load_tracker.RecordTaskTime(
+      start_time, end_time);
+  GetMainThreadOnly().background_main_thread_load_tracker.RecordTaskTime(
+      start_time, end_time);
+
   // TODO(altimin): See whether this metric is still useful after
   // adding RendererScheduler.TaskDurationPerQueueType.
-  UMA_HISTOGRAM_ENUMERATION("RendererScheduler.NumberOfTasksPerQueueType",
+  UMA_HISTOGRAM_ENUMERATION("RendererScheduler.NumberOfTasksPerQueueType2",
                             static_cast<int>(queue_type),
                             static_cast<int>(TaskQueue::QueueType::COUNT));
 
@@ -1870,8 +1877,6 @@ void RendererSchedulerImpl::RecordTaskMetrics(TaskQueue::QueueType queue_type,
 void RendererSchedulerImpl::RecordTaskDurationPerQueueType(
     TaskQueue::QueueType queue_type,
     base::TimeDelta duration) {
-  duration = std::min(duration, kMaxTaskDurationForReporting);
-
   // Report only whole milliseconds to avoid overflow.
   base::TimeDelta& unreported_duration =
       GetMainThreadOnly()