8
\$\begingroup\$

I've noticed some discrepancies in timings in our system, and I'm trying to narrow down what could be causing them.

I'm reviewing out time abstraction, and as far as I can determine it's fine.

Am I missing anything, and is it portable (besides being constrained to POSIX)?

typedef struct timespec Time;
static inline Time getTime() {
    static struct timespec time;
    clock_gettime(CLOCK_MONOTONIC, &time);
    return time;
}
static inline __constant Time timeAdd(Time oldTime, Time time) {
    if (time.tv_nsec + oldTime.tv_nsec >= 1E9)
        return (Time){
            tv_sec: time.tv_sec + oldTime.tv_sec + 1,
            tv_nsec: time.tv_nsec + oldTime.tv_nsec - 1E9
        };
    else
        return (Time){
            tv_sec: time.tv_sec + oldTime.tv_sec,
            tv_nsec: time.tv_nsec + oldTime.tv_nsec
        };
}
static inline __constant Time timeDiff(Time oldTime, Time time) {
    if (time.tv_nsec < oldTime.tv_nsec)
        return (Time){
            tv_sec: time.tv_sec - 1 - oldTime.tv_sec,
            tv_nsec: 1E9 + time.tv_nsec - oldTime.tv_nsec
        };
    else
        return (Time){
            tv_sec: time.tv_sec - oldTime.tv_sec,
            tv_nsec: time.tv_nsec - oldTime.tv_nsec
        };
}
static __always_inline __constant double timeSeconds(Time time) {
    return time.tv_sec + time.tv_nsec/1E9;
}
static inline double timeSince(Time oldTime) {
    struct timespec time;
    clock_gettime(CLOCK_MONOTONIC, &time);
    return timeSeconds(timeDiff(oldTime, time));
}
\$\endgroup\$

3 Answers 3

Reset to default
11
\$\begingroup\$

I can see nothing wrong, but I think it is not optimal. My compiler warns me about floating point conversions of those '1E9' constants. If I rewrite your timeAdd as follows, the code is easier to read and the generated code is significantly smaller:

#define BILLION 1000000000
Time timeAdd(Time t1, Time t2)
{
    long sec = t2.tv_sec + t1.tv_sec;
    long nsec = t2.tv_nsec + t1.tv_nsec;
    if (nsec >= BILLION) {
        nsec -= BILLION;
        sec++;
    }
    return (Time){ .tv_sec = sec, .tv_nsec = nsec };
}

The same goes for the other functions, I think.

\$\endgroup\$
4
  • 2
    \$\begingroup\$ If you're using integer BILLION then the timeSeconds expression should include a cast to double, i.e. time.tv_sec + (double)time.tv_nsec/BILLION; \$\endgroup\$ Commented Jan 27, 2014 at 20:58
  • \$\begingroup\$ I haven't seen you answer any questions in a while. I sure hope we haven't seen the last of William Morris. \$\endgroup\$ Commented Feb 27, 2014 at 1:20
  • 1
    \$\begingroup\$ @syb0rg not intentionally, but I only really review C and there are some great reviewers (yourself included) who seem always to beat me to it recently :-) \$\endgroup\$ Commented Feb 27, 2014 at 13:37
  • \$\begingroup\$ @WilliamMorris That's usually because I hang out in our chat room, and there is a constantly updating feed of new questions so I can see when they are posted. :P \$\endgroup\$ Commented Feb 27, 2014 at 15:17
3
\$\begingroup\$

Just be careful with your double timeSeconds(Time time) function, since a double does not have enough bits to fully express a large number of seconds and a small number of fractional seconds. For example, using a timestamp in the year of 2014, resolution for fractions of a second starts breaking down somewhere around 10us.

\$\endgroup\$
1
\$\begingroup\$

You may also want an implementation for timeCmp in order to complete your library:

// Return 1 if a > b, -1 if a < b, 0 if a == b
static inline __constant int timeCmp(Time a, Time b) {
    if (a.tv_sec != b.tv_sec) {
        if (a.tv_sec > b.tv_sec)
            return 1;
        else 
            return -1;
    } else {
        if (a.tv_nsec > b.tv_nsec)
            return 1;
        else if (a.tv_nsec < b.tv_nsec)
            return -1;
        else
            return 0;
    }
}
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.