Is this timer efficient?

I found myself writing code to run at a specified frequency more than once, so I decided to make this simple timer. My main concern is that this is not the most efficient way of doing this since with SDL I would just ask for the time in ms and sleep in ms.

I'm storing the current time in timer_update() to avoid the possibility of it becoming greater than the final time and generating a huge pause since it's unsigned.

Usage is pretty simple, timer_init() is called once before the loop and timer_update() is called once at the end of every iteration.

timer.h

#ifndef TIMER_H
#define TIMER_H

typedef struct {
    double step;
    double tfinal;
} Timer;

void timer_init(Timer *timer, double hertz);
void timer_update(Timer *timer);

#endif

timer.c

#include "timer.h"

#define _POSIX_C_SOURCE 199309L
#include 
#include 
#include 

static unsigned long get_ms(void)
{
    struct timespec buffer;
    unsigned long temp;

    if(clock_gettime(CLOCK_MONOTONIC, &buffer) != 0){
        perror("clock_gettime");
        return 0;
    }

    temp = buffer.tv_sec * 1000;
    temp += buffer.tv_nsec / 1000000;
    return temp;
}

static struct timespec ms_to_timespec(unsigned long ms)
{
    struct timespec temp = {ms / 1000, (ms % 1000) * 1000000};
    return temp;
}

void timer_init(Timer *timer, double hertz)
{
    timer->step = 1000 / hertz;
    timer->tfinal = get_ms() + timer->step;
}

void timer_update(Timer *timer)
{
    unsigned long now = get_ms();
    struct timespec required_sleep, rem;

    if(now tfinal){
        required_sleep = ms_to_timespec(timer->tfinal - now);
        timer->tfinal += timer->step;

        try_to_sleep:
        if(nanosleep(&required_sleep, &rem) == -1 && errno == EINTR)
            if(nanosleep(&rem, &required_sleep) == -1 && errno == EINTR)
                goto try_to_sleep;
    }

    else
        timer->tfinal += timer->step;
}

Is this timer efficient?

-
Yes, but there are two optimizations that I can see. Get rid of the goto
and use a do-while loop. I have noticed a slight performance
increase when it has been written this way in other programs.

This increase will become non-existent however if you have compiler
optimization settings enabled. If they are enabled, they compile
into the exact same assembly code. However, I also find a loop
more readable than a goto, so that is another reason to switch
over.

-
There other optimization I could see is by cleaning up your if-else a tiny bit (untested). This should speed up your code a tiny bit (again, if optimizations on your compiler are enabled, your results will vary).

if(now tfinal)
{
    required_sleep = ms_to_timespec(timer->tfinal - now);

    // loop implementation
}
timer->tfinal += timer->step;

You may wonder why this will speed up your code. The more branches you have, the more susceptible you are to a branch prediction failure and those failures can be expensive when it comes to efficiency.

A few other notes:

-
Use a designated initializer to initialize a structure (C99).

struct timespec temp = 
{ 
    .step = ms / 1000, 
    .tfinal = (ms % 1000) * 1000000 
};

-
Group your #includes and #defines together in a more organized fashion.

#include   // group library headers
#include 
#include 
#include "timer.h" // group user defined headers

#define _POSIX_C_SOURCE 199309L  // group defines

-
Your != 0 comparision check in your if conditional could be removed since it is redundant (the if will always execute if the conditional isn't equal to 0). However, this may increase readability to you, so it would be okay to keep it.

StackExchange Code Review Q#47688, answer score: 10