WebAssembly / wasi-clocks

Clocks API for WASI
40 stars 14 forks source link

monotonic `now` should not care about rollover, except to describe maximum duration #27

Open codefromthecrypt opened 1 year ago

codefromthecrypt commented 1 year ago

In the review session there was a question about if now should handle overflow. I think more precisely it shouldn't be interpreted in a way that assumes monotonic starts at zero. The only important part is the max knowable duration and that depends on if it is interpreted as signed or not.

java's docs are pretty decent on this https://docs.oracle.com/javase/8/docs/api/java/lang/System.html#nanoTime-- note that it indicates the value as a signed number

The value returned represents nanoseconds since some fixed but arbitrary origin time (perhaps in the future, so values may be negative)

sunfishcode commented 1 year ago

It's not clear to me what an origin time "in the future" means for a monotonic clock. I imagine the reason Java returns a signed number is that it doesn't have unsigned types in its type system.

I think the main concern for WASI here is an implementation that happens to start a monotonic clock at some great enough number which happens to ive it significantly less than the 584 years in a full u64 of nanoseconds. In that case, overflow might not be completely implausible. My sense here though is that that's still a very unlikely scenario, and it doesn't seem worth burdening every WASI program that wants monotonicity with the need to think about the case where wraparound occurs.

My proposal here is to say that now() traps if its clock has overflowed. We can also give implementations the guidance that they should pick initial values for their monotonic clocks which don't put them in practical danger of hitting this.

codefromthecrypt commented 1 year ago

I think assumptions underpinning things should be validated by a diversity of environments minimally adding windows and probably also a VM like java. I don't think we want to make something trap on "overflow" knowing that there exist monotonic sources that start in the past. If you feel otherwise, that's fine, just I think some tests should be in place so that when these assumptions fail they fail near the spec that defines these things.

codefromthecrypt commented 1 year ago

ps the main project I work on doesn't have this problem as we save a base time off.

For example, here's how we implement the monotonic clock which won't be negative for some hundreds of years. However, we only implement it this way due to how go decided to implement monotonic readings (by hacking the timespec).

// nanoBase uses time.Now to ensure a monotonic clock reading on all platforms
// via time.Since.
var nanoBase = time.Now()

// nanotimePortable implements sys.Nanotime with time.Since.
//
// Note: This is less efficient than it could be is reading runtime.nanotime(),
// Just to do that requires CGO.
func nanotimePortable() int64 {
    return time.Since(nanoBase).Nanoseconds()
}

Also, ack JVM isn't a likely runtime to embed a wasm runtime, rather more likely the other way around (teavm -> wasm)

Just I've seen bugs around assumptions about the value of monotonic readings enough to raise this issue. Also, on the stream I thought you were trying to be less strictly posix. Finally, the wasi-testsuite will have trouble detecting a sporadic negative reading, so mainly when a trap occurs, folks will have to go back to the source of the runtime and make a fix like ^^ to ensure whatever their clock is can never be negative for a long time.

meanwhile I don't personally care as we won't have this problem except when users plug in a custom clock that can return a negative.

sunfishcode commented 1 year ago

One thing I want to be clear about: in my livestream, I mentioned this being something of a "last call" moment for these APIs. However, it will continue to be possible to make changes, up until preview2 is voted on by the WASI Subgroup. And after preview2, there will likely be a preview3, and eventually a standardized 1.0. There absolutely is time for these APIs to be implemented in a diversity of environments, and to make changes in response to feedback. The "last call" just means that this is a moment when not many toolchains and engines are implementing preview2 yet, so in this moment, we can make changes with minimal coordination.

codefromthecrypt commented 1 year ago

However, it will continue to be possible to make changes, up until preview2 is voted on by the WASI Subgroup. And after preview2, there will likely be a preview3, and eventually a standardized 1.0.

This is a good point to make loudly, also with some expectations of what timeframe is expected. For example, there were two years between snapshot-01 aka preview1, if those are the same things. If people know the magnitude of time expected between preview2 and 3, I think they might behave or react differently.