[ccan] New module: rprof, runtime profiler/timers

[Christian Thaeter]

Am Mon, 29 Aug 2011 14:34:41 +0930
schrieb Rusty Russell <rusty at rustcorp.com.au>:

> On Fri, 26 Aug 2011 22:03:45 +0200, Christian Thaeter <ct at pipapo.org>
> wrote:
> > Some observations: It is a bit exceptional as I designed it not to
> code, I think.
> 
> > This possibly could
> > improved with asserts or runtime checks, but I strive to make the
> > timer function as deterministic AND small as possible. This makes
> > writing tests a bit superfluous.
> 
> Err, no it doesn't!
> 
> Even a very simple program which mallocs a struct rprof, calls
> rprof_init(), runs rprof_start() then rprof_stop(), then tests that
> rprof_sum() >= 0 will test that you initialize all the fields
> correctly (since it's run under valgrind).

ok, doing that

> 
> Other suggestions:
> 
> 1) Split the benchmark stuff into rprof_bench.h?

IMO this makes no sense, the benchmark stuff are just 2 macros.
Arguably the cpu-pinning might be splitted out since it's rather some
other kind of functionality, i can do that.

> 2) Use ccan/time?

Can't do, I'm intentionally using the POSIX Realtime timers, ccan/time
uses gettimeofday (which is marked deprecated by POSIX'08, nevertheless
I bet it will stay for our lifetime). Realtime timers have some
advantages, you can query the resolution of each timer which gives an
idea how accurate the timings are. The 'struct timespec' fractional
part counts in nanoseconds, so it can have a higher resolution than
struct timeval which goes only down to microseconds. You have quite
some more clocksources, ranging from wallclock (like gettimeofday) down
to per-thread cputime. And finally I decided to convert measured times
to double, albeit inexact this makes further computations (statistics)
much easier than handling timeval's or timespec's.

There would be a possibility to beef up ccan/time but this needs to be
carefully considered, just adding a
  struct timespec time_clock(void)

to return the thread's cputime is certainly not enough. Changing the
interface to use double is rather intrusive and breaks everything,
having conversions between timespec, timeval and double looks rather
ugly to me. Any ideas?

> 3) Something to avoid compiler elimination of code, such as an
>    enhancement to RPROF_BENCH* which make sure gcc thinks it needs
>    the result?

I don't think this is really needed and even doable from the macro. The
macro itself calls the rprof functions which are sequence points and
thus must not be eliminated (if they would be inline then things might
look differently), also these functions are not pure, gcc should know
that. The loop can not be unrolled (since the iterations are
determined by non-pure function calls)

That should be enough for the loop. For the user code I have no
idea how to do this but I suspect that the user knows how to rig this
and its his obligation anyways.

> 4) #if HAVE_FOR_LOOP_DECLARATION || RPROF_STDC_VERSION >= 199901L
>    is ugly.  Preferred is to include "config.h", maybe like:
> 
>         #if 1 /* If you have a config.h, eg in ccan */
>         #include "config.h"
>         #else
>         # if __STDC_VERSION__ >= 199901L
>         #  define HAVE_FOR_LOOP_DECLARATION 1
>         # endif
>         #endif

ok, will do

> 
>    Though hacking up a config.h is expected abilities for someone
> using ccan.
> 5) Your documentation style manages to be nonstandard *and* ugly,
>    but I'm sure you know that :)

I like it :), it generates pretty docs, even if its a bit raw currently.

> 
> > * How to disable valgrind for ccanlint testing?
> 
> (I answered this on IRC, but repeating here for posterity):
> 
> As a once-off:
> 
>    ccanlint -x run_tests_valgrind
> 
> To suppress it in general, use _info.  Either per-test:
> 
>  * Ccanlint:
>  *	// valgrind breaks on setsched().
>  *	tests_pass_valgrind test/run.c:FAIL
> 
> Or to add some args to the valgrind cmdline:
>         
>  * Ccanlint:
>  *	// valgrind breaks on setsched().
>  *	tests_pass_valgrind test/run.c:--args
> 
> Without the "testname:" prefix, it applies to all the tests.
> 
> > * This needs '-lrt' any way to put that in the metadata?
> 
> Yep! _info can tell what libraries to use:
> 	if (strcmp(argv[1], "libs") == 0) {
> 		printf("rt\n");
> 		return 0;
>         }	
> 
> It can depend on config.h #ifdefs, but I wouldn't worry about that
> until someone ports it and fails...
> 
> Thanks!
> Rusty.

	Thanks
		Christian