Implement Comparable time marks in a time source returned by Clock.asTimeSource() #271
Conversation
core/common/src/Clock.kt
Outdated
|
|
||
| override fun hashCode(): Int = instant.hashCode() | ||
|
|
||
| override fun toString(): String = instant.toString() |
There was a problem hiding this comment.
I'm wary of time marks with different clocks looking the same but not being equal. I don't think anyone will want to parse these, so I don't see any downsides to including the clock in the output as well.
| private fun Instant.saturatingAdd(duration: Duration): Instant { | ||
| if (isSaturated()) { | ||
| if (duration.isInfinite() && duration.isPositive() != this.isDistantFuture) { | ||
| throw IllegalArgumentException("Summing infinities of different signs") |
There was a problem hiding this comment.
But we already have Instant.plus(Duration). Its behavior is slightly different, and I even like saturatingAdd more, but shouldn't saturatingDiff also throw on NaN if we keep saturatingAdd the way it is here?
There was a problem hiding this comment.
shouldn't saturatingDiff also throw on NaN
No, the operation minus on time marks is defined to return zero when calculating difference between two equal infinitely distant time marks.
There was a problem hiding this comment.
Isn't it internally inconsistent? This would make sense in the Instant infinity model of "all events in the far future are just one event," but in the Mark infinity model of "infinitely far events are fuzzily distributed along the timeline," this is surprising.
There was a problem hiding this comment.
It's consistent in time mark infinity model: all the distant events are the same infinitely distant event. It's not consistent with math infinity, but we're ok with that. See the Libraries DM at 2023-04-11.
There was a problem hiding this comment.
Oh, ok. This model is so unintuitive to me that I didn't even think about it. Well, since the behavior is already established and you implemented it precisely, I guess there's no need to discuss anything further, as my gripes are with the behavior, not the implementation.
| assertEquals(Duration.ZERO, markFuture - markFuture) | ||
| assertEquals(Duration.ZERO, markPast - markPast) |
There was a problem hiding this comment.
This highlights that we have a problem with Instant saturation. If we have something like Instant.MAX + 1.hours - (Instant.MAX - 1.hours), we get something like 1.hours, whereas, due to saturation, it should be a NaN. Lacking NaN, I'd settle for some other nonsensical value like Instant.MAX or Instant.MIN, but certainly not what we have now, which is a plausible but incorrect result.
There was a problem hiding this comment.
These are not instants, but time marks, they have different saturation rules. Some are infinitely distant, but we defined the difference between them as zero.
There was a problem hiding this comment.
These are not instants, but time marks, they have different saturation rules
Is there a reason why we can't consider Instant values just a particular sort of time marks where we know exactly which point in real-life time the time mark corresponds to? To me, the distinction is blurry enough that I wouldn't expect their behavior to differ. In fact, even when saturation happens, they behave the same in most cases.
Looks like the difference is just the following:
Instant.MAX - Duration.INFINITEisInstant.MIN(so theDurationis "more infinite"), butMark(Instant.MAX) - Duration.INFINITEthrows.Instant.MAX - (Instant.MAX - 1.seconds)is1.seconds, butMark(Instant.MAX) - Mark(Instant.MAX - 1.seconds)is infinity.
So, we have two models of infinity:
- Beyond
Instant.MAX, all events happen simultaneously, butInstant.MAXhappens immediately afterInstant.MAX - epsilon. So, the time "stops" after a point, collapsing into one event.Durationbehaves consistently with that: since this event that everything collapsed into is just a specific moment in time, we can work with the moment of that event as usual. - Beyond
Mark(Instant.MAX), all events happen simultaneously, andMark(Instant.MAX)is infinitely far fromMark(Instant.MAX - epsilon). So,Mark(Instant.MAX)can be thought of as some undefined moment in the infinitely-far region.Durationbehaves consistently: since that moment is undefined but infinitely far, we don't know anything about it, and adding/subtracting non-infinite durations stays in the undefined region, whereas adding conflicting infinities is erroneous.
Personally, after thinking about it, I like the Mark() one better, I think it reflects the intuition more nicely. WDYT about unifying the behaviors of these concepts? If there's any particular reason why they have to have different concepts of infinity, then sure, use cases are the kings, we'll have to live with the discrepancy, but I just couldn't think of any.
There was a problem hiding this comment.
In InstantTimeMark, the values Instant.MIN and Instant.MAX are chosen to represent infinitely distant time marks. Thus they are sacrificed for that and we can't have a time mark that corresponds exactly to the Instant.MIN/MAX. Technically we could avoid that by introducing some flag to represent infinite saturation, but I don't see much value in that, given that Instant.MIN/MAX are just our internal constants (different on different platforms) and they would hardly be used in practice for something meaningful.
There was a problem hiding this comment.
Technically we could avoid that by introducing some flag to represent infinite saturation, but I don't see much value in that
Yeah, me neither. What I see value in, is making sure Instant and InstantTimeMark work the same way, probably by implementing the operations on Instant to follow suit. With your clarification that Mark(Instant.MAX) also follows the model of "all events in the far future collapse into one," the difference becomes just that Instant considers Instant.MAX to be that last event, whereas Mark considers some infinitely far-away event to be the last one, even though it's represented as Instant.MAX. I don't understand why this distinction is worth introducing.
There was a problem hiding this comment.
Instant considers Instant.MAX to be that last event, whereas Mark considers some infinitely far-away event to be the last one, even though it's represented as Instant.MAX. I don't understand why this distinction is worth introducing.
It's expected that timeMark + Duration.INFINITE returns -Duration.INFINITE when queried for elapsedNow, which would not be possible if InstantTimeMark saturated to some finite moment, like Instant.MAX. Thus we have to either give the infinite meaning to some values of Instant, or to encode infiniteness with some additional flag.
There was a problem hiding this comment.
It's expected that
timeMark + Duration.INFINITEreturns-Duration.INFINITEwhen queried forelapsedNow
By the same logic, wouldn't we also expect the following to be true?
assertEquals(-Duration.INFINITE, instant - (instant + Duration.INFINITE))Thus we have to either give the infinite meaning to some values of Instant
Sure, why not consider Instant.MAX a "truly" infinite value?
There was a problem hiding this comment.
I believe this is out of scope of this PR
No description provided.