Bug 1685937 - Between sampling loops, never sleep less than 1/4 the requested interval - r=canaltinova

More exactly: Instead of trying to compensate for only the previous sleep over/undershoot, we now try to keep each sampling loop to a schedule based on the very beginning of sampling, by adding the requested interval to the scheduled sampling time.

In addition, the sleep time is always kept to a minimum, to avoid making the system busier by having one loop right after the other -- also, this very-close data may be less useful.

And in presumably very busy times, one sleep and the following sampling work may take much more time than the requested sampling interval, trying to keep to schedule is now futile (it would require trying to effectively multiply the sampling rate, which seems unlikely to succeed, and would impact Firefox even more), in which case we revert to the full sampling interval.

Differential Revision: https://phabricator.services.mozilla.com/D102253

tools/profiler/core/platform.cpp

@@ -3348,18 +3348,25 @@ void SamplerThread::Run() {
   // Will be kept between collections, to know what each collection does.
   auto previousState = localBuffer.GetState();
 
-  // This will be positive if we are running behind schedule (sampling less
-  // frequently than desired) and negative if we are ahead of schedule.
-  TimeDuration lastSleepOvershoot = 0;
-  TimeStamp sampleStart = TimeStamp::NowUnfuzzed();
-
   // This will be set inside the loop, from inside the lock scope, to capture
   // all callbacks added before that, but none after the lock is released.
   UniquePtr<PostSamplingCallbackListItem> postSamplingCallbacks;
   // This will be set inside the loop, before invoking callbacks outside.
   SamplingState samplingState{};
 
+  const TimeDuration sampleInterval =
+      TimeDuration::FromMicroseconds(mIntervalMicroseconds);
+  const uint32_t minimumIntervalSleepUs =
+      static_cast<uint32_t>(mIntervalMicroseconds / 4);
+
+  // This is the scheduled time at which each sampling loop should start.
+  // It will determine the ideal next sampling start by adding the expected
+  // interval, unless when sampling runs late -- See end of while() loop.
+  TimeStamp scheduledSampleStart = TimeStamp::NowUnfuzzed();
+
   while (true) {
+    const TimeStamp sampleStart = TimeStamp::NowUnfuzzed();
+
     // This scope is for |lock|. It ends before we sleep below.
     {
       // There should be no local callbacks left from a previous loop.
@@ -3762,21 +3769,32 @@ void SamplerThread::Run() {
 
     ProfilerChild::ProcessPendingUpdate();
 
-    // Calculate how long a sleep to request.  After the sleep, measure how
-    // long we actually slept and take the difference into account when
-    // calculating the sleep interval for the next iteration.  This is an
-    // attempt to keep "to schedule" in the presence of inaccuracy of the
-    // actual sleep intervals.
-    TimeStamp targetSleepEndTime =
-        sampleStart + TimeDuration::FromMicroseconds(mIntervalMicroseconds);
-    TimeStamp beforeSleep = TimeStamp::NowUnfuzzed();
-    TimeDuration targetSleepDuration = targetSleepEndTime - beforeSleep;
-    double sleepTime = std::max(
-        0.0, (targetSleepDuration - lastSleepOvershoot).ToMicroseconds());
-    SleepMicro(static_cast<uint32_t>(sleepTime));
-    sampleStart = TimeStamp::NowUnfuzzed();
-    lastSleepOvershoot =
-        sampleStart - (beforeSleep + TimeDuration::FromMicroseconds(sleepTime));
+    // We expect the next sampling loop to start `sampleInterval` after this
+    // loop here was scheduled to start.
+    scheduledSampleStart += sampleInterval;
+
+    // Try to sleep until we reach that next scheduled time.
+    const TimeStamp beforeSleep = TimeStamp::NowUnfuzzed();
+    if (scheduledSampleStart >= beforeSleep) {
+      // There is still time before the next scheduled sample time.
+      const uint32_t sleepTimeUs = static_cast<uint32_t>(
+          (scheduledSampleStart - beforeSleep).ToMicroseconds());
+      if (sleepTimeUs >= minimumIntervalSleepUs) {
+        SleepMicro(sleepTimeUs);
+      } else {
+        // If we're too close to that time, sleep the minimum amount of time.
+        // Note that the next scheduled start is not shifted, so at the end of
+        // the next loop, sleep may again be adjusted to get closer to schedule.
+        SleepMicro(minimumIntervalSleepUs);
+      }
+    } else {
+      // This sampling loop ended after the next sampling should have started!
+      // There is little point to try and keep up to schedule now, it would
+      // require more work, while it's likely we're late because the system is
+      // already busy. Try and restart a normal schedule from now.
+      scheduledSampleStart = beforeSleep + sampleInterval;
+      SleepMicro(static_cast<uint32_t>(sampleInterval.ToMicroseconds()));
+    }
   }
 
   // End of `while` loop. We can only be here from a `break` inside the loop.