| Message ID | AANLkTikWVbdF113pgV-RvUb2yxLdmJt=Us-cZO_X608a@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 02/03/2011 04:45 AM, Janne Blomqvist wrote: > On Wed, Feb 2, 2011 at 21:23, Janne Blomqvist<blomqvist.janne@gmail.com> wrote: >> Hi, >> >> I reviewed the Gfortran time intrinsics, both standard and extensions, >> and AFAIK the only place where a monotonic clock fits the API, and >> where we can expose high resolution, is the SYSTEM_CLOCK intrinsic. >> >> Now, due to the PR 47571 fix where we now call clock_gettime() via a >> weakref, some of the original reason behind the gf_gettime() wrapper >> has been lost, as now we still need to include fallback code in case >> the weakref is not available. This needlessly bloats the code for >> date_and_time() which is unable to use either the extra precision and >> needs a realtime clock, not a monotonic clock. Additionally, in some >> not quite bleeding edge Linux kernels, gettimeofday() is a vsyscall >> and clock_gettime() isn't, leading to higher overhead for no benefit >> (the difference is something like 1e6 vs 1e7 calls/s on current hw, so >> it's exceedingly unlikely to actually matter in reality). The only >> fly in the ointment here is that POSIX 2008 has marked gettimeofday() >> as obsolescent, however I don't see this mattering in reality for the >> foreseeable future. >> >> The attached patch moves all the weakref trickery and clock_gettime() >> usage from time_1.h to system_clock.c, where the only benefactor also >> lives. > > Hi, > > updated patch attached. Compared to the previous one this might fix > the build regression reported on HP-UX which does support weak symbols > but apparently does barf on weak undefined references. This is done in > a slightly kludgy way, in that in acinclude.m4 we previously have set > GTHREAD_USE_WEAK to 0 for such targets, hence in the updated patch > GTHREAD_USE_WEAK is defined and set to 1 if it's not previously > present, and then we check for both SUPPORTS_WEAK and > GTHREAD_USE_WEAK. > > 2011-02-03 Janne Blomqvist<jb@gcc.gnu.org> > > PR libfortran/47571 > * intrinsics/time_1.h (GF_CLOCK_MONOTONIC): Move to system_clock.c. > (weak_gettime): Likewise. > (gf_gettime): Change API, move weak_gettime() usage to > system_clock.c > * intrinsics/system_clock.c (GTHREAD_USE_WEAK): Define. > (gf_gettime_mono): New function. > (system_clock_4): Use gf_gettime_mono(). > (system_clock_8): Likewise. > * intrinsics/date_and_time.c (date_and_time): Update gf_gettime() > usage. > > Regtested on x86_64-unknown-linux-gnu, Ok for trunk? > OK and thanks for patch. Jerry
On Sat, Feb 5, 2011 at 17:58, Jerry DeLisle <jvdelisle@frontier.com> wrote: > On 02/03/2011 04:45 AM, Janne Blomqvist wrote: >> >> On Wed, Feb 2, 2011 at 21:23, Janne Blomqvist<blomqvist.janne@gmail.com> >> wrote: >>> >>> Hi, >>> >>> I reviewed the Gfortran time intrinsics, both standard and extensions, >>> and AFAIK the only place where a monotonic clock fits the API, and >>> where we can expose high resolution, is the SYSTEM_CLOCK intrinsic. >>> >>> Now, due to the PR 47571 fix where we now call clock_gettime() via a >>> weakref, some of the original reason behind the gf_gettime() wrapper >>> has been lost, as now we still need to include fallback code in case >>> the weakref is not available. This needlessly bloats the code for >>> date_and_time() which is unable to use either the extra precision and >>> needs a realtime clock, not a monotonic clock. Additionally, in some >>> not quite bleeding edge Linux kernels, gettimeofday() is a vsyscall >>> and clock_gettime() isn't, leading to higher overhead for no benefit >>> (the difference is something like 1e6 vs 1e7 calls/s on current hw, so >>> it's exceedingly unlikely to actually matter in reality). The only >>> fly in the ointment here is that POSIX 2008 has marked gettimeofday() >>> as obsolescent, however I don't see this mattering in reality for the >>> foreseeable future. >>> >>> The attached patch moves all the weakref trickery and clock_gettime() >>> usage from time_1.h to system_clock.c, where the only benefactor also >>> lives. >> >> Hi, >> >> updated patch attached. Compared to the previous one this might fix >> the build regression reported on HP-UX which does support weak symbols >> but apparently does barf on weak undefined references. This is done in >> a slightly kludgy way, in that in acinclude.m4 we previously have set >> GTHREAD_USE_WEAK to 0 for such targets, hence in the updated patch >> GTHREAD_USE_WEAK is defined and set to 1 if it's not previously >> present, and then we check for both SUPPORTS_WEAK and >> GTHREAD_USE_WEAK. >> >> 2011-02-03 Janne Blomqvist<jb@gcc.gnu.org> >> >> PR libfortran/47571 >> * intrinsics/time_1.h (GF_CLOCK_MONOTONIC): Move to system_clock.c. >> (weak_gettime): Likewise. >> (gf_gettime): Change API, move weak_gettime() usage to >> system_clock.c >> * intrinsics/system_clock.c (GTHREAD_USE_WEAK): Define. >> (gf_gettime_mono): New function. >> (system_clock_4): Use gf_gettime_mono(). >> (system_clock_8): Likewise. >> * intrinsics/date_and_time.c (date_and_time): Update gf_gettime() >> usage. >> >> Regtested on x86_64-unknown-linux-gnu, Ok for trunk? >> > > OK and thanks for patch. > > Jerry > Thanks, Sending libgfortran/ChangeLog Sending libgfortran/intrinsics/date_and_time.c Sending libgfortran/intrinsics/system_clock.c Sending libgfortran/intrinsics/time_1.h Transmitting file data .... Committed revision 169852.
diff --git a/libgfortran/intrinsics/date_and_time.c b/libgfortran/intrinsics/date_and_time.c index 714df14..c58d114 100644 --- a/libgfortran/intrinsics/date_and_time.c +++ b/libgfortran/intrinsics/date_and_time.c @@ -162,11 +162,11 @@ date_and_time (char *__date, char *__time, char *__zone, struct tm local_time; struct tm UTC_time; - long nanosecs; + long usecs; - if (!gf_gettime(GF_CLOCK_REALTIME, <, &nanosecs)) + if (!gf_gettime (<, &usecs)) { - values[7] = nanosecs / 1000000; + values[7] = usecs / 1000; localtime_r (<, &local_time); gmtime_r (<, &UTC_time); diff --git a/libgfortran/intrinsics/system_clock.c b/libgfortran/intrinsics/system_clock.c index 3715562..3a44dd9 100644 --- a/libgfortran/intrinsics/system_clock.c +++ b/libgfortran/intrinsics/system_clock.c @@ -29,6 +29,75 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #include "time_1.h" +#ifdef HAVE_CLOCK_GETTIME +/* POSIX states that CLOCK_REALTIME must be present if clock_gettime + is available, others are optional. */ +#ifdef CLOCK_MONOTONIC +#define GF_CLOCK_MONOTONIC CLOCK_MONOTONIC +#else +#define GF_CLOCK_MONOTONIC CLOCK_REALTIME +#endif + +/* Weakref trickery for clock_gettime(). On Glibc, clock_gettime() + requires us to link in librt, which also pulls in libpthread. In + order to avoid this by default, only call clock_gettime() through a + weak reference. + + Some targets don't support weak undefined references; on these + GTHREAD_USE_WEAK is 0. So we need to define it to 1 on other + targets. */ +#ifndef GTHREAD_USE_WEAK +#define GTHREAD_USE_WEAK 1 +#endif + +#if SUPPORTS_WEAK && GTHREAD_USE_WEAK +static int weak_gettime (clockid_t, struct timespec *) + __attribute__((__weakref__("clock_gettime"))); +#else +static inline int weak_gettime (clockid_t clk_id, struct timespec *res) +{ + return clock_gettime (clk_id, res); +} +#endif +#endif + + +/* High resolution monotonic clock, falling back to the realtime clock + if the target does not support such a clock. + + Arguments: + secs - OUTPUT, seconds + nanosecs - OUTPUT, nanoseconds + + If the target supports a monotonic clock, the OUTPUT arguments + represent a monotonically incrementing clock starting from some + unspecified time in the past. + + If a monotonic clock is not available, falls back to the realtime + clock which is not monotonic. + + Return value: 0 for success, -1 for error. In case of error, errno + is set. +*/ +static inline int +gf_gettime_mono (time_t * secs, long * nanosecs) +{ + int err; +#ifdef HAVE_CLOCK_GETTIME + if (weak_gettime) + { + struct timespec ts; + err = weak_gettime (GF_CLOCK_MONOTONIC, &ts); + *secs = ts.tv_sec; + *nanosecs = ts.tv_nsec; + return err; + } +#endif + err = gf_gettime (secs, nanosecs); + *nanosecs *= 1000; + return err; +} + extern void system_clock_4 (GFC_INTEGER_4 *, GFC_INTEGER_4 *, GFC_INTEGER_4 *); export_proto(system_clock_4); @@ -56,7 +125,7 @@ system_clock_4(GFC_INTEGER_4 *count, GFC_INTEGER_4 *count_rate, if (sizeof (secs) < sizeof (GFC_INTEGER_4)) internal_error (NULL, "secs too small"); - if (gf_gettime (GF_CLOCK_MONOTONIC, &secs, &nanosecs) == 0) + if (gf_gettime_mono (&secs, &nanosecs) == 0) { GFC_UINTEGER_4 ucnt = (GFC_UINTEGER_4) secs * TCK; ucnt += (nanosecs + 500000000 / TCK) / (1000000000 / TCK); @@ -103,7 +172,7 @@ system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_8 *count_rate, if (sizeof (secs) < sizeof (GFC_INTEGER_4)) internal_error (NULL, "secs too small"); - if (gf_gettime (GF_CLOCK_MONOTONIC, &secs, &nanosecs) == 0) + if (gf_gettime_mono (&secs, &nanosecs) == 0) { GFC_UINTEGER_8 ucnt = (GFC_UINTEGER_8) secs * TCK; ucnt += (nanosecs + 500000000 / TCK) / (1000000000 / TCK); diff --git a/libgfortran/intrinsics/time_1.h b/libgfortran/intrinsics/time_1.h index 86e4331..073595a 100644 --- a/libgfortran/intrinsics/time_1.h +++ b/libgfortran/intrinsics/time_1.h @@ -175,87 +175,40 @@ gf_cputime (long *user_sec, long *user_usec, long *system_sec, long *system_usec #endif -/* POSIX states that CLOCK_REALTIME must be present if clock_gettime - is available, others are optional. */ -#ifdef CLOCK_REALTIME -#define GF_CLOCK_REALTIME CLOCK_REALTIME -#else -#define GF_CLOCK_REALTIME 0 -#endif - -#ifdef CLOCK_MONOTONIC -#define GF_CLOCK_MONOTONIC CLOCK_MONOTONIC -#else -#define GF_CLOCK_MONOTONIC GF_CLOCK_REALTIME -#endif - -/* Weakref trickery for clock_gettime(). On Glibc, clock_gettime() - requires us to link in librt, which also pulls in libpthread. In - order to avoid this by default, only call clock_gettime() through a - weak reference. */ -#ifdef HAVE_CLOCK_GETTIME -#ifdef SUPPORTS_WEAK -static int weak_gettime (clockid_t, struct timespec *) - __attribute__((__weakref__("clock_gettime"))); -#else -static inline int weak_gettime (clockid_t clk_id, struct timespec *res) -{ - return clock_gettime (clk_id, res); -} -#endif -#endif +/* Realtime clock with microsecond resolution, falling back to less + precise functions if the target does not support gettimeofday(). -/* Arguments: - clock_id - INPUT, must be either GF_CLOCK_REALTIME or GF_CLOCK_MONOTONIC + Arguments: secs - OUTPUT, seconds - nanosecs - OUTPUT, OPTIONAL, nanoseconds + usecs - OUTPUT, microseconds - If clock_id equals GF_CLOCK_REALTIME, the OUTPUT arguments shall be - the number of seconds and nanoseconds since the Epoch. If clock_id - equals GF_CLOCK_MONOTONIC, and if the target supports it, the - OUTPUT arguments represent a monotonically incrementing clock - starting from some unspecified time in the past. + The OUTPUT arguments shall represent the number of seconds and + nanoseconds since the Epoch. Return value: 0 for success, -1 for error. In case of error, errno is set. */ static inline int -gf_gettime (int clock_id __attribute__((unused)), time_t * secs, - long * nanosecs) +gf_gettime (time_t * secs, long * usecs) { -#ifdef HAVE_CLOCK_GETTIME - if (weak_gettime) - { - struct timespec ts; - int err; - err = weak_gettime (clock_id, &ts); - *secs = ts.tv_sec; - if (nanosecs) - *nanosecs = ts.tv_nsec; - return err; - } -#endif #ifdef HAVE_GETTIMEOFDAY struct timeval tv; int err; err = gettimeofday (&tv, NULL); *secs = tv.tv_sec; - if (nanosecs) - *nanosecs = tv.tv_usec * 1000; + *usecs = tv.tv_usec; return err; #elif HAVE_TIME time_t t, t2; t = time (&t2); *secs = t2; - if (nanosecs) - *nanosecs = 0; + *usecs = 0; if (t == ((time_t)-1)) return -1; return 0; #else *secs = 0; - if (nanosecs) - *nanosecs = 0; + *usecs = 0; errno = ENOSYS; return -1; #endif