Kotlin / kotlinx-datetime

KotlinX multiplatform date/time library
Apache License 2.0
2.43k stars 102 forks source link

Consider removing `Clock.asTimeSource` #372

Open dkhalanskyjb opened 8 months ago

dkhalanskyjb commented 8 months ago

There are no known public usages of this function: https://grep.app/search?q=asTimeSource&words=true Also, Clock.System.asTimeSource().measureTime { } looks fairly benign, even though it doesn't do what you want, given that the system clock can jump in time by several minutes occasionally.

mgroth0 commented 8 months ago

What's the purpose of it then? What can it do that Monotonic cannot?

dkhalanskyjb commented 8 months ago

What's the purpose of what? Clock? It's to show you the current time, according to the system. 2024-03-22T09:43:15Z, as of writing.

mgroth0 commented 8 months ago

Sorry, I meant what is the purpose of Clock.System.asTimeSource() as opposed to using Monotonic. Both are instances of TimeSource, but the Monotonic kdoc says it is the "most precise time source available in the platform". So I just asked out of curisosity if there is any use imaginable use case for Clock.System.asTimeSource(), since you suggest removing it.

dkhalanskyjb commented 7 months ago

The purpose could be, for example, to define your own Clock (not Clock.System) for testing and then also use it as a TimeSource to synchronize the understanding of the time between different parts of your application.

hfhbd commented 6 months ago

How is this issue linked to #382?

Also, isn't the (wall) clock based Timesource more precise than the monotonic due to ntp syncs/monotonic pauses, see https://github.com/Kotlin/kotlinx-datetime/pull/164#issuecomment-997859203?

dkhalanskyjb commented 6 months ago

How is this issue linked to https://github.com/Kotlin/kotlinx-datetime/issues/382?

If we decide this function is useful after all, I think that it, too, should migrate to the stdlib.

Also, isn't the (wall) clock based Timesource more precise than the monotonic due to ntp syncs/monotonic pauses, see https://github.com/Kotlin/kotlinx-datetime/pull/164#issuecomment-997859203?

Wall clocks are neither more nor less precise than TimeSource.Monotonic, they simply serve different needs and fail in different scenarios.

dkhalanskyjb commented 6 months ago

Unrelated to the discussion, but a note about something that made me curious:

because the computer didn't consider some amount of time due to being hibernated.

Looks like this isn't actually what's going to happen with TimeSource.Monotonic: internally, it delegates to https://en.cppreference.com/w/cpp/chrono/steady_clock (https://github.com/JetBrains/kotlin/blob/d00030674aa8323f945fda8d23d55480c1868ba3/kotlin-native/runtime/src/main/cpp/Porting.cpp#L258-L267, https://github.com/JetBrains/kotlin/blob/d00030674aa8323f945fda8d23d55480c1868ba3/kotlin-native/runtime/src/main/kotlin/kotlin/time/MonotonicTimeSource.kt#L16), which promises that "the time between ticks of this clock is constant." If we take this at face value to mean "real time" and not things like "CPU time", hibernation shouldn't affect this.

dalewking commented 6 months ago

Not a public usage of this, but our project uses this for one reason. We have a cache class that is supposed to expire. When we store a value we also store a TimeMark with it to know whether the value has expired. That time mark comes from an object that implements Clock so that we can control time in tests to tests things like the cache expiration.

I think the problem with Monotonic is that it doesn't have a good way to control time in tests. It could probably be made to do so, but I think it is not quite there.

dkhalanskyjb commented 6 months ago

I think the problem with Monotonic is that it doesn't have a good way to control time in tests.

Did you try https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.time/-test-time-source/ ? What's missing from it?

hfhbd commented 6 months ago

I think the problem with Monotonic is that it doesn't have a good way to control time in tests.

What about TestTimeSource?

ilya-g commented 3 weeks ago

That time mark comes from an object that implements Clock so that we can control time in tests to tests things like the cache expiration.

@dalewking However in production, do you use Monotonic time source or the one that is obtained from Clock.System as well? I.e. are your cache expiration timeouts tied to the wall clock time or monotonic time?

dalewking commented 1 week ago

@dalewking However in production, do you use Monotonic time source or the one that is obtained from Clock.System as well? I.e. are your cache expiration timeouts tied to the wall clock time or monotonic time?

Just now getting back to this and we were not using Monotonic and now I agree that Clock.asTimeSource should go away.

For a good discussion on the topic see: https://www.youtube.com/watch?v=i1wysckHg2o