c++c++-chronontp

NTP timestamps using std::chrono


I'm trying to represent NTP timestamps (including the NTP epoch) in C++ using std::chrono. Therefore, I decided to use a 64-bit unsigned int (unsigned long long) for the ticks and divide it such that the lowest 28-bit represent the fraction of a second (accepting trunction of 4 bits in comparison to the original standard timestamps), the next 32-bit represent the seconds of an epoch and the highest 4-bit represent the epoch. This means that every tick takes 1 / (2^28 - 1) seconds.

I now have the following simple implementation:

#include <chrono>

/**
 * Implements a custom C++11 clock starting at 1 Jan 1900 UTC with a tick duration of 2^(-28) seconds.
 */
class NTPClock
{

public:

    static constexpr bool is_steady = false;
    static constexpr unsigned int era_bits = 4;                                     // epoch uses 4 bits
    static constexpr unsigned int fractional_bits = 32-era_bits;                    // fraction uses 28 bits
    static constexpr unsigned int seconds_bits = 32;                                // second uses 32 bits

    using duration = std::chrono::duration<unsigned long long, std::ratio<1, (1<<fractional_bits)-1>>;
    using rep = typename duration::rep;
    using period = typename  duration::period;
    using time_point = std::chrono::time_point<NTPClock>;

    /**
     * Return the current time of this. Note that the implementation is based on the assumption
     * that the system clock starts at 1 Jan 1970, which is not defined with C++11 but seems to be a
     * standard in most compilers.
     * 
     * @return The current time as represented by an NTP timestamp
     */
    static time_point now() noexcept
    {
        return time_point
        (
            std::chrono::duration_cast<duration>(std::chrono::system_clock::now().time_since_epoch())
                + std::chrono::duration_cast<duration>(std::chrono::hours(24*25567))   // 25567 days have passed between 1 Jan 1900 and 1 Jan 1970
        );
    };
}

Unfortunately, a simple test reveals this does not work as expected:

#include <chrono>
#include <iostream>

#include <catch2/catch.hpp>
#include "NTPClock.h"

using namespace std::chrono;

TEST_CASE("NTPClock_now")
{
    auto ntp_dur = NTPClock::now().time_since_epoch();
    auto sys_dur = system_clock::now().time_since_epoch();
    std::cout << duration_cast<hours>(ntp_dur) << std::endl;
    std::cout << ntp_dur << std::endl;
    std::cout << duration_cast<hours>(sys_dur) << std::endl;
    std::cout << sys_dur << std::endl;
    REQUIRE(duration_cast<hours>(ntp_dur)-duration_cast<hours>(sys_dur) == hours(24*25567));
}

Output:

613612h
592974797620267184[1/268435455]s
457599h
16473577714886015[1/10000000]s

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
PackageTest.exe is a Catch v2.11.1 host application.
Run with -? for options

-------------------------------------------------------------------------------
NTPClock_now
-------------------------------------------------------------------------------
D:\Repos\...\TestNTPClock.cpp(10)
...............................................................................

D:\Repos\...\TestNTPClock.cpp(18): FAILED:
  REQUIRE( duration_cast<hours>(ntp_dur)-duration_cast<hours>(sys_dur) == hours(24*25567) )
with expansion:
  156013h == 613608h

===============================================================================
test cases: 1 | 1 failed
assertions: 1 | 1 failed

I also removed the offset of 25567 days in NTPClock::now asserting equality without success. I'm not sure what is going wrong here. Can anybody help?


Solution

  • Your tick period: 1/268'435'455 is unfortunately both extremely fine and also doesn't lend itself to much of a reduced fraction when your desired conversions are used (i.e. between system_clock::duration and NTPClock::duration. This is leading to internal overflow of your unsigned long long NTPClock::rep.

    For example, on Windows the system_clock tick period is 1/10,000,000 seconds. The current value of now() is around 1.6 x 1016. To convert this to NTPClock::duration you have to compute 1.6 x 1016 times 53,687,091/2,000,000. The first step in that is the value times the numerator of the conversion factor which is about 8 x 1023, which overflows unsigned long long.

    There's a couple of ways to overcome this overflow, and both involve using at least an intermediate representation with a larger range. One could use a 128 bit integral type, but I don't believe that is available on Windows, except perhaps by a 3rd party library. long double is another option. This might look like:

    static time_point now() noexcept
    {
        using imd = std::chrono::duration<long double, period>;
        return time_point
        (
            std::chrono::duration_cast<duration>(imd{std::chrono::system_clock::now().time_since_epoch()
                + std::chrono::hours(24*25567)})
        );
    };
    

    That is, perform the offset shift with no conversion (system_clock::duration units), then convert that to the intermediate representation imd which has a long double rep, and the same period as NTPClock. This will use long double to compute 1.6 x 1016 times 53,687,091/2,000,000. Then finally duration_cast that to NTPClock::duration. This final duration_cast will end up doing nothing but casting long double to unsigned long long as the conversion factor is simply 1/1.

    Another way to accomplish the same thing is:

    static time_point now() noexcept
    {
        return time_point
        (
            std::chrono::duration_cast<duration>(std::chrono::system_clock::now().time_since_epoch()
                + std::chrono::hours(24*25567)*1.0L)
        );
    };
    

    This takes advantage of the fact that you can multiply any duration by 1, but with alternate units and the result will have a rep with the common_type of the two arguments, but otherwise have the same value. I.e. std::chrono::hours(24*25567)*1.0L is a long double-based hours. And that long double carries through the rest of the computation until the duration_cast brings it back to NTPClock::duration.

    This second way is simpler to write, but code reviewers may not understand the significance of the *1.0L, at least until it becomes a more common idiom.