Avoid stability problems

README.md

@@ -71,7 +71,7 @@ repositories {
 ...
 dependencies {
   ...
-  implementation 'com.github.knokko.update-loop:implementation:v1.0.0'
+  implementation 'com.github.knokko.update-loop:implementation:v1.0.1'
 }
 ```
 
@@ -89,6 +89,6 @@ dependencies {
 <dependency>
   <groupId>com.github.knokko.update-loop</groupId>
   <artifactId>implementation</artifactId>
-  <version>v1.0.0</version>
+  <version>v1.0.1</version>
 </dependency>
 ```

implementation/src/main/java/com/github/knokko/update/UpdateLoop.java

@@ -2,21 +2,30 @@
 
 import java.util.function.Consumer;
 
+import static java.lang.Long.min;
 import static java.lang.Math.max;
 
 /**
  * An `UpdateLoop` tries to ensure that a given (update) function is executed periodically with a given period.
  */
 public class UpdateLoop implements Runnable {
 
-    static long determineSleepTime(SlidingWindow window, long currentTime, long period) {
+    static long determineSleepTime(SlidingWindow window, long currentTime, long period, long lastUpdateTime) {
         SlidingEntry oldest = window.oldest();
         if (oldest == null) return 0L;
 
-        long updateAt = oldest.value + (oldest.age + 1) * period;
-        long sleepNanoTime = updateAt - currentTime;
+        long nextUpdateAt = oldest.value + (oldest.age + 1) * period;
+        long nextSleepTime = nextUpdateAt - currentTime;
 
-        return sleepNanoTime / 1_000_000;
+        if (period >= 1_000_000L) {
+            int futureDistance = window.values.length / 2;
+            long futureUpdateAt = oldest.value + (oldest.age + futureDistance) * period;
+            long futureNanoTime = futureUpdateAt - currentTime;
+            futureNanoTime -= (futureDistance - 1) * lastUpdateTime;
+            nextSleepTime = min(nextSleepTime, futureNanoTime / futureDistance);
+        }
+
+        return nextSleepTime / 1000_000;
     }
 
     private final Consumer<UpdateLoop> updateFunction;
@@ -44,12 +53,12 @@ public UpdateLoop(Consumer<UpdateLoop> updateFunction, long initialPeriod, int w
 
     /**
      * Constructs an <i>UpdateLoop</i> that attempts to execute {@code updateFunction} every {@code initialPeriod}
-     * nanoseconds. It will use a default sliding window of 1 second.
+     * nanoseconds. It will use a default sliding window of 300 milliseconds.
      * @param updateFunction The function that should be called periodically
      * @param initialPeriod The initial period of the update function, in nanoseconds
      */
     public UpdateLoop(Consumer<UpdateLoop> updateFunction, long initialPeriod) {
-        this(updateFunction, initialPeriod, max(4, (int) (1_000_000_000L / initialPeriod)));
+        this(updateFunction, initialPeriod, max(4, (int) (300_000_000L / initialPeriod)));
     }
 
     /**
@@ -97,11 +106,12 @@ public void run() {
         if (didStart) throw new IllegalStateException("This update loop has already started");
         didStart = true;
 
+        long lastUpdateTime = period;
         outerLoop:
         while (shouldContinue) {
             long sleepTime;
             do {
-                sleepTime = determineSleepTime(slidingWindow, System.nanoTime(), period);
+                sleepTime = determineSleepTime(slidingWindow, System.nanoTime(), period, lastUpdateTime);
                 if (sleepTime > 0L) {
                     try {
                         //noinspection BusyWait
@@ -113,7 +123,11 @@ public void run() {
             } while (sleepTime > 0L);
 
             slidingWindow.insert(System.nanoTime());
-            if (shouldContinue) updateFunction.accept(this);
+            if (shouldContinue) {
+                long startUpdateTime = System.nanoTime();
+                updateFunction.accept(this);
+                lastUpdateTime = System.nanoTime() - startUpdateTime;
+            }
         }
     }
 }

implementation/src/test/java/com/github/knokko/update/TestUpdateLoop.java

@@ -2,54 +2,17 @@
 
 import org.junit.jupiter.api.Test;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static com.github.knokko.update.UpdateLoop.determineSleepTime;
 import static java.lang.Math.abs;
+import static java.lang.Math.max;
 import static org.junit.jupiter.api.Assertions.*;
 
 public class TestUpdateLoop {
 
-    @Test
-    public void testDetermineSleepTime() {
-        assertEquals(0L, determineSleepTime(new SlidingWindow(5), 1234, 10));
-
-        long m = 1_000_000L;
-        long period = 10 * m;
-        SlidingWindow window = new SlidingWindow(4);
-
-        // Oldest entry is (0, 2500)
-        window.insert(2500 * m);
-        assertEquals(7, determineSleepTime(window, 2503 * m, period));
-        assertEquals(6, determineSleepTime(window, 1 + 2503 * m, period));
-
-        // Oldest entry is (1, 2500)
-        window.insert(2510 * m);
-        assertEquals(17, determineSleepTime(window, 2503 * m, period));
-        assertEquals(7, determineSleepTime(window, 2513 * m, period));
-        assertTrue(determineSleepTime(window, 2523 * m, period) <= 0);
-
-        // Oldest entry is (2, 2500)
-        window.insert(2600 * m);
-        assertEquals(7, determineSleepTime(window, 2523 * m, period));
-        assertTrue(determineSleepTime(window, 2550 * m, period) <= 0);
-
-        // Oldest entry is (3, 2500)
-        window.insert(2601 * m);
-        assertEquals(37, determineSleepTime(window, 2503 * m, period));
-        assertTrue(determineSleepTime(window, 2540 * m, period) <= 0);
-
-        // Oldest entry becomes 2510
-        window.insert(2602 * m);
-        assertEquals(4L, determineSleepTime(window, 2546 * m, period));
-        assertTrue(determineSleepTime(window, 1 + 2549 * m, period) <= 0);
-        assertTrue(determineSleepTime(window, 2615 * m, period) <= 0);
-
-        // Oldest entry becomes 2600
-        window.insert(2610 * m);
-        assertEquals(8L, determineSleepTime(window, 2632 * m, period));
-    }
-
     private void testConstantPeriod(long period, long sleepTime) throws InterruptedException {
         AtomicInteger counter = new AtomicInteger(0);
 
@@ -115,4 +78,43 @@ public void testCannotStartTwice() throws InterruptedException {
 
         assertThrows(IllegalStateException.class, updateLoop::run);
     }
+
+    @Test
+    public void testInitialDisturbance() {
+        class State {
+            final SlidingWindow window = new SlidingWindow(20);
+            final long period = 20_000_000;
+            long currentTime = 1234;
+
+            final List<Long> sleepTimes = new ArrayList<>();
+
+            void addUpdateTime(long millis) {
+                currentTime += millis * 1000_000L;
+                window.insert(currentTime);
+                long sleepTime = max(0, determineSleepTime(window, currentTime, period, 1000_000L * millis));
+                sleepTimes.add(sleepTime);
+                currentTime += sleepTime * 1000_000L;
+            }
+        }
+
+        State state = new State();
+
+        for (int counter = 0; counter < 70; counter++) {
+            state.addUpdateTime(40);
+        }
+
+        for (int counter = 0; counter < 2000; counter++) {
+            state.addUpdateTime(8);
+        }
+
+        // The first 30 updates took too long, so there is no time to sleep
+        for (int index = 0; index < 35; index++) assertEquals(0, state.sleepTimes.get(index));
+
+        // The system should be stable before the 200th update
+        for (int index = 200; index < 2000; index++) {
+            // Since each update takes 8 of the 20 milliseconds, the sleep times should be between 11 and 13
+            long sleepTime = state.sleepTimes.get(index);
+            if (sleepTime < 10 || sleepTime > 14) assertEquals(12, sleepTime);
+        }
+    }
 }

update-loops-overview.md

@@ -138,13 +138,45 @@ because the number of updates per second is always smaller
 than desired. When the execution time of the update
 function suddenly drops, the system will try to compensate
 for the *missed updates during the last second* rather
-than all missed updates during the peak hours. Thus the
+than all missed updates during the peak hours. Thus, the
 system will continue normal operation after at most 1
 second.
 
+The drawback of the sliding window approach is that the
+system may become unstable. The system ensures
+that enough updates happen during each sliding window,
+but it does **not** necessarily spread the updates
+equally **within** a sliding window:
+1. The system is working normally
+2. Peak hour starts, and updating takes longer, causing
+updates to be missed
+3. Peak hour ends, and the update function is called
+more often to catch up with the recent missed updates.
+Let's call this **rapid mode**.
+4. When the last peak update leaves the sliding window,
+the sliding window contains the **rapid mode** updates,
+but forgot about peak hour. Thus, it sees that it updated
+too often during the past sliding period. To compensate,
+the system will sleep more. Let's call this
+**cooldown mode**.
+5. When the **rapid mode** updates leave the
+sliding window, the system sees the **cooldown mode**
+updates, but not the **rapid mode** updates. Thus, it sees
+that it didn't update enough during the past sliding
+period. To compensate, it will start a new 
+**rapid mode**...
+6. Step 4 and 5 will keep alternating. Even though the
+update function will be called the right number of times
+**on average**, the system will have a periodic
+rapid/cooldown cycle that goes on forever. In games, this
+could look like time speeding up and slowing down
+periodically, which is a bad experience.
+
 ## The `UpdateLoop` class
 The `UpdateLoop` class of this library uses a sliding
 window to track the number of updates that happened
-during the past N updates. Both the update period and the
+during the past N updates. It also takes countermeasures
+to avoid the rapid/cooldown cycle illustrated above.
+Both the update period and the
 length of the sliding window are configurable. Together,
 they determine how much time it 'looks back'