KT-58046 Use saturated math in LongTimeMark implementation

- keep offset in range (-1..+1) of time source units
- when adding big or infinite offset, saturate startedAt instead
- displace initial reading to zero, similar to MonotonicTimeSource
- fix the zero reading in TestTimeSource by calling markNow in constructor
- use more precise reading adjustment in TestTimeSource for big durations
- add a note about comparable time marks for AbstractLongTimeSource and TestTimeSource

libraries/stdlib/jvm/test/time/TimeMarkJVMTest.kt

@@ -14,7 +14,7 @@ class TimeMarkJVMTest {
 
     @Test
     fun longDurationElapsed() {
-        TimeMarkTest().testLongDisplacement(TimeSource.Monotonic, { waitDuration -> Thread.sleep((waitDuration * 1.1).inWholeMilliseconds) })
+        TimeMarkTest().testLongAdjustmentElapsedPrecision(TimeSource.Monotonic, { waitDuration -> Thread.sleep((waitDuration * 1.1).inWholeMilliseconds) })
     }
 
     @Test

libraries/stdlib/src/kotlin/time/TimeSources.kt

@@ -5,9 +5,7 @@
 
 package kotlin.time
 
-import kotlin.time.Duration.Companion.milliseconds
+import kotlin.math.sign
-import kotlin.time.Duration.Companion.nanoseconds
-import kotlin.time.Duration.Companion.seconds
 
 @SinceKotlin("1.3")
 @ExperimentalTime
@@ -21,6 +19,9 @@ internal expect object MonotonicTimeSource : TimeSource.WithComparableMarks {
 /**
  * An abstract class used to implement time sources that return their readings as [Long] values in the specified [unit].
  *
+ * Time marks returned by this time source can be compared for difference with other time marks
+ * obtained from the same time source.
+ *
  * @property unit The unit in which this time source's readings are expressed.
  */
 @SinceKotlin("1.3")
@@ -29,58 +30,62 @@ public abstract class AbstractLongTimeSource(protected val unit: DurationUnit) :
     /**
      * This protected method should be overridden to return the current reading of the time source expressed as a [Long] number
      * in the unit specified by the [unit] property.
+     *
+     * Note that the value returned by this method when [markNow] is called the first time is used as "zero" reading
+     * and the difference from this "zero" reading is calculated for subsequent values.
+     * Therefore, it's not recommended to return values farther than `±Long.MAX_VALUE` from the first returned reading
+     * as this will cause this time source flip over future/past boundary for the returned time marks.
      */
     protected abstract fun read(): Long
 
+    private val zero by lazy { read() }
+    private fun adjustedRead(): Long = read() - zero
+
     private class LongTimeMark(private val startedAt: Long, private val timeSource: AbstractLongTimeSource, private val offset: Duration) : ComparableTimeMark {
-        override fun elapsedNow(): Duration = if (offset.isInfinite()) -offset else (timeSource.read() - startedAt).toDuration(timeSource.unit) - offset
+        override fun elapsedNow(): Duration =
-        override fun plus(duration: Duration): ComparableTimeMark = LongTimeMark(startedAt, timeSource, offset + duration)
+            saturatingOriginsDiff(timeSource.adjustedRead(), startedAt, timeSource.unit) - offset
+
+        override fun plus(duration: Duration): ComparableTimeMark {
+            val unit = timeSource.unit
+            if (duration.isInfinite()) {
+                val newValue = saturatingAdd(startedAt, unit, duration)
+                return LongTimeMark(newValue, timeSource, Duration.ZERO)
+            }
+            val durationInUnit = duration.truncateTo(unit)
+            val rest = (duration - durationInUnit) + offset
+            var sum = saturatingAdd(startedAt, unit, durationInUnit)
+            val restInUnit = rest.truncateTo(unit)
+            sum = saturatingAdd(sum, unit, restInUnit)
+            var restUnderUnit = rest - restInUnit
+            val restUnderUnitNs = restUnderUnit.inWholeNanoseconds
+            if (sum != 0L && restUnderUnitNs != 0L && (sum xor restUnderUnitNs) < 0L) {
+                // normalize offset to be the same sign as new startedAt
+                val correction = restUnderUnitNs.sign.toDuration(unit)
+                sum = saturatingAdd(sum, unit, correction)
+                restUnderUnit -= correction
+            }
+            val newValue = sum
+            val newOffset = if (newValue.isSaturated()) Duration.ZERO else restUnderUnit
+            return LongTimeMark(newValue, timeSource, newOffset)
+        }
+
         override fun minus(other: ComparableTimeMark): Duration {
             if (other !is LongTimeMark || this.timeSource != other.timeSource)
                 throw IllegalArgumentException("Subtracting or comparing time marks from different time sources is not possible: $this and $other")
 
-//            val thisValue = this.effectiveDuration()
+            val startedAtDiff = saturatingOriginsDiff(this.startedAt, other.startedAt, timeSource.unit)
-//            val otherValue = other.effectiveDuration()
+            return startedAtDiff + (offset - other.offset)
-//            if (thisValue == otherValue && thisValue.isInfinite()) return Duration.ZERO
-//            return thisValue - otherValue
-            if (this.offset == other.offset && this.offset.isInfinite()) return Duration.ZERO
-            val offsetDiff = this.offset - other.offset
-            val startedAtDiff = (this.startedAt - other.startedAt).toDuration(timeSource.unit)
-//            println("$startedAtDiff, $offsetDiff")
-            return if (startedAtDiff == -offsetDiff) Duration.ZERO else startedAtDiff + offsetDiff
         }
 
         override fun equals(other: Any?): Boolean =
             other is LongTimeMark && this.timeSource == other.timeSource && (this - other) == Duration.ZERO
 
-        internal fun effectiveDuration(): Duration {
+        override fun hashCode(): Int = offset.hashCode() * 37 + startedAt.hashCode()
-            if (offset.isInfinite()) return offset
-            val unit = timeSource.unit
-            if (unit >= DurationUnit.MILLISECONDS) {
-                return startedAt.toDuration(unit) + offset
-            }
-            val scale = convertDurationUnit(1L, DurationUnit.MILLISECONDS, unit)
-            val startedAtMillis = startedAt / scale
-            val startedAtRem = startedAt % scale
 
-            return offset.toComponents { offsetSeconds, offsetNanoseconds ->
+        override fun toString(): String = "LongTimeMark($startedAt${timeSource.unit.shortName()} + $offset, $timeSource)"
-                val offsetMillis = offsetNanoseconds / NANOS_IN_MILLIS
-                val offsetRemNanos = offsetNanoseconds % NANOS_IN_MILLIS
-
-                // add component-wise
-                (startedAtRem.toDuration(unit) + offsetRemNanos.nanoseconds) +
-                        (startedAtMillis + offsetMillis).milliseconds +
-                        offsetSeconds.seconds
-            }
-
-        }
-
-        override fun hashCode(): Int = effectiveDuration().hashCode()
-
-        override fun toString(): String = "LongTimeMark($startedAt${timeSource.unit.shortName()} + $offset (=${effectiveDuration()}), $timeSource)"
     }
 
-    override fun markNow(): ComparableTimeMark = LongTimeMark(read(), this, Duration.ZERO)
+    override fun markNow(): ComparableTimeMark = LongTimeMark(adjustedRead(), this, Duration.ZERO)
 }
 
 /**
@@ -137,6 +142,9 @@ public abstract class AbstractDoubleTimeSource(protected val unit: DurationUnit)
  * timeSource += 10.seconds
  * ```
  *
+ * Time marks returned by this time source can be compared for difference with other time marks
+ * obtained from the same time source.
+ *
  * Implementation note: the current reading value is stored as a [Long] number of nanoseconds,
  * thus it's capable to represent a time range of approximately ±292 years.
  * Should the reading value overflow as the result of [plusAssign] operation, an [IllegalStateException] is thrown.
@@ -146,6 +154,10 @@ public abstract class AbstractDoubleTimeSource(protected val unit: DurationUnit)
 public class TestTimeSource : AbstractLongTimeSource(unit = DurationUnit.NANOSECONDS) {
     private var reading: Long = 0L
 
+    init {
+        markNow() // fix zero reading in the super time source
+    }
+
     override fun read(): Long = reading
 
     /**
@@ -159,17 +171,25 @@ public class TestTimeSource : AbstractLongTimeSource(unit = DurationUnit.NANOSEC
      */
     public operator fun plusAssign(duration: Duration) {
         val longDelta = duration.toLong(unit)
-        reading = if (longDelta != Long.MIN_VALUE && longDelta != Long.MAX_VALUE) {
+        if (!longDelta.isSaturated()) {
             // when delta fits in long, add it as long
             val newReading = reading + longDelta
             if (reading xor longDelta >= 0 && reading xor newReading < 0) overflow(duration)
-            newReading
+            reading = newReading
         } else {
-            val delta = duration.toDouble(unit)
+            val half = duration / 2
-            // when delta is greater than long, add it as double
+            if (!half.toLong(unit).isSaturated()) {
-            val newReading = reading + delta
+                val readingBefore = reading
-            if (newReading > Long.MAX_VALUE || newReading < Long.MIN_VALUE) overflow(duration)
+                try {
-            newReading.toLong()
+                    plusAssign(half)
+                    plusAssign(duration - half)
+                } catch (e: IllegalStateException) {
+                    reading = readingBefore
+                    throw e
+                }
+            } else {
+                overflow(duration)
+            }
         }
     }
 

libraries/stdlib/src/kotlin/time/longSaturatedMath.kt

@@ -76,5 +76,5 @@ private fun saturatingFiniteDiff(value1: Long, value2: Long, unit: DurationUnit)
 }
 
 @Suppress("NOTHING_TO_INLINE")
-private inline fun Long.isSaturated(): Boolean =
+internal inline fun Long.isSaturated(): Boolean =
     (this - 1) or 1 == Long.MAX_VALUE // == either MAX_VALUE or MIN_VALUE

libraries/stdlib/test/time/TestTimeSourceTest.kt

@@ -41,8 +41,10 @@ class TestTimeSourceTest {
         run {
             val timeSource = TestTimeSource()
             timeSource += moderatePositiveDuration
+            val mark = timeSource.markNow()
             // does not overflow even if duration doesn't fit in long, but the result fits
             timeSource += -moderatePositiveDuration - Long.MAX_VALUE.nanoseconds
+            assertEquals(-(moderatePositiveDuration + Long.MAX_VALUE.nanoseconds), mark.elapsedNow())
         }
     }
 

libraries/stdlib/test/time/TimeMarkTest.kt

@@ -6,40 +6,66 @@
 @file:OptIn(ExperimentalTime::class)
 package test.time
 
+import kotlin.math.sign
 import kotlin.test.*
 import kotlin.time.*
 import kotlin.time.Duration.Companion.microseconds
 import kotlin.time.Duration.Companion.milliseconds
 import kotlin.time.Duration.Companion.nanoseconds
+import kotlin.time.Duration.Companion.seconds
 
 @OptIn(ExperimentalTime::class)
 class TimeMarkTest {
+    private val units = DurationUnit.values()
+
+    private fun TimeMark.assertHasPassed(hasPassed: Boolean) {
+        assertEquals(!hasPassed, this.hasNotPassedNow(), "Expected mark in the future")
+        assertEquals(hasPassed, this.hasPassedNow(), "Expected mark in the past")
+
+        assertEquals(
+            !hasPassed,
+            this.elapsedNow() < Duration.ZERO,
+            "Mark elapsed: ${this.elapsedNow()}, expected hasPassed: $hasPassed"
+        )
+    }
+
+    fun testAdjustment(timeSource: TimeSource.WithComparableMarks) {
+        val mark = timeSource.markNow()
+        for (unit in units) {
+            val markFuture1 = (mark + 1.toDuration(unit)).apply { assertHasPassed(false) }
+            val markFuture2 = (mark - (-1).toDuration(unit)).apply { assertHasPassed(false) }
+            assertDifferentMarks(markFuture1, mark, 1)
+            assertDifferentMarks(markFuture2, mark, 1)
+
+            val markPast1 = (mark - 1.toDuration(unit)).apply { assertHasPassed(true) }
+            val markPast2 = (markFuture1 + (-2).toDuration(unit)).apply { assertHasPassed(true) }
+            assertDifferentMarks(markPast1, mark, -1)
+            assertDifferentMarks(markPast2, mark, -1)
+
+            if (unit > DurationUnit.NANOSECONDS) {
+                val d = 1.toDuration(unit)
+                val h = d / 2
+                val markH1 = mark + h
+                val markH2 = mark + d - h
+                assertEqualMarks(markH1, markH2)
+            }
+        }
+    }
 
     @Test
     fun adjustment() {
-        val timeSource = TestTimeSource()
+        testAdjustment(TestTimeSource())
-
+        for (unit in units) {
-        fun TimeMark.assertHasPassed(hasPassed: Boolean) {
+            testAdjustment(LongTimeSource(unit))
-            assertEquals(!hasPassed, this.hasNotPassedNow(), "Expected mark in the future")
-            assertEquals(hasPassed, this.hasPassedNow(), "Expected mark in the past")
-
-            assertEquals(
-                !hasPassed,
-                this.elapsedNow() < Duration.ZERO,
-                "Mark elapsed: ${this.elapsedNow()}, expected hasPassed: $hasPassed"
-            )
         }
+    }
 
+    @Test
+    fun adjustmentTestTimeSource() {
+        val timeSource = TestTimeSource()
         val mark = timeSource.markNow()
-        val markFuture1 = (mark + 1.milliseconds).apply { assertHasPassed(false) }
+        val markFuture1 = mark + 1.milliseconds
-        val markFuture2 = (mark - (-1).milliseconds).apply { assertHasPassed(false) }
+        val markPast1 = mark - 1.milliseconds
-        assertTrue(markFuture1 > mark)
-        assertTrue(markFuture2 > mark)
-
-        val markPast1 = (mark - 1.milliseconds).apply { assertHasPassed(true) }
-        val markPast2 = (markFuture1 + (-2).milliseconds).apply { assertHasPassed(true) }
-        assertTrue(markPast1 < mark)
-        assertTrue(markPast2 < mark)
 
         timeSource += 500_000.nanoseconds
 
@@ -52,14 +78,12 @@ class TimeMarkTest {
 
         assertEquals(0.5.milliseconds, elapsed)
         assertEquals(elapsedFromFuture, markFuture1.elapsedNow())
-        assertEquals(elapsedFromFuture, markFuture2.elapsedNow())
         assertEquals(elapsedDiff, elapsed)
 
         val markToElapsed = mark + elapsedDiff
         assertEqualMarks(markElapsed, markToElapsed)
 
         assertEquals(elapsedFromPast, markPast1.elapsedNow())
-        assertEquals(elapsedFromPast, markPast2.elapsedNow())
 
         markFuture1.assertHasPassed(false)
         markPast1.assertHasPassed(true)
@@ -68,7 +92,6 @@ class TimeMarkTest {
 
         markFuture1.assertHasPassed(true)
         markPast1.assertHasPassed(true)
-
     }
 
     fun testAdjustmentInfinite(timeSource: TimeSource.WithComparableMarks) {
@@ -76,9 +99,9 @@ class TimeMarkTest {
         val infiniteFutureMark = baseMark + Duration.INFINITE
         val infinitePastMark = baseMark - Duration.INFINITE
 
-        assertTrue(infinitePastMark < baseMark)
+        assertDifferentMarks(infinitePastMark, baseMark, -1)
-        assertTrue(baseMark < infiniteFutureMark)
+        assertDifferentMarks(infiniteFutureMark, baseMark, 1)
-        assertTrue(infinitePastMark < infiniteFutureMark)
+        assertDifferentMarks(infinitePastMark, infiniteFutureMark, -1)
 
         assertEquals(Duration.INFINITE, infiniteFutureMark - infinitePastMark)
         assertEquals(Duration.INFINITE, infiniteFutureMark - baseMark)
@@ -95,6 +118,13 @@ class TimeMarkTest {
         assertFailsWith<IllegalArgumentException> { infiniteFutureMark - Duration.INFINITE }
         assertFailsWith<IllegalArgumentException> { infinitePastMark + Duration.INFINITE }
 
+        for (infiniteMark in listOf(infiniteFutureMark, infinitePastMark)) {
+            for (offset in listOf(Duration.ZERO, 1.nanoseconds, 10.microseconds, 1.milliseconds, 15.seconds)) {
+                assertEqualMarks(infiniteMark, infiniteMark + offset)
+                assertEqualMarks(infiniteMark, infiniteMark - offset)
+            }
+        }
+
 
         val longDuration = Long.MAX_VALUE.nanoseconds
         val long2Duration = longDuration + 1001.milliseconds
@@ -117,9 +147,12 @@ class TimeMarkTest {
     @Test
     fun adjustmentInfinite() {
         testAdjustmentInfinite(TestTimeSource())
+        for (unit in units) {
+            testAdjustmentInfinite(LongTimeSource(unit))
+        }
     }
 
-    fun testLongDisplacement(timeSource: TimeSource.WithComparableMarks, wait: (Duration) -> Unit) {
+    fun testLongAdjustmentElapsedPrecision(timeSource: TimeSource.WithComparableMarks, wait: (Duration) -> Unit) {
         val baseMark = timeSource.markNow()
         val longDuration = Long.MAX_VALUE.nanoseconds
         val waitDuration = 20.milliseconds
@@ -131,13 +164,13 @@ class TimeMarkTest {
         assertTrue(elapsed > longDuration)
         assertTrue(elapsed >= longDuration + waitDuration, "$elapsed, $longDuration, $waitDuration")
         assertTrue(elapsedDiff >= longDuration + waitDuration)
-        assertTrue(elapsed >= elapsedDiff)
+        assertEquals(elapsed, elapsedDiff)
     }
 
     @Test
     fun longDisplacement() {
         val timeSource = TestTimeSource()
-        testLongDisplacement(timeSource, { waitDuration -> timeSource += waitDuration })
+        testLongAdjustmentElapsedPrecision(timeSource, { waitDuration -> timeSource += waitDuration })
     }
 
     private fun assertEqualMarks(mark1: ComparableTimeMark, mark2: ComparableTimeMark) {
@@ -149,6 +182,16 @@ class TimeMarkTest {
         assertEquals(mark1.hashCode(), mark2.hashCode(), "hashCodes of: $mark1, $mark2")
     }
 
+    private fun assertDifferentMarks(mark1: ComparableTimeMark, mark2: ComparableTimeMark, expectedCompare: Int) {
+        assertNotEquals(Duration.ZERO, mark1 - mark2)
+        assertNotEquals(Duration.ZERO, mark2 - mark1)
+        assertEquals(expectedCompare, (mark1 compareTo mark2).sign)
+        assertEquals(-expectedCompare, (mark2 compareTo mark1).sign)
+        assertNotEquals(mark1, mark2)
+        // can't say anything about hash codes for non-equal marks
+        // assertNotEquals(mark1.hashCode(), mark2.hashCode(), "hashCodes of: $mark1, $mark2")
+    }
+
     @Test
     fun timeMarkDifferenceAndComparison() {
         val timeSource = TestTimeSource()
@@ -193,20 +236,27 @@ class TimeMarkTest {
 
     @Test
     fun longTimeMarkInfinities() {
-        val timeSource = LongTimeSource(unit = DurationUnit.MILLISECONDS).apply { reading = Long.MIN_VALUE + 1 }
+        for (unit in units) {
+            val timeSource = LongTimeSource(unit).apply {
+                markNow() // fix zero reading
+                reading = Long.MIN_VALUE + 1
+            }
 
-        val mark1 = timeSource.markNow()
+            val mark1 = timeSource.markNow()
-        timeSource.reading = 0
+            timeSource.reading = 0
-        val mark2 = timeSource.markNow() - Duration.INFINITE
+            val mark2 = timeSource.markNow() - Duration.INFINITE
-        assertEquals(Duration.INFINITE, mark1.elapsedNow())
+            if (unit >= DurationUnit.MILLISECONDS) {
-        assertEquals(Duration.INFINITE, mark2.elapsedNow())
+                assertEquals(Duration.INFINITE, mark1.elapsedNow())
-        assertEqualMarks(mark1, mark2)
+            }
+            assertEquals(Duration.INFINITE, mark2.elapsedNow())
+            assertDifferentMarks(mark1, mark2, 1)
 
-        val mark3 = mark1 + Duration.INFINITE
+            val mark3 = mark1 + Duration.INFINITE
-        assertEquals(-Duration.INFINITE, mark3.elapsedNow(), "infinite offset should override distant past reading")
+            assertEquals(-Duration.INFINITE, mark3.elapsedNow(), "infinite offset should override distant past reading")
-        val mark4 = timeSource.markNow() + Duration.INFINITE
+            val mark4 = timeSource.markNow() + Duration.INFINITE
-        assertEquals(-Duration.INFINITE, mark4.elapsedNow())
+            assertEquals(-Duration.INFINITE, mark4.elapsedNow())
-        assertEqualMarks(mark3, mark4) // different readings, same infinite offset
+            assertEqualMarks(mark3, mark4) // different readings, same infinite offset
+        }
     }
 
     @Test
@@ -223,51 +273,52 @@ class TimeMarkTest {
 
     @Test
     fun longTimeMarkRoundingEqualHashCode() {
-        // TODO: small reading, small offset
         run {
             val step = Long.MAX_VALUE / 4
             val timeSource = LongTimeSource(DurationUnit.NANOSECONDS)
-            val mark0 = timeSource.markNow() + (step * 2).nanoseconds
+            val mark0 = timeSource.markNow() + step.nanoseconds + step.nanoseconds
             timeSource.reading += step
             val mark1 = timeSource.markNow() + step.nanoseconds
             timeSource.reading += step
             val mark2 = timeSource.markNow()
             assertEqualMarks(mark1, mark2)
             assertEqualMarks(mark0, mark2)
-//            assertEqualMarks(mark0, mark1) // doesn't pass
+            assertEqualMarks(mark0, mark1)
         }
 
+        for (unit in units) {
+            val baseReading = Long.MAX_VALUE - 1000
+            val timeSource = LongTimeSource(unit).apply { reading = baseReading }
+            // large reading, small offset
+            val baseMark = timeSource.markNow()
+            for (delta in listOf((1..<500).random(), (500..<1000).random())) {
+                val deltaDuration = delta.toDuration(unit)
+                timeSource.reading = baseReading + delta
+                val mark1e = timeSource.markNow()
+                assertEquals(deltaDuration, mark1e - baseMark)
+                val mark1d = baseMark + deltaDuration
+                assertEqualMarks(mark1e, mark1d)
 
-        // TODO: small reading, large offset
+                val subUnit = units.getOrNull(units.indexOf(unit) - 1) ?: continue
+                val deltaSubUnitDuration = delta.toDuration(subUnit)
+                val mark1s = baseMark + deltaSubUnitDuration
+                assertDifferentMarks(mark1s, baseMark, 1)
+                assertEquals(deltaSubUnitDuration, mark1s - baseMark)
+            }
 
-        val unit = DurationUnit.MICROSECONDS
+            // compared saturated reading from time source and saturated time mark as a result of plus operation
-        val baseReading = Long.MAX_VALUE - 1000
+            run {
-        val timeSource = LongTimeSource(unit).apply { reading = baseReading }
+                val delta = 1000
-        // large reading, small offset
+                val deltaDuration = delta.toDuration(unit)
-        val baseMark = timeSource.markNow()
+                timeSource.reading = baseReading + 1000
-        for (delta in listOf((1..<500).random(), (500..<1000).random())) {
+                val mark2 = timeSource.markNow()
-            val deltaDuration = delta.toDuration(unit)
+                assertEquals(deltaDuration, mark2 - baseMark)
-            timeSource.reading = baseReading + delta
+                val offset = Long.MAX_VALUE.nanoseconds
-            val mark1e = timeSource.markNow()
+                val mark2e = mark2 + offset
-            assertEquals(deltaDuration, mark1e - baseMark)
+                val mark2d = baseMark + offset + deltaDuration
-            val mark1d = baseMark + deltaDuration
+                assertEqualMarks(mark2e, mark2d)
-            assertEqualMarks(mark1e, mark1d)
+            }
         }
-
-        // large reading, large offset
-        run {
-            val delta = 1000
-            val deltaDuration = delta.toDuration(unit)
-            timeSource.reading = baseReading + 1000
-            val offset = Long.MAX_VALUE.nanoseconds
-            val mark2 = timeSource.markNow()
-            assertEquals(deltaDuration, mark2 - baseMark)
-            val mark2e = mark2 + offset
-            val mark2d = baseMark + offset + deltaDuration
-            assertEqualMarks(mark2e, mark2d)
-        }
-
-        // TODO: small offset, large offset
     }