[scheduler] Change ThreadLoadTracker to use only recent data.

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

Index: third_party/WebKit/Source/platform/scheduler/base/thread_load_tracker.cc
diff --git a/third_party/WebKit/Source/platform/scheduler/base/thread_load_tracker.cc b/third_party/WebKit/Source/platform/scheduler/base/thread_load_tracker.cc
index 9e5aaaf57823abc912eb9e0f171e9dfb0974a255..12d5b63630cfff7e281d2d7bbdabdaf498546acb 100644
--- a/third_party/WebKit/Source/platform/scheduler/base/thread_load_tracker.cc
+++ b/third_party/WebKit/Source/platform/scheduler/base/thread_load_tracker.cc
@@ -19,14 +19,15 @@ const int kWaitingPeriodBeforeReportingInSeconds = 10;
 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() {}
 
@@ -40,6 +41,9 @@ 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,
@@ -48,6 +52,7 @@ void ThreadLoadTracker::RecordTaskTime(base::TimeTicks 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);
 }
 
@@ -55,6 +60,23 @@ 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.
@@ -65,7 +87,6 @@ void ThreadLoadTracker::Advance(base::TimeTicks now, TaskState task_state) {
   if (thread_state_ == ThreadState::PAUSED) {
     // If the load tracker is paused, bail out early.
     time_ = now;
-    next_reporting_time_ = now + reporting_interval_;
     return;
   }
 
@@ -77,31 +98,34 @@ void ThreadLoadTracker::Advance(base::TimeTicks now, TaskState task_state) {
 
     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