kotlin
kotlin copied to clipboard
JetBrains
Switch to use System.nanoTime in measureTimeMillis
Previously measureTimeMillis used System.currentTimeMillis which is not ideal for several reasons. Firstly it often has a resolution of 10s of milliseconds and secondly because it can be adjusted by something like an NTP update so the returned value could be negative.
Instead it now delegates to measureNanoTime which uses System.nanoTime which has the highest resolution the system provides as well as being guaranteed to be monotonically increasing.
This fixes KT-52686 and KT-24431
I would argue that on Android we still want to use nanoTime over elapsedRealtime as it's more likely that the caller does not want to include time spent in sleep.
There is probably related pull request: https://github.com/JetBrains/kotlin/pull/1964
Yeah definitely. But holy moly, open for 4 years?!
That PR seems dead. I've incorporated the changes from the discussion.
Hi folks and sorry for our (missing) PR handling policy!
We've discussed it internally and would like to discuss an alternative proposal.
We do acknowledge the need of measureTimeMillis and that this is the appropriate time unit for most cases. We also acknowledge that in its current state, the API is really dangerous to use -- it's unlikely that an average user expects this method to return a negative number (or even zero), which may lead to potential crashes and unspecified behaviour, not talking about its resolution issue.
Apart from that, we have an adjacent inconsistently-named API: measureNanoTime (named respectively for its underlying source, System.nanoTime()), which looks off when compared with measureTime or measureTimeMillis.
The preliminary idea is the following:
- Stabilize
measureTime -
Soft-deprecate* of
measureTimeMillisandmeasureNanoTimeto nudge people towardsmeasureTime {}ormeasureTime {}.inWholeMilliseconds(the latter is required only if the programmatic use oflongis required)
In that way, we do avoid potentially breaking changes, get rid of APIs we do not like, and encourage people to use the corresponding Duration type (long for time units is well-known to be error-prone).
The downsides are also clear:
- We preserve the status quo, and it's not really helpful for current users (arguably we save a breaking change here, though the impact is likely to be negligible)
- We [soft-]deprecate common API, forcing users to make an extra effort
- We potentially make the previously concise pattern more verbose (
.inWholeMilliseconds), at least for the initial migration phase.
What do you think about that?
- On a side note, it seems like the language itself needs a mechanism for "soft-deprecation" -- not a "this is a warning and bad API, you should migrate from it now", but rather a nudge "here is a better alternative that we kindly suggest with IDE assistance"
On a side note, it seems like the language itself needs a mechanism for "soft-deprecation"
Maybe DeprecationLevel.NOTE? Or INFORMATIVE?
Maybe DeprecationLevel.NOTE? Or INFORMATIVE?
Love the suggestion. I'll try to write a proper YT proposal (aka pre-KEEP) around next week and will definitely use these as viable options
That seems like a good solution and I have already been using measureTime since it was added. But regardless I still think this PR should land as it is a strictly good improvement.