RTT: align base history interval boundaries to fixed steps

Sources/SwiftNetwork/QUIC/RTT.swift

@@ -44,7 +44,8 @@ struct RTT: ~Copyable, PrefixedLoggable {
     // Base RTT history samples
     private var baseRTTs = [NetworkDuration](repeatElement(RTT.invalidRTT, count: RTT.nbaseHistory))
 
-    // Last time when a new minute interval began
+    // Start of the current history interval. Always a whole number of
+    // `historyInterval`s after the first sample.
     private var lastRolloverTimestamp: NetworkClock.Instant? = nil
 
     // Smallest measured base rtt for specified history
@@ -93,16 +94,26 @@ struct RTT: ~Copyable, PrefixedLoggable {
         }
         if now > lastRollover && lastRollover.duration(to: now) > RTT.historyInterval {
             let timeSinceLastRollover = lastRollover.duration(to: now)
-            let rolloverIndex = timeSinceLastRollover / RTT.historyInterval
-            let times = Int(baseRTTIndex) + Int(rolloverIndex)
-
-            // Set the base rtt to invalid for idle periods
-            for i in stride(from: Int(baseRTTIndex + 1), to: Int(times), by: 1) {
+            // Number of whole history intervals elapsed since the start of the
+            // current interval.
+            let rolloverIndex = Int(timeSinceLastRollover / RTT.historyInterval)
+            let times = baseRTTIndex + rolloverIndex
+
+            // Set the base rtt to invalid for idle periods. The slots strictly
+            // between the old and new index had no samples. Once the gap
+            // reaches nbaseHistory every slot (including the old index) is
+            // stale, so cap the work instead of wrapping over the ring again
+            // for each idle minute.
+            let slotsToClear = min(rolloverIndex - 1, RTT.nbaseHistory)
+            for i in stride(from: baseRTTIndex + 1, through: baseRTTIndex + slotsToClear, by: 1) {
                 baseRTTs[i % RTT.nbaseHistory] = RTT.invalidRTT
             }
             baseRTTIndex = times % RTT.nbaseHistory
             baseRTTs[baseRTTIndex] = currentRTT
-            lastRolloverTimestamp = now
+            // Advance by whole intervals so boundaries stay aligned to fixed
+            // steps from the first sample, rather than drifting to whenever a
+            // sample happens to arrive.
+            lastRolloverTimestamp = lastRollover.advanced(by: RTT.historyInterval * rolloverIndex)
 
             baseRTT = baseRTTs[0]
             for i in 0..<RTT.nbaseHistory {

Tests/QUICTests/RTTTests.swift

@@ -404,6 +404,78 @@ final class RTTTests: XCTestCase {
         XCTAssertEqual(rtt.baseRTT, .microseconds(90))
         XCTAssertEqual(rtt.minRTT, .microseconds(40))
     }
+
+    // Verifies that interval boundaries stay aligned to fixed steps from the
+    // first sample when samples are sparse and irregularly spaced. If the
+    // rollover timestamp were moved to the arrival time of each sample, the
+    // fractional part of each gap would be lost, elapsed intervals would be
+    // undercounted, and old minimums would stay in the window for more than
+    // `nbaseHistory` intervals.
+    func testRTTBaseRolloverIrregularSpacing() {
+        // Interval 0 [0s, 60s): 40us.
+        rtt.processNewSample(
+            ackDuration: .microseconds(40),
+            packetAckedTime: NetworkClock.Instant(.seconds(0)),
+            ackDelay: .microseconds(0)
+        )
+        // Interval 1 [60s, 120s): 80us.
+        rtt.processNewSample(
+            ackDuration: .microseconds(80),
+            packetAckedTime: NetworkClock.Instant(.seconds(119)),
+            ackDelay: .microseconds(0)
+        )
+        // Interval 3 [180s, 240s): 90us. Three full intervals have elapsed
+        // since t=0, even though only 62s have elapsed since the last sample.
+        rtt.processNewSample(
+            ackDuration: .microseconds(90),
+            packetAckedTime: NetworkClock.Instant(.seconds(181)),
+            ackDelay: .microseconds(0)
+        )
+        XCTAssertEqual(rtt.baseRTT, .microseconds(40))
+
+        // Interval 9 [540s, 600s): the interval 0 minimum is still in the window.
+        rtt.processNewSample(
+            ackDuration: .microseconds(100),
+            packetAckedTime: NetworkClock.Instant(.seconds(599)),
+            ackDelay: .microseconds(0)
+        )
+        XCTAssertEqual(rtt.baseRTT, .microseconds(40))
+
+        // Interval 10 [600s, 660s): 10 intervals have elapsed, so interval 0's
+        // slot is reused and its 40us minimum is evicted. The base RTT becomes
+        // the minimum of intervals 1, 3, 9 and 10.
+        rtt.processNewSample(
+            ackDuration: .microseconds(120),
+            packetAckedTime: NetworkClock.Instant(.seconds(605)),
+            ackDelay: .microseconds(0)
+        )
+        XCTAssertEqual(rtt.baseRTT, .microseconds(80))
+        XCTAssertEqual(rtt.minRTT, .microseconds(40))
+    }
+
+    // Verifies that a long idle period (much longer than the history window)
+    // invalidates every slot, including the one that was current before the
+    // idle period, even when the new sample lands on a different slot.
+    func testRTTBaseRolloverLongIdle() {
+        // Interval 0, slot 0: 40us.
+        rtt.processNewSample(
+            ackDuration: .microseconds(40),
+            packetAckedTime: NetworkClock.Instant(.seconds(0)),
+            ackDelay: .microseconds(0)
+        )
+        XCTAssertEqual(rtt.baseRTT, .microseconds(40))
+
+        // About a day later: 1443 intervals have elapsed, so the new sample
+        // lands on slot 3. The slot 0 minimum is long out of the window and
+        // must not survive.
+        rtt.processNewSample(
+            ackDuration: .microseconds(100),
+            packetAckedTime: NetworkClock.Instant(.seconds(1443 * 60 + 1)),
+            ackDelay: .microseconds(0)
+        )
+        XCTAssertEqual(rtt.baseRTT, .microseconds(100))
+        XCTAssertEqual(rtt.minRTT, .microseconds(40))
+    }
 }
 
 #endif