std.time.Timer counts suspend time

silversquirl commented 2 years ago

Zig Version

0.10.0-dev.1318+84f96779c

Steps to Reproduce

Create a file with the following content:

const std = @import("std");
pub fn main() !void {
    var t = try std.time.Timer.start();
    std.time.sleep(std.time.ns_per_s * 5);
    std.debug.print("{}\n", .{std.fmt.fmtDuration(t.read())});
}

Compile this file, run the executable, then immediately suspend the machine. Wake up the machine 10-20s later.

Expected Behavior

The timer should report around 5 seconds; the time actually spent awake.

Actual Behavior

The timer includes suspended time. This change appears to have been made purposefully in #10972, in order to attempt to standardize the behaviour across platforms. However, this makes Timer unsuitable for many timing applications, such as performance and framerate measuring.

I would suggest to either revert the behaviour and fix the Windows implementation unconditionally; or to implement an options struct for both Instant.now and Timer.start to allow choosing whether to include suspended time or not.

In order to fix the Windows implementation, it seems like you can read the QpcBias to adjust for suspended time.

komuw commented 2 years ago

kprotty commented 2 years ago

Is there a way to get time without suspend on platforms like netbsd and wasi? These platforms seem to only provide MONOTONIC clock sources and it would feel odd to encounter this issue on those platforms if support was added only for the others.

silversquirl commented 2 years ago

Regardless of whether it's supported by all platforms, there really needs to be a way to get suspend-exclusive timing on the platforms that do support it. If it's not supported everywhere, the "options struct" route could be taken, with a compile error on platforms where it's not supported.

kprotty commented 2 years ago

Not sure the need is apparent to add suspend-exclusive timing for the stdlib in particular. Most uses of time measurements are done on continuously running systems so the stdlib provides apis for that. This is also the least common denominator for platform support as mentioned earlier.

Suspend-exclusive timing, at worst, could be its own library. If a PR is made however, and you wish to use the std.time.Instant api, i'd suggest making it a different type to avoid the foot-gun of using Instants from different clock sources together. The "options struct" approach sounds fine for std.time.Timer though.

ziglang / zig