> On 13 Jul 2017, at 11:16 am, Scott Cheloha <[email protected]> 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 -0000 1.28 > +++ bin/csh/csh.h 12 Jul 2017 04:15:04 -0000 > @@ -122,7 +122,7 @@ char *seterr; /* Error message from > #include <sys/time.h> > #include <sys/resource.h> > > -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 -0000 1.25 > +++ bin/csh/extern.h 12 Jul 2017 04:15:04 -0000 > @@ -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 -0000 1.30 > +++ bin/csh/proc.c 12 Jul 2017 04:15:04 -0000 > @@ -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 -0000 1.3 > +++ bin/csh/proc.h 12 Jul 2017 04:15:04 -0000 > @@ -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 file: /cvs/src/bin/csh/time.c,v > retrieving revision 1.14 > diff -u -p -r1.14 time.c > --- bin/csh/time.c 22 Aug 2013 04:43:40 -0000 1.14 > +++ bin/csh/time.c 12 Jul 2017 04:15:04 -0000 > @@ -46,7 +46,7 @@ settimes(void) > { > struct rusage ruch; > > - (void) gettimeofday(&time0, NULL); > + (void) clock_gettime(CLOCK_MONOTONIC, &time0); > (void) getrusage(RUSAGE_SELF, &ru0); > (void) getrusage(RUSAGE_CHILDREN, &ruch); > ruadd(&ru0, &ruch); > @@ -60,13 +60,13 @@ void > /*ARGSUSED*/ > dotime(Char **v, struct command *t) > { > - struct timeval timedol; > + struct timespec timedol; > struct rusage ru1, ruch; > > (void) getrusage(RUSAGE_SELF, &ru1); > (void) getrusage(RUSAGE_CHILDREN, &ruch); > ruadd(&ru1, &ruch); > - (void) gettimeofday(&timedol, NULL); > + (void) clock_gettime(CLOCK_MONOTONIC, &timedol); > prusage(&ru0, &ru1, &timedol, &time0); > } > > @@ -112,8 +112,8 @@ ruadd(struct rusage *ru, struct rusage * > } > > void > -prusage(struct rusage *r0, struct rusage *r1, struct timeval *e, > - struct timeval *b) > +prusage(struct rusage *r0, struct rusage *r1, struct timespec *e, > + struct timespec *b) > { > time_t t = > (r1->ru_utime.tv_sec - r0->ru_utime.tv_sec) * 100 + > @@ -125,7 +125,7 @@ prusage(struct rusage *r0, struct rusage > struct varent *vp = adrof(STRtime); > > int ms = > - (e->tv_sec - b->tv_sec) * 100 + (e->tv_usec - b->tv_usec) / 10000; > + (e->tv_sec - b->tv_sec) * 100 + (e->tv_nsec - b->tv_nsec) / 10000000; > > cp = "%Uu %Ss %E %P %X+%Dk %I+%Oio %Fpf+%Ww"; > > Index: bin/ksh/c_sh.c > =================================================================== > RCS file: /cvs/src/bin/ksh/c_sh.c,v > retrieving revision 1.59 > diff -u -p -r1.59 c_sh.c > --- bin/ksh/c_sh.c 4 Mar 2016 15:11:06 -0000 1.59 > +++ bin/ksh/c_sh.c 12 Jul 2017 04:18:27 -0000 > @@ -18,7 +18,8 @@ > > #include "sh.h" > > -static void p_time(struct shf *, int, struct timeval *, int, char *, char *); > +static void p_tv(struct shf *, int, struct timeval *, int, char *, char *); > +static void p_ts(struct shf *, int, struct timespec *, int, char *, char *); > > /* :, false and true */ > int > @@ -670,7 +671,7 @@ c_unset(char **wp) > } > > static void > -p_time(struct shf *shf, int posix, struct timeval *tv, int width, char > *prefix, > +p_tv(struct shf *shf, int posix, struct timeval *tv, int width, char *prefix, > char *suffix) > { > if (posix) > @@ -683,18 +684,34 @@ p_time(struct shf *shf, int posix, struc > tv->tv_usec / 10000, suffix); > } > > +static void > +p_ts(struct shf *shf, int posix, struct timespec *ts, int width, char > *prefix, > + char *suffix) > +{ > + if (posix) > + shf_fprintf(shf, "%s%*lld.%02ld%s", prefix ? prefix : "", > + width, (long long)ts->tv_sec, ts->tv_nsec / 10000000, > + suffix); > + else > + shf_fprintf(shf, "%s%*lldm%02lld.%02lds%s", prefix ? prefix : > "", > + width, (long long)ts->tv_sec / 60, > + (long long)ts->tv_sec % 60, > + ts->tv_nsec / 10000000, suffix); > +} > + > + > int > c_times(char **wp) > { > struct rusage usage; > > (void) getrusage(RUSAGE_SELF, &usage); > - p_time(shl_stdout, 0, &usage.ru_utime, 0, NULL, " "); > - p_time(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n"); > + p_tv(shl_stdout, 0, &usage.ru_utime, 0, NULL, " "); > + p_tv(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n"); > > (void) getrusage(RUSAGE_CHILDREN, &usage); > - p_time(shl_stdout, 0, &usage.ru_utime, 0, NULL, " "); > - p_time(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n"); > + p_tv(shl_stdout, 0, &usage.ru_utime, 0, NULL, " "); > + p_tv(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n"); > > return 0; > } > @@ -710,11 +727,12 @@ timex(struct op *t, int f, volatile int > #define TF_POSIX BIT(2) /* report in posix format */ > int rv = 0; > struct rusage ru0, ru1, cru0, cru1; > - struct timeval usrtime, systime, tv0, tv1; > + struct timeval usrtime, systime; > + struct timespec ts0, ts1, ts2; > int tf = 0; > extern struct timeval j_usrtime, j_systime; /* computed by j_wait */ > > - gettimeofday(&tv0, NULL); > + clock_gettime(CLOCK_MONOTONIC, &ts0); > getrusage(RUSAGE_SELF, &ru0); > getrusage(RUSAGE_CHILDREN, &cru0); > if (t->left) { > @@ -731,7 +749,7 @@ timex(struct op *t, int f, volatile int > rv = execute(t->left, f | XTIME, xerrok); > if (t->left->type == TCOM) > tf |= t->left->str[0]; > - gettimeofday(&tv1, NULL); > + clock_gettime(CLOCK_MONOTONIC, &ts1); > getrusage(RUSAGE_SELF, &ru1); > getrusage(RUSAGE_CHILDREN, &cru1); > } else > @@ -749,20 +767,20 @@ timex(struct op *t, int f, volatile int > } > > if (!(tf & TF_NOREAL)) { > - timersub(&tv1, &tv0, &tv1); > + timespecsub(&ts1, &ts0, &ts2); > if (tf & TF_POSIX) > - p_time(shl_out, 1, &tv1, 5, "real ", "\n"); > + p_ts(shl_out, 1, &ts2, 5, "real ", "\n"); > else > - p_time(shl_out, 0, &tv1, 5, NULL, " real "); > + p_ts(shl_out, 0, &ts2, 5, NULL, " real "); > } > if (tf & TF_POSIX) > - p_time(shl_out, 1, &usrtime, 5, "user ", "\n"); > + p_tv(shl_out, 1, &usrtime, 5, "user ", "\n"); > else > - p_time(shl_out, 0, &usrtime, 5, NULL, " user "); > + p_tv(shl_out, 0, &usrtime, 5, NULL, " user "); > if (tf & TF_POSIX) > - p_time(shl_out, 1, &systime, 5, "sys ", "\n"); > + p_tv(shl_out, 1, &systime, 5, "sys ", "\n"); > else > - p_time(shl_out, 0, &systime, 5, NULL, " system\n"); > + p_tv(shl_out, 0, &systime, 5, NULL, " system\n"); > shf_flush(shl_out); > > return rv; > 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 -0000 1.21 > +++ usr.bin/time/time.c 12 Jul 2017 04:19:40 -0000 > @@ -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/10000); > + (long long)during.tv_sec, after.tv_nsec/10000000); > fprintf(stderr, "user %9lld.%02ld\n", > (long long)ru.ru_utime.tv_sec, > ru.ru_utime.tv_usec/10000); > fprintf(stderr, "sys %9lld.%02ld\n", > (long long)ru.ru_stime.tv_sec, > ru.ru_stime.tv_usec/10000); > } else { > - > fprintf(stderr, "%9lld.%02ld real ", > - (long long)after.tv_sec, after.tv_usec/10000); > + (long long)during.tv_sec, after.tv_nsec/10000000); > fprintf(stderr, "%9lld.%02ld user ", > (long long)ru.ru_utime.tv_sec, > ru.ru_utime.tv_usec/10000); > fprintf(stderr, "%9lld.%02ld sys\n",
