-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Use Duration as the ground truth for communicating durations #4256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
It's a draft to signify that I'm not sure this change is needed and want a second opinion. The simplified code looks nicer to me, but other than that, it's technically a breaking change without a clear user need. I mostly did this to thoroughly familiarize myself with time-based operations in our library. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am generally okay with the approach (probably we'd like to target a major release here).
Maybe we should also align it with all-compatibility
and provide a migration window just in case.
Note that I'd rather skimmed through than reviewed carefully, will do when you'll promote it to the PR
Relates to #3920 and specifically this issue comment. I really like this change and that it theoretically allows for delays to be more granular than milliseconds. |
0255a44
to
f05e1a1
Compare
Historically, the library evolved using "a Long of milliseconds" as the standard of denoting durations. Since then, `kotlin.time.Duration` appeared, encompassing a number of useful conversions. There are several consequences to this change. - Before, `delay(Long)` and `delay(Duration)` were not easily expressed via one another. For example, `delay(Long.MAX_VALUE / 2 + 1)` (up until `Long.MAX_VALUE`) used to be considered a valid delay, but it was not expressible in `delay(Duration)`. Therefore, `delay(Long)` was the more fundamental implementation. However, `delay(Duration)` could not just be expressed as `delay(inWholeMilliseconds)`, as we need to round the durations up when delaying events, and this required complex logic. With this change, `delay(Duration)` is taken as the standard implementation, and `delay(Long)` is just `delay(timeMillis.milliseconds)`, simplifying the conceptual space. - The same goes for other APIs accepting either a duration or some Long number of milliseconds. - In several platform APIs, we are actually able to pass nanoseconds as the duration to wait for. We can now accurately do that. This precision is unlikely to be important in practice, but it is still nice that we are not losing any information in transit. - On Android's main thread, it's no longer possible to wait for `Long.MAX_VALUE / 2` milliseconds: it's considered an infinite duration. `Long.MAX_VALUE / 2 - 1` is still fine. - In `kotlinx-coroutines-test`, before, it was possible to observe correct behavior for up to `Long.MAX_VALUE` milliseconds. Now, this value is drastically reduced, to be able to test the nanosecond precision. - In `kotlinx-coroutines-test`, we now fail with an `IllegalStateException` if we enter the representable ceiling of time during the test. Before, we used to continue the test execution, only using the order in which tasks arrived but not their virtual time values.
f05e1a1
to
3e11443
Compare
Historically, the library evolved using "a Long of milliseconds" as the standard of denoting durations.
Since then,
kotlin.time.Duration
appeared, encompassing a number of useful conversions.There are several consequences to this change.
delay(Long)
anddelay(Duration)
were not easily expressed via one another. For example,delay(Long.MAX_VALUE / 2 + 1)
(up untilLong.MAX_VALUE
) used to be considered a valid delay, but it was not expressible indelay(Duration)
. Therefore,delay(Long)
was the more fundamental implementation. However,delay(Duration)
could not just be expressed asdelay(inWholeMilliseconds)
, as we need to round the durations up when delaying events, and this required complex logic. With this change,delay(Duration)
is taken as the standard implementation, anddelay(Long)
is justdelay(timeMillis.milliseconds)
, simplifying the conceptual space.Long.MAX_VALUE / 2
milliseconds: it's considered an infinite duration.Long.MAX_VALUE / 2 - 1
is still fine.kotlinx-coroutines-test
, before, it was possible to observe correct behavior for up toLong.MAX_VALUE
milliseconds. Now, this value is drastically reduced, to be able to test the nanosecond precision.kotlinx-coroutines-test
, we now fail with anIllegalStateException
if we enter the representable ceiling of time during the test. Before, we used to continue the test execution, only using the order in which tasks arrived but not their virtual time values.