This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 31 Aug 2015 17:07, Maria Guseva wrote: > The attached patch is a draft implementation of generic timing: via > clock_gettime, vdso or system call, where available and via gettimeofday > function call otherwise. > > In order to eliminate performance impact in existing usages of HP_TIMING_NOW > Âthe HP_TIMING_AVAIL is still set to 0. The generic timing is enabled only > in DL_DEBUG_STATISTICS cases where performance is not critical. > However I suggest it may be wrong and we can use at least vdso clock_gettime > as real HP_TIMING. And the rest probably shouldn't be even considered as > high-precision timers and should be moved from hp-timing header to some new > one. i tend to agree -- vdso+clock_gettime support should be sufficient for HP timing > -#ifndef HP_TIMING_NONAVAIL > +#if HP_TIMING_AVAIL i wouldn't mind renaming this as a sep patch. i dislike negative names. > +#if HP_TIMING_AVAIL > if (HP_SMALL_TIMING_AVAIL) > HP_TIMING_NOW (_dl_cpuclock_offset); > +#endif we should avoid CPP here imo. if !HP_TIMING_AVAIL, then shouldn't it already be !HP_SMALL_TIMING_AVAIL ? so you don't need the #if here. > +/* Macros to define timing in case of DL_DEBUG_STATISTICS set */ your comment style needs fixing in many places. should be (note the end): /* Macros to define timing in case of DL_DEBUG_STATISTICS set. */ > +# if HP_TIMING_AVAIL || HP_GENERIC_TIMING_AVAIL > +#define DL_STATISTICS_TIMING_NOW(tval) \ > +if (__glibc_unlikely (HP_SMALL_TIMING_AVAIL && \ > + (GLRO(dl_debug_mask) & DL_DEBUG_STATISTICS))) \ > + HP_TIMING_NOW (tval) these macros are dangerous as written. they should be wrapped in a: do { ... } while (0) > +#define HP_TIMING_NOW(var) \ > +do \ > + { \ > + struct timespec __t; \ > + INTERNAL_SYSCALL_DECL (__err); \ > + __INTERNAL_SYSCALL (clock_gettime, __err, 2, CLOCK_MONOTONIC, &(__t));\ don't need the paren around the __t > +#define HP_TIMING_NOW(var) \ > +do \ > + { \ > + struct timeval __t; \ > + (void) __gettimeofday (&__t, NULL) \ why do you need the (void) ? also, this fails to compile -- there's a missing ; at the end. -mike
Attachment:
signature.asc
Description: Digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |