Re: [PATCH] use monotonic clock where possible

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On Mon, Dec 16, 2024 at 11:52 PM dana <dana@xxxxxxx> wrote:
>
> several shell features use wall time to calculate deltas and intervals, which
> makes them unreliable if ntp or something else updates the system clock. this
> change makes most of them use the monotonic clock instead
>
> features updated:
>
> * MAILCHECK parameter
> * PERIOD parameter
> * SECONDS parameter
> * %(nS.t.f) prompt-expansion sequence
> * time built-in's elapsed time and cpu % values
> * zsh/zftp ZFTP_TMOUT parameter
> * zsh/zprof timings
>
> these changes also give us (nominally) nanosecond-precise SECONDS and `time`
> elapsed times on most platforms
>
> features that arguably should use monotonic time but still don't after this
> change:
>
> * TMOUT parameter. this is currently implemented by checking tty access time
>   via the file system
>
> * history command durations. i was going to add new monotonic fields to struct
>   histent for calculating the duration, but it felt messy and i would need to
>   study it more to understand when it's ok to use wall vs mono for some of the
>   comparisons we do
>
> * timed functions and zsh/sched commands. i think it should be possible to set
>   these based on either monotonic or wall time (with the sched built-in using
>   monotonic for relative times), but idk how to square that with the current
>   expectation that timedfn/schedcmd times be comparable with each other
>
> * zsh/watch checks. the check time is used both for the LOGCHECK interval and
>   to compare against the utmp log-in times
>
> related changes included:
>
> * zgettime_monotonic_if_available() now uses CLOCK_MONOTONIC_RAW instead of
>   CLOCK_MONOTONIC on macos. it's explained why in the source
>
> * added a zmonotime() convenience function to replace time(2) where applicable
>
> * updated TIMEFMT to allow %nU, %nS, %nE. note that %U and %S remain limited
>   to microsecond precision (if that) on all platforms by the getrusage(2) and
>   times(2) apis
>
> almost none of the features i changed had existing tests and i had trouble
> coming up with any myself. some of the ones i made may be brittle. please lmk
> if you have better ideas
>
> ps: i think it would be pleasing to eliminate gettimeofday(2) and timeval from
> the code base (nearly) entirely so that nobody has to think about the
> distinction or worry about conversion. replacing select(2) by pselect(2) would
> address most of the remaining uses. but i didn't do that here
>
> dana
>
>
> diff --git a/Test/A08time.ztst b/Test/A08time.ztst
> index 22a460f5e..4a41cc76a 100644
> --- a/Test/A08time.ztst
> +++ b/Test/A08time.ztst
> @@ -11,9 +11,44 @@
>    (time cat) >&/dev/null
>  0:`time' keyword (status only)
>
> -  ( TIMEFMT='%E %mE %uE %* %m%mm %u%uu'; time (:) )
> +  ( TIMEFMT='%E %mE %uE %nE %* %m%mm %u%uu %n%nn'; time (:) )
>  0:`time' keyword with custom TIMEFMT
> -*?[0-9]##.[0-9](#c2)s [0-9]##ms [0-9]##us %\* %m%mm %u%uu
> +*?[0-9]##.[0-9](#c2)s [0-9]##ms [0-9]##us [0-9]##ns %\* %m%mm %u%uu %n%nn
> +
> +  ( TIMEFMT='x %U %S %E'; time (:) )
> +0:TIMEFMT %[USE] use centisecond precision
> +*?x( <0-9>.<00-99>s)(#c3)
> +
> +  ( TIMEFMT='x %*U %*S %*E'; time (:) )
> +0:TIMEFMT %*[USE] use millisecond precision
> +*?x( <0-9>.<000-999>)(#c3)
> +
> +  ( TIMEFMT='%nU %nS'; time (read -k3 -t0.1) )
> +1:TIMEFMT %nU and %nS are limited to microsecond precision
> +*?[1-9][0-9]#000ns [1-9][0-9]#000ns

This test fails for me, the output is most of the time just "0ns 0ns",
would this patch be correct? (When it is not, I get 1000000ns)
diff --git a/Test/A08time.ztst b/Test/A08time.ztst
index 4a41cc76a6..071038d1f9 100644
--- a/Test/A08time.ztst
+++ b/Test/A08time.ztst
@@ -25,7 +25,7 @@

   ( TIMEFMT='%nU %nS'; time (read -k3 -t0.1) )
 1:TIMEFMT %nU and %nS are limited to microsecond precision
-*?[1-9][0-9]#000ns [1-9][0-9]#000ns
+*?(0|[1-9][0-9]#000)ns (0|[1-9][0-9]#000)ns

 # SECONDS (after - before) must be greater than the elapsed time, but not much
 # greater. 25% was picked arbitrarily as something that hopefully will prevent


-- 
Mikael Magnusson