2

From a comment to this answer: https://stackoverflow.com/a/459704/2812104

I tried to make a macro function that takes a function as argument and times it using clock(). However I keep getting an error message saying that start is undeclared and I can't figure it out since I do declare it earlier in the macro

(this is all on the same line in my code in case it makes a difference; I'm breaking it up in two lines here for readability):

#define timing(a): clock_t start = clock(); a; clock_t stop = clock(); 
printf("Elapsed: %f seconds\n", (double)(stop - start) / CLOCKS_PER_SEC);
Community
  • 1
  • 1
jeremy radcliff
  • 1,049
  • 3
  • 11
  • 27
  • 1
    You don't want the colon in `timing(a):` (or, if you keep it, you have to use `label timing(function(1, 2, 3));` so you have a label for the colon). And it matters whether it is on one logical line — you should continue the line with a backslash so it is clearer what you're doing. I'm not convinced that's a good macro to be using. – Jonathan Leffler Jan 30 '17 at 05:55
  • 2
    In addition, if you plan to use the macro in more than one place in your program, enclose it in `{}`, or else you will have multiple instances of `stop` and `start`. – DYZ Jan 30 '17 at 06:04
  • @JonathanLeffler, thank you it worked, but why do you say it might not be a good macro to be using? – jeremy radcliff Jan 30 '17 at 06:06
  • @DYZ, do you mean { timing(function()) } every time I use it, or enclose the definition of the macro itself in curly braces? Also if you don't mind, what do you mean by multiple instances? I mostly think of instances in the context of OOP, which is probably not what you mean, right? – jeremy radcliff Jan 30 '17 at 06:08
  • Enclose the macro definition in `{}`. Otherwise, you may have `clock_t start; .... clock_t start; ....`, which is illegal. With the braces, each variable is declared within its own block: `{clock_t start; ....} {clock_t start; ....}` – DYZ Jan 30 '17 at 06:14
  • My discomfort arises from what you're going to use as the argument to the macro. A simple function call is OK. Making it a loop or more complex is theoretically feasible, but not necessarily a good idea (so `timing(for (int i = 0; i < 1000000; i++) some_func(i, i * 10, i + 10);)` starts to feel uncomfortable). More protracted code in the argument gets less and less comfortable. Your decision — if you fix it as suggested, it is usable, but be cautious. (I'd probably aim at higher-precision timers than `clock()` — and therefore I prefer a functional interface to deal with system idiosyncrasies.) – Jonathan Leffler Jan 30 '17 at 06:22
  • @JonathanLeffler Indeed. It seems all of this reinvention-of-the-wheel could be avoided by *just using a profiler*... – autistic Jan 30 '17 at 06:24
  • @JonathanLeffler, thank you for explaining. What would you recommend as a higher-precision timer? – jeremy radcliff Jan 30 '17 at 06:26
  • 1
    It depends on the o/s — which is why it needs to be hidden. If it's available, `clock_gettime()` with `CLOCK_MONOTONIC` gives you nanosecond resolution (but not necessarily nanosecond precision). If that's not available, maybe `gettimeofday()` is available (microsecond resolution), or `ftime()` (millisecond resolution) or `clock()` as you're using (resolution system-dependent), or `times()` (resolution system-dependent), or `GetTickCount()`, or … hey, it's fun being fussy. Using `clock()` is mostly portable, but the resolution you get isn't necessarily very good. – Jonathan Leffler Jan 30 '17 at 06:33

1 Answers1

5

The immediate problem is the colon after timing(a) — that wants a label to precede it, but you can't use labels on variable definitions, so you need to lose the colon:

#define timing(a) \
    clock_t start = clock(); \
    a; \
    clock_t stop = clock(); \
    printf("Elapsed: %f seconds\n", (double)(stop - start) / CLOCKS_PER_SEC);

You should probably also avoid polluting the function's name space. The classic way to do that is a do { … } while (0) loop:

#define timing(a) \
    do { \
        clock_t start = clock(); \
        a; \
        clock_t stop = clock(); \
        printf("Elapsed: %f seconds\n", (double)(stop - start) / CLOCKS_PER_SEC); \
    } while (0)

This creates a statement block that can be used legitimately even in an unprotected if block:

if (do_it_this_way)
    timing(some_func(1, 37, 91));
else
    timing(some_func(2, 41, 87));

If you use just { … } around the macro body, this generates an error (because the semicolon after 91 means the else doesn't belong to any if — the expansion is if (do_it_this_way) { … }; else …).

I'm not particularly keen on this technique. I use a logically opaque type, Clock, with support functions such as:

extern void  clk_init(Clock *clk);
extern void  clk_start(Clock *clk);
extern void  clk_stop(Clock *clk);
extern char *clk_elapsed_ms(Clock *clk, char *buffer, size_t buflen);
extern char *clk_elapsed_us(Clock *clk, char *buffer, size_t buflen);
extern char *clk_elapsed_ns(Clock *clk, char *buffer, size_t buflen);

The calling code creates and initializes a Clock, and can then start the clock before the loop, stop the clock after it, and at its leisure analyze the elapsed time. You can find this code on Github https://github.com/jleffler/soq/tree/master/src/libsoq in timer.c and timer.h. You can find examples of it being used if you search on SO with the term 'user:15168 clk_elapsed_us' (e.g. How to wrap around a range?).

Community
  • 1
  • 1
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278