Helper utilities for easier benchmarking in C

Question

I have this tiny library for measuring the execution time in milliseconds and returning the duration plus the result:

execres.h

/******************
* File: execres.h *
******************/

#ifndef EXECRES_H
#define EXECRES_H

#include <sys/time.h>

typedef struct {
    unsigned long long  millis;
    void*               result;
} ExecResult, *PExecResult;

PExecResult execTime(void* (*code)(void*), void* param);

#endif  /* EXECRES_H */

execres.c

/******************
* File: execres.c *
******************/

#include "execres.h"
#include <stdlib.h>

PExecResult execTime(void* (*code)(void*), void* param)
{
    PExecResult result;
    struct timeval tva;
    struct timeval tvb;

    if (!code)
    {
        return NULL;
    }

    result = malloc(sizeof(ExecResult));

    if (!result)
    {
        return NULL;
    }

    gettimeofday(&tva, NULL);
    result->result = code(param);
    gettimeofday(&tvb, NULL);

    result->millis = (unsigned long long)
                     (tvb.tv_sec * 1000 + tvb.tv_usec / 1000 -
                      tva.tv_sec * 1000 - tva.tv_usec / 1000);
    return result;
}

main.c

/***************
* File: main.c *
***************/

#include <stdint.h>
#include <stdio.h>
#include "execres.h"

static uint64_t Fibonacci(uint64_t n)
{
    switch (n)
    {
        case 0:
            return 0;

        case 1:
            return 1;

        default:
            return Fibonacci(n - 1) + Fibonacci(n - 2);
    }
}

int main()
{
    PExecResult result;

    puts("Begin computation.");

    result = execTime((void* (*)(void*)) Fibonacci, (void*) 40);

    printf("Fibonacci took %u milliseconds and returned %llu.\n",
           (unsigned) result->millis,
           (uint64_t) result->result);

    return 0;
}

Answer 1

Nice bit of code! Here are a few suggestions:

• Initialize your variables when you declare them. This helps shorten the code a bit and reduce possible bugs.

• You need documenting comments within your code. Especially since you are considering this as a library.

• I would remove the NULL check on code in your execTime method, and document that a valid function pointer should be passed in. I would expect the code to break if I tried to time NULL, not NULL to be returned back. The standard library does this same thing with a few of their functions, like strlen().

• Good job checking the return from malloc().

• I would use the C11 standard function timespec_get() for your timing code. It's a C standard (important!) function that has nanosecond precision.

• If you want a better Fibonacci algorithm, here is an answer I wrote up a while ago on it.

• Good job using puts() in place of printf() when you can.

• You don't have to return 0 at the end of main(). The C standard knows how frequently this is used, and lets us omit it.

  C99 & C11 §5.1.2.2(3)

  ...reaching the } that terminates the main() function returns a value of 0.

Answer 2

Your code is very nice. Some extra comments (adding to those of syb0rg):

• I prefer not to typedef pointer types. So I'd use ExecResult with an explicit pointer, rather than PExecResult.

• Contradicting myself a little, function pointers are much more understandable if you typedef them! So:

  typedef void* (*Exec_fn)(void*);
  ExecResult* execTime(Exec_fn code, void* param);
  ...
  
  result = execTime((Exec_fn) Fibonacci, (void*) 40);
  

• I'd extract the milliseconds calculation to an inline function:

  static inline unsigned long long
  milliseconds(struct timeval* t)
  {
      return (unsigned long long)(t->tv_sec * 1000 + t->tv_usec / 1000);
  }
  

• execres.h is unnecessarily exporting sys/time.h to all users. sys/time.h is used only in execres.c and should be there not in the header.

Answer 3

result = malloc(sizeof(ExecResult));

Any particular reason for allocating an ExecResult dynamically? It is such a tiny struct, so you could just return by value. Allocating it dynamically makes the code more complex and adds the overhead of having to free the retuned pointer on the call site (which you didn't in your example, BTW).

execTime((void* (*)(void*)) Fibonacci, (void*) 40);

Forcing the cast to your function pointer type is quite unsafe. We are assuming that the parameter of the passed function is always the size of a void* (true for uint64_t only on 64bits systems!) and returns something at least as big as a void*. But what if the function expected a char instead? Or worse, what if by mistake I pass in a function that returns nothing?

Unfortunately, there's no better way in C to safely pass abstract functions as parameters, so rather than defining the interface as you did, I would go with a less invasive approach.

MyTimeVal t0 = beginMeasurement();

// Measured code here
uint64_t fib = Fibonacci(40);

MyTimeVal t1 = endMeasurement();

MyTimeVal timeTaken = measureTimeTaken(t0, t1);

printf("Fibonacci took %u milliseconds and returned %llu.\n",
       timeTaken.milliseconds, fib);

You'd also gain in flexibility with a simpler interface, as it won't limit yourself to only measuring functions taking one argument and returning a single value.