[scheduler] Change ThreadLoadTracker to use only recent data.

third_party/WebKit/Source/platform/scheduler/base/thread_load_tracker.cc

 // Copyright 2016 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.
 
 #include "platform/scheduler/base/thread_load_tracker.h"
 
 #include <algorithm>
 
 namespace blink {
 namespace scheduler {
 
 namespace {
 
 const int kLoadReportingIntervalInSeconds = 1;
 const int kWaitingPeriodBeforeReportingInSeconds = 10;

[Sami, 2016/10/05 15:28:12]

The description talks about a one minute interval but this is still 10s?

[altimin, 2016/10/05 16:26:56]

Description is wrong, fixed.

 
 }  // namespace
 
 ThreadLoadTracker::ThreadLoadTracker(base::TimeTicks now,
                                      const Callback& callback)
     : time_(now),
-      next_reporting_time_(now),
       thread_state_(ThreadState::ACTIVE),
       last_state_change_time_(now),
       waiting_period_(
           base::TimeDelta::FromSeconds(kWaitingPeriodBeforeReportingInSeconds)),
       reporting_interval_(
           base::TimeDelta::FromSeconds(kLoadReportingIntervalInSeconds)),
-      callback_(callback) {}
+      callback_(callback) {
+  next_reporting_time_ = now + waiting_period_;
+}
 
 ThreadLoadTracker::~ThreadLoadTracker() {}
 
 void ThreadLoadTracker::Pause(base::TimeTicks now) {
   Advance(now, TaskState::IDLE);
   thread_state_ = ThreadState::PAUSED;
   last_state_change_time_ = now;
 }
 
 void ThreadLoadTracker::Resume(base::TimeTicks now) {
   Advance(now, TaskState::IDLE);
   thread_state_ = ThreadState::ACTIVE;
   last_state_change_time_ = now;
+
+  next_reporting_time_ = now + reporting_interval_;
+  run_time_inside_window_ = base::TimeDelta();
 }
 
 void ThreadLoadTracker::RecordTaskTime(base::TimeTicks start_time,
                                        base::TimeTicks end_time) {
   start_time = std::max(last_state_change_time_, start_time);
   end_time = std::max(last_state_change_time_, end_time);
 
   Advance(start_time, TaskState::IDLE);
+

[Sami, 2016/10/05 15:28:12]

Meant to add this blank line?

[altimin, 2016/10/05 16:26:55]

Done.

   Advance(end_time, TaskState::TASK_RUNNING);
 }
 
 void ThreadLoadTracker::RecordIdle(base::TimeTicks now) {
   Advance(now, TaskState::IDLE);
 }
 
+namespace {
+
+// Calculates length of intersection of two time intervals.
+base::TimeDelta Intersection(base::TimeTicks left1,
+                             base::TimeTicks right1,
+                             base::TimeTicks left2,
+                             base::TimeTicks right2) {
+  base::TimeTicks left = std::max(left1, left2);

[Sami, 2016/10/05 15:28:12]

DCHECK_LT(left1, right1);
DCHECK_LT(left2, right2); ?

[altimin, 2016/10/05 16:26:55]

Done.

+  base::TimeTicks right = std::min(right1, right2);
+
+  if (left <= right)
+    return right - left;
+
+  return base::TimeDelta();
+}
+}

[Sami, 2016/10/05 15:28:12]

}  // namespace

[altimin, 2016/10/05 16:26:55]

Done.

+
 void ThreadLoadTracker::Advance(base::TimeTicks now, TaskState task_state) {
   // This function advances |time_| to now and calls |callback_|
   // when appropriate.
   if (time_ > now) {

[Sami, 2016/10/05 15:28:12]

Random question: why would the time ever go backwards?

[altimin, 2016/10/05 16:26:55]

I was afraid of renderer going offscreen / onscreen in the middle of a task, but
now realized that it should not be possible.

     return;
   }
 
   if (thread_state_ == ThreadState::PAUSED) {
     // If the load tracker is paused, bail out early.
     time_ = now;
-    next_reporting_time_ = now + reporting_interval_;
     return;
   }
 
   while (time_ < now) {
     // Forward time_ to the earliest of following:

[alex clarke (OOO till 29th), 2016/10/05 15:25:22]

nit pick: s/Forward/Advance (here and below)

[altimin, 2016/10/05 16:26:55]

Done.

     // a) time to call |callback_|
     // b) requested time to forward (|now|).
     base::TimeTicks next_current_time = std::min(next_reporting_time_, now);
 
     base::TimeDelta delta = next_current_time - time_;
 
-    // Forward time and recalculate |total_time_| and |total_runtime_|.
+    // Forward time and recalculate |recorded_time_| and

[alex clarke (OOO till 29th), 2016/10/05 15:25:22]

maybe: Keep a running total of the time spent running tasks within the window
and the total time.

[altimin, 2016/10/05 16:26:55]

Done.

+    // |run_time_inside_window_|.
     if (thread_state_ == ThreadState::ACTIVE) {

[alex clarke (OOO till 29th), 2016/10/05 15:25:23]

Currently thread_state_ is always ThreadState::ACTIVE if we got here.

I suppose callback_.Run might change it, although that would be weird.  How
about removing this if and on line 116 and put DCHECK(thread_state_ ==
ThreadState::ACTIVE); after the callback_.Run()?

This would make the code a bit simpler.

[altimin, 2016/10/05 16:26:55]

Very well noticed, thanks!

-      total_time_ += delta;
+      recorded_time_ += delta;
       if (task_state == TaskState::TASK_RUNNING) {
-        total_runtime_ += delta;
+        run_time_inside_window_ +=
+            Intersection(next_reporting_time_ - reporting_interval_,
+                         next_reporting_time_, time_, time_ + delta);

[Sami, 2016/10/05 15:28:12]

time_ + delta will never be later than next_reporting_time_ so it seems we only
need the left side intersection here, right?

[altimin, 2016/10/05 16:26:55]

Yes, but I believe that a simple intersection is easier to read.

       }
     }
+
     time_ = next_current_time;
 
     if (time_ == next_reporting_time_) {
       // Call |callback_| if need and update next callback time.
       if (thread_state_ == ThreadState::ACTIVE &&
-          total_time_ >= waiting_period_) {
+          recorded_time_ >= waiting_period_) {
         callback_.Run(time_, Load());
       }
       next_reporting_time_ += reporting_interval_;
+      run_time_inside_window_ = base::TimeDelta();
     }
   }
 }
 
 double ThreadLoadTracker::Load() {
-  if (total_time_.is_zero()) {
-    return 0;
-  }
-  return total_runtime_.InSecondsF() / total_time_.InSecondsF();
+  return run_time_inside_window_.InSecondsF() /
+         reporting_interval_.InSecondsF();
 }
 
 }  // namespace scheduler
 }  // namespace blink