Handle `time` conversions from constants.
miladHakimi opened this issue · 1 comments
We use 2 different clocks, TimestampClock and DurationClock. They are aliases for std::chrono::system_clock and std::chrono::steady_clock. I created a wrapper around these clocks to make them easier to work with. E.g: #134,
I had 2 assumptions in mind when writing these classes.
First, the DurationClock and TimestampClock have the same precision. This caused a bug on a system in which this didn't hold. I fixed it in #145 by casting the duration from one clock to the other.
My second assumption was that the clocks always are in the nanoseconds precision. The second bug happens in this code:
class Timestamp {
public:
static Timestamp FromNanoseconds(int64_t nanos) {
return Timestamp(
TimestampClock::time_point(TimestampClock::duration(nanos)));
}
Timestamp(TimestampClock::time_point timestamp) : timestamp_{timestamp} {}
int64_t ToNanoseconds() const {
return std::chrono::nanoseconds(timestamp_.time_since_epoch()).count();
}
private:
TimestampClock::time_point timestamp_;
};
The Timestamp class is a wrapper around a TimestampClock. It has a FromNanoseconds() factory method that receives an input and stores it in the underlying TimestampClock::time_point variable assuming the TimestampClock has a nanoseconds precision. That is why this line of test fails in a system that has a different clock.
Knowing more about time_point, duration, and clock helps solve this problem.
chrono::time_point: Is a point in time. It stores a duration type value indicating the time since the clock's epoch.
chrono::duration: Holds the number of ticks, where the tick period is a compile-time rational fraction representing the time in seconds from one tick to the next.
clock: Is a bundle containing a time_point and a duration. The origin of the clock's time_point is referred to as the clock's epoch.
To create a time_point from a number, we should create a TimestampClock::duration. The given number is used to set the ticks of the duration variable of the time_point. Since we want this number to represent a duration in nanoseconds, the period of the duration must be in nanoseconds. The problem with the current FromNanoseconds is that the TimestampClock can vary from one system to another. Hence, the result will not always be a duration in nanoseconds precision.
To fix this issue, we could write something like this:
std::chrono::nanoseconds duration_ns{nanos};To create a time_point from the duration_ns, we must convert the ticks in duration_ns into the ticks in a TimestampClock::duration. To do that, we have to explicitly cast the duration; otherwise, the implicit cast leads to a compile time error when the precision of the TimestampClock::duration is less than nanoseconds. The explicit cast tells the compiler we are aware of the possible data loss.
TimestampClock::duration casted_dur = std::chrono::duration_cast<TimestampClock::duration>(duration_ns);Now that we have a duration in TimestampClock::duration, we can create a TimestampClock::time_point from it.
TimestampClock::time_point timestamp(casted_dur);A sample code for different casting scenarios: https://godbolt.org/z/5cEEoGdon
references: