Re: time(1): use monotonic clock for computing elapsed time
On Sat, Jul 22, 2017 at 10:19:22AM +0200, Anton Lindqvist wrote: > On Fri, Jul 21, 2017 at 04:34:53PM -0500, Scott Cheloha wrote: > > ~1 week bump. Changes to time(1) were committed by tedu@. > > > > Any feedback on the ksh/csh portions of the patch? > > Looks good. I'm willing to commit this diff if I can get another ok. Committed, thanks!
Re: time(1): use monotonic clock for computing elapsed time
On Fri, Jul 21, 2017 at 04:34:53PM -0500, Scott Cheloha wrote: > ~1 week bump. Changes to time(1) were committed by tedu@. > > Any feedback on the ksh/csh portions of the patch? Looks good. I'm willing to commit this diff if I can get another ok.
Re: time(1): use monotonic clock for computing elapsed time
~1 week bump. Changes to time(1) were committed by tedu@. Any feedback on the ksh/csh portions of the patch? -- Scott Cheloha
Re: time(1): use monotonic clock for computing elapsed time
On Thu, Jul 13, 2017 at 13:44 +1000, David Gwynne wrote: > > > On 13 Jul 2017, at 11:16 am, Scott Cheloha wrote: > > > > Hi, > > > > The "real" elapsed time for time(1) and the ksh/csh time builtins is > > currently computed with gettimeofday(2), so it's subject to changes > > by adjtime(2) and, if you're really unlucky, clock_settime(2) or > > settimeofday(2). In pathological cases you can get negative values > > in the output. > > > > This seems wrong to me. I personally use these tools like a stopwatch, > > and I was surprised to see that the elapsed difference wasn't (more) > > immune to changes to the system clock. > > > > The attached patches change the "real" listing for time(1), ksh's time > > builtin, and csh's time builtin to use a monotonic clock, which I think > > more closely matches what the typical user and programmer expects. This > > interpretation is, near as I can tell, also compatible with the POSIX.1 > > 2008 description of the time(1) utility. In particular, the use of > > "elapsed," implying a scalar value, makes me think that this is the > > intended behavior. [1] > > > > NetBSD did this in 2011 without much fanfare, though for some reason they > > did it for time(1) and csh's builtin but not for ksh's builtin. [2] > > > > I've tested pathological cases in each of the three and these patches > > correct the result in said cases without (perceptibly) changing the > > result in the typical case. > > > > Thoughts? Feedback? > > this makes sense to me, id like to see it go in. > Same here. I'm surprised to learn time(1) is not using CLOCK_MONOTONIC.
Re: time(1): use monotonic clock for computing elapsed time
> On 13 Jul 2017, at 11:16 am, Scott Cheloha wrote:
>
> Hi,
>
> The "real" elapsed time for time(1) and the ksh/csh time builtins is
> currently computed with gettimeofday(2), so it's subject to changes
> by adjtime(2) and, if you're really unlucky, clock_settime(2) or
> settimeofday(2). In pathological cases you can get negative values
> in the output.
>
> This seems wrong to me. I personally use these tools like a stopwatch,
> and I was surprised to see that the elapsed difference wasn't (more)
> immune to changes to the system clock.
>
> The attached patches change the "real" listing for time(1), ksh's time
> builtin, and csh's time builtin to use a monotonic clock, which I think
> more closely matches what the typical user and programmer expects. This
> interpretation is, near as I can tell, also compatible with the POSIX.1
> 2008 description of the time(1) utility. In particular, the use of
> "elapsed," implying a scalar value, makes me think that this is the
> intended behavior. [1]
>
> NetBSD did this in 2011 without much fanfare, though for some reason they
> did it for time(1) and csh's builtin but not for ksh's builtin. [2]
>
> I've tested pathological cases in each of the three and these patches
> correct the result in said cases without (perceptibly) changing the
> result in the typical case.
>
> Thoughts? Feedback?
this makes sense to me, id like to see it go in.
>
> --
> Scott Cheloha
>
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/time.html
> [2] http://gnats.netbsd.org/45592
>
> Index: bin/csh/csh.h
> ===
> RCS file: /cvs/src/bin/csh/csh.h,v
> retrieving revision 1.28
> diff -u -p -r1.28 csh.h
> --- bin/csh/csh.h 26 Dec 2015 13:48:38 - 1.28
> +++ bin/csh/csh.h 12 Jul 2017 04:15:04 -
> @@ -122,7 +122,7 @@ char *seterr; /* Error message from
> #include
> #include
>
> -struct timeval time0;/* Time at which the shell started */
> +struct timespec time0; /* Time at which the shell started */
> struct rusage ru0;
>
> /*
> Index: bin/csh/extern.h
> ===
> RCS file: /cvs/src/bin/csh/extern.h,v
> retrieving revision 1.25
> diff -u -p -r1.25 extern.h
> --- bin/csh/extern.h 26 Dec 2015 13:48:38 - 1.25
> +++ bin/csh/extern.h 12 Jul 2017 04:15:04 -
> @@ -272,7 +272,7 @@ void plist(struct varent *);
> void donice(Char **, struct command *);
> void dotime(Char **, struct command *);
> void prusage(struct rusage *, struct rusage *,
> - struct timeval *, struct timeval *);
> + struct timespec *, struct timespec *);
> void ruadd(struct rusage *, struct rusage *);
> void settimes(void);
> void pcsecs(long);
> Index: bin/csh/proc.c
> ===
> RCS file: /cvs/src/bin/csh/proc.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 proc.c
> --- bin/csh/proc.c26 Dec 2015 13:48:38 - 1.30
> +++ bin/csh/proc.c12 Jul 2017 04:15:04 -
> @@ -108,7 +108,7 @@ found:
> }
> else {
> if (pp->p_flags & (PTIME | PPTIME) || adrof(STRtime))
> - (void) gettimeofday(&pp->p_etime, NULL);
> + (void) clock_gettime(CLOCK_MONOTONIC, &pp->p_etime);
>
> pp->p_rusage = ru;
> if (WIFSIGNALED(w)) {
> @@ -494,7 +494,7 @@ palloc(int pid, struct command *t)
> }
> pp->p_next = proclist.p_next;
> proclist.p_next = pp;
> -(void) gettimeofday(&pp->p_btime, NULL);
> +(void) clock_gettime(CLOCK_MONOTONIC, &pp->p_btime);
> }
>
> static void
> @@ -799,8 +799,8 @@ prcomd:
> static void
> ptprint(struct process *tp)
> {
> -struct timeval tetime, diff;
> -static struct timeval ztime;
> +struct timespec tetime, diff;
> +static struct timespec ztime;
> struct rusage ru;
> static struct rusage zru;
> struct process *pp = tp;
> @@ -809,8 +809,8 @@ ptprint(struct process *tp)
> tetime = ztime;
> do {
> ruadd(&ru, &pp->p_rusage);
> - timersub(&pp->p_etime, &pp->p_btime, &diff);
> - if (timercmp(&diff, &tetime, >))
> + timespecsub(&pp->p_etime, &pp->p_btime, &diff);
> + if (timespeccmp(&diff, &tetime, >))
> tetime = diff;
> } while ((pp = pp->p_friends) != tp);
> prusage(&zru, &ru, &tetime, &ztime);
> Index: bin/csh/proc.h
> ===
> RCS file: /cvs/src/bin/csh/proc.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 proc.h
> --- bin/csh/proc.h2 Jun 2003 23:32:07 - 1.3
> +++ bin/csh/proc.h12 Jul 2017 04:15:04 -
> @@ -50,8 +50,8 @@ struct process {
> pid_t p_pid;
> pid_t p_jobid; /* pid of job leader */
> /* if a job is stopped/background p_jobid gives its pgrp */
> -struct timeval p_btime; /* begin time */
> -struct timeval p_etime; /* end time */
> +struc
Re: time(1): use monotonic clock for computing elapsed time
Whoops, prior diff for usr.bin/time/time.c has a dumb typo,
corrected diff attached.
--
Scott Cheloha
Index: usr.bin/time/time.c
===
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.21
diff -u -p -r1.21 time.c
--- usr.bin/time/time.c 10 Oct 2015 14:49:23 - 1.21
+++ usr.bin/time/time.c 13 Jul 2017 01:29:00 -
@@ -52,7 +52,7 @@ main(int argc, char *argv[])
{
pid_t pid;
int ch, status;
- struct timeval before, after;
+ struct timespec before, after, during;
struct rusage ru;
int exitonsig = 0;
@@ -79,7 +79,7 @@ main(int argc, char *argv[])
if (argc < 1)
usage();
- gettimeofday(&before, (struct timezone *)NULL);
+ clock_gettime(CLOCK_MONOTONIC, &before);
switch(pid = vfork()) {
case -1:/* error */
perror("time");
@@ -97,24 +97,23 @@ main(int argc, char *argv[])
(void)signal(SIGQUIT, SIG_IGN);
while (wait3(&status, 0, &ru) != pid)
;
- gettimeofday(&after, (struct timezone *)NULL);
+ clock_gettime(CLOCK_MONOTONIC, &after);
if (WIFSIGNALED(status))
exitonsig = WTERMSIG(status);
if (!WIFEXITED(status))
fprintf(stderr, "Command terminated abnormally.\n");
- timersub(&after, &before, &after);
+ timespecsub(&after, &before, &during);
if (portableflag) {
fprintf(stderr, "real %9lld.%02ld\n",
- (long long)after.tv_sec, after.tv_usec/1);
+ (long long)during.tv_sec, during.tv_nsec/1000);
fprintf(stderr, "user %9lld.%02ld\n",
(long long)ru.ru_utime.tv_sec,
ru.ru_utime.tv_usec/1);
fprintf(stderr, "sys %9lld.%02ld\n",
(long long)ru.ru_stime.tv_sec,
ru.ru_stime.tv_usec/1);
} else {
-
fprintf(stderr, "%9lld.%02ld real ",
- (long long)after.tv_sec, after.tv_usec/1);
+ (long long)during.tv_sec, during.tv_nsec/1000);
fprintf(stderr, "%9lld.%02ld user ",
(long long)ru.ru_utime.tv_sec,
ru.ru_utime.tv_usec/1);
fprintf(stderr, "%9lld.%02ld sys\n",
time(1): use monotonic clock for computing elapsed time
Hi,
The "real" elapsed time for time(1) and the ksh/csh time builtins is
currently computed with gettimeofday(2), so it's subject to changes
by adjtime(2) and, if you're really unlucky, clock_settime(2) or
settimeofday(2). In pathological cases you can get negative values
in the output.
This seems wrong to me. I personally use these tools like a stopwatch,
and I was surprised to see that the elapsed difference wasn't (more)
immune to changes to the system clock.
The attached patches change the "real" listing for time(1), ksh's time
builtin, and csh's time builtin to use a monotonic clock, which I think
more closely matches what the typical user and programmer expects. This
interpretation is, near as I can tell, also compatible with the POSIX.1
2008 description of the time(1) utility. In particular, the use of
"elapsed," implying a scalar value, makes me think that this is the
intended behavior. [1]
NetBSD did this in 2011 without much fanfare, though for some reason they
did it for time(1) and csh's builtin but not for ksh's builtin. [2]
I've tested pathological cases in each of the three and these patches
correct the result in said cases without (perceptibly) changing the
result in the typical case.
Thoughts? Feedback?
--
Scott Cheloha
[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/time.html
[2] http://gnats.netbsd.org/45592
Index: bin/csh/csh.h
===
RCS file: /cvs/src/bin/csh/csh.h,v
retrieving revision 1.28
diff -u -p -r1.28 csh.h
--- bin/csh/csh.h 26 Dec 2015 13:48:38 - 1.28
+++ bin/csh/csh.h 12 Jul 2017 04:15:04 -
@@ -122,7 +122,7 @@ char *seterr; /* Error message from
#include
#include
-struct timeval time0; /* Time at which the shell started */
+struct timespec time0; /* Time at which the shell started */
struct rusage ru0;
/*
Index: bin/csh/extern.h
===
RCS file: /cvs/src/bin/csh/extern.h,v
retrieving revision 1.25
diff -u -p -r1.25 extern.h
--- bin/csh/extern.h26 Dec 2015 13:48:38 - 1.25
+++ bin/csh/extern.h12 Jul 2017 04:15:04 -
@@ -272,7 +272,7 @@ void plist(struct varent *);
void donice(Char **, struct command *);
void dotime(Char **, struct command *);
void prusage(struct rusage *, struct rusage *,
- struct timeval *, struct timeval *);
+ struct timespec *, struct timespec *);
void ruadd(struct rusage *, struct rusage *);
void settimes(void);
void pcsecs(long);
Index: bin/csh/proc.c
===
RCS file: /cvs/src/bin/csh/proc.c,v
retrieving revision 1.30
diff -u -p -r1.30 proc.c
--- bin/csh/proc.c 26 Dec 2015 13:48:38 - 1.30
+++ bin/csh/proc.c 12 Jul 2017 04:15:04 -
@@ -108,7 +108,7 @@ found:
}
else {
if (pp->p_flags & (PTIME | PPTIME) || adrof(STRtime))
- (void) gettimeofday(&pp->p_etime, NULL);
+ (void) clock_gettime(CLOCK_MONOTONIC, &pp->p_etime);
pp->p_rusage = ru;
if (WIFSIGNALED(w)) {
@@ -494,7 +494,7 @@ palloc(int pid, struct command *t)
}
pp->p_next = proclist.p_next;
proclist.p_next = pp;
-(void) gettimeofday(&pp->p_btime, NULL);
+(void) clock_gettime(CLOCK_MONOTONIC, &pp->p_btime);
}
static void
@@ -799,8 +799,8 @@ prcomd:
static void
ptprint(struct process *tp)
{
-struct timeval tetime, diff;
-static struct timeval ztime;
+struct timespec tetime, diff;
+static struct timespec ztime;
struct rusage ru;
static struct rusage zru;
struct process *pp = tp;
@@ -809,8 +809,8 @@ ptprint(struct process *tp)
tetime = ztime;
do {
ruadd(&ru, &pp->p_rusage);
- timersub(&pp->p_etime, &pp->p_btime, &diff);
- if (timercmp(&diff, &tetime, >))
+ timespecsub(&pp->p_etime, &pp->p_btime, &diff);
+ if (timespeccmp(&diff, &tetime, >))
tetime = diff;
} while ((pp = pp->p_friends) != tp);
prusage(&zru, &ru, &tetime, &ztime);
Index: bin/csh/proc.h
===
RCS file: /cvs/src/bin/csh/proc.h,v
retrieving revision 1.3
diff -u -p -r1.3 proc.h
--- bin/csh/proc.h 2 Jun 2003 23:32:07 - 1.3
+++ bin/csh/proc.h 12 Jul 2017 04:15:04 -
@@ -50,8 +50,8 @@ struct process {
pid_t p_pid;
pid_t p_jobid; /* pid of job leader */
/* if a job is stopped/background p_jobid gives its pgrp */
-struct timeval p_btime;/* begin time */
-struct timeval p_etime;/* end time */
+struct timespec p_btime; /* begin time */
+struct timespec p_etime; /* end time */
struct rusage p_rusage;
Char *p_command; /* first PMAXLEN chars of command */
};
Index: bin/csh/time.c
===
RCS fil
