delay(Duration) improperly coerces nanoseconds and microseconds

[kevincianfarini]

Describe the bug

I am currently unsure if this is desired behavior, so if it is, please let me know.

When invoking a delay with a Duration, the value of that duration is coerced to milliseconds and then delegated to delay(Long). It is not documented behavior that delaying with a duration loses granularity for nanoseconds or microseconds. For example, delaying with 998.75909ms gets coerced to a call of 998ms when delegating to delay(Long).

The offending code is the following:

internal fun Duration.toDelayMillis(): Long =
    if (this > Duration.ZERO) inWholeMilliseconds.coerceAtLeast(1) else 0

What happened? What should have happened instead?

The symptom of this issue is that when working on implementing a cron-like API for delay based job scheduling, I was having jobs invoked up to 600us before they should have been invoked. The end result was that jobs scheduled to occur at the beginning of a second in a certain time zone would invoke early once and then as expected afterwards in quick succession.

The above function Duration.toDelaymillis should likely coerce durations which have a nanosecond component up duration.inWholeMilliseconds + 1.

Locally this implementation is working well for me.

internal fun Duration.myToMillis(): Long = if (this > Duration.ZERO) {
    val millis = inWholeMilliseconds
    if (millis * 1_000_000 < inWholeNanoseconds) millis + 1 else millis
} else 0

Provide a Reproducer

    @Test fun foo() = runTest(timeout = 1.days) {
        withContext(Dispatchers.Default) {
            while (true) {
                val expectedDuration = 998.milliseconds + 902752.nanoseconds
                val actualDuration = measureTime { delay(expectedDuration) }
                val difference = actualDuration - expectedDuration
                assertTrue(difference.isPositive()) // fails
            }
        }
    }

[kevincianfarini]

I am happy to raise a PR if this is indeed a bug!

[dkhalanskyjb]

The proposal to round sub-millisecond components up sounds completely reasonable.

[kevincianfarini]

When merged, #3921 should solve this bug, but it's worth considering that the delegation of delay(Duration) to delay(Long) is backwards. The Worker API supports scheduling execution to the microsecond granularity, for example.

I don't know if having nanosecond or microsecond granularity has a real use case, so maybe not worth the effort.

[SPC-code]

Microseconds granularity has a use-case in device operation scheduling. It is not frequently used, but I see some cases, where it could appear.

[dkhalanskyjb]

@SPC-code, that's irrelevant to this PR, as the precision of delay was always only with millisecond granularity. The only thing that's changed is that with this PR, we guarantee that the sleep will last for at least this long.

Please file an issue if you have a use case for sub-millisecond scheduling precision.