[PATCH] Deprecate current-milliseconds in favor of current-process-milliseconds and improve behaviour a bit [was: Re: Exposing subsecond precision in current-seconds]

[Peter Bex]

On Thu, Apr 30, 2020 at 12:11:31PM +0200, address@hidden wrote:
> > Yeah, like I said in my other message, we could use the name
> > current-process-milliseconds, like Racket;
> > https://docs.racket-lang.org/reference/time.html
> >
> > I like the name, because it's pretty clear what it does; it yields the
> > number of milliseconds the process has been running for.
> >
> > We can make it a procedure without arguments, because that part of the
> > Racket interface looks messy and confusing to me.
> 
> Yes, makes sense.

Attached is a set of two patches.  The first one simply adds a
deprecation notice to current-milliseconds and adds the new procedure
current-process-milliseconds.

While I was looking into this, I've noticed a few strange things.
First off, on Unix, C_startup_time_seconds uses just the second since
startup, ignoring the microseconds.  This leads to erratic behaviour:

$ csi -e -R chicken.time -p '(current-process-milliseconds)'
301
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
910
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
82
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
269
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
905
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
15

With the patch, we get sane behaviour:
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
23
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
24
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
26
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
24

On Windows, we used to use clock(), but according to
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/clock?view=vs-2019
that function stops behaving sanely after 24.8 days of uptime,
at which point it starts returning -1.  This shouldn't usually
be a problem, but on a server running some long-running CHICKEN
process this could be an issue.

Besides, that also says CLOCKS_PER_SEC is defined as 1000 by
Microsoft.  So the logic in there doesn't make much sense since
it's defined as such, and C_NONUNIX is only true on MINGW32.

And if that code were triggered on non-Windows non-UNIX systems,
that would result in this undefined issue where the time since
system startup might be used.  To me it makes a lot more sense
to define current-process-milliseconds as time since process
startup, and drop the wishy-washy documentation that it's
time "since process or system startup".

Cheers,
Peter

0001-Deprecate-current-milliseconds-in-favor-of-current-p.patch
Description: Text Data

0002-Fix-a-few-platform-specific-issues-with-current-proc.patch
Description: Text Data

signature.asc
Description: PGP signature