threading: use std::chrono for timestamps

[theuni]

Obviously not for 0.14.

Though boost's chrono hides the racy gmtime() issue nicely, it's really just using gmtime_r internally. This switches to the same thing, but without the boost indirection.

PRing this separately from other boost removals because it's not a 1:1 replacement.

[fanquake]

Failed on both Windows builds with:

utiltime.cpp: In function ‘const tm gmtime_int(time_t)’:
utiltime.cpp:31:25: error: cannot convert ‘time_t* {aka long int*}’ to ‘tm*’ for argument ‘1’ to ‘errno_t gmtime_s(tm*, const time_t*)’
     gmtime_s(&time, &out);

[theuni]

Of course windows has reversed arguments. Why wouldn't it?

Pushed the reversal.

[laanwj]

Needing something like this makes me wonder if we're not better off just using the C time functions, which guarantee being based on the UNIX epoch.

Especially as we're already probing for gmtime_int anyway. This seems double.

Do we need something that is only offered by std::chrono? (or needs much less code with std::chrono)

[theuni]

@laanwj Best I can tell from google and reading the c spec, it's not guaranteed to be based on the UNIX epoch either :(. Of course, I've never seen an implementation that isn't, as that would likely break years worth of assumptions. And based on that, I would assume that c++ implementations will be using the underlying c implementation.

So my logic was: if we can run a quick sanity check here to be sure that c/c++/unix epoch are all aligned, then we can use c apis and std::chrono interchangeably throughout the codebase without worry.

As for why chrono here, it was to avoid gettimeofday portability issues. If you'd prefer to work around those instead, no problem.

You're right though that DateTimeStrFormat is kinda clunky. I only used c++ there to avoid c string size guessing. I'm also happy to switch that if you'd prefer.

[laanwj]

Best I can tell from google and reading the c spec, it's not guaranteed to be based on the UNIX epoch either :(

Which ones? The man page of "gmtime" tells me the following:

       The  ctime(),  gmtime()  and localtime() functions all take an argument of data type time_t, which represents calendar time.  When interpreted as an absolute time value, it represents the
       number of seconds elapsed since the Epoch, 1970-01-01 00:00:00 +0000 (UTC).

Same for gettimeofday:

       and gives the number of seconds and microseconds since the Epoch (see time(2)).  The tz argument is a struct timezone:

If using std::chrono makes anything less hassle I'm fine with using it. It just seems like a lot of code necessary to work around issues (and that's just to get started using it!), but you're right that gettimeofday and strftime have their own issues.

[theuni]

@laanwj Posix uses a value since the Unix epoch, but time_t is implementation-defined according to the c spec. But it really doesn't matter, realistically it'll be the Unix epoch everywhere we run. Sorry for the distraction.

I only brought it up because the c++ system_clock's epoch is technically implementation-defined as well, I was just making the case that it's probably just as safe to assume as with c.

[zstardust]

There's a few places in Bitcoin Core where non monotonic time stamps can cause the node to act strangely, for example if NTP tweaks the time backwards between a p2p PING and PONG, the int64 for node latency can overflow. Without much review, does this change any of the behavior in that respect? chrono::steady_clock assures monotone time where required.

[theuni]

@zstardust There is (should be) no behavioral change here. steady_clock would indeed make sense for some of those cases.

[laanwj]

Agreed, using monotonic clock makes sense where only differences in time are important, when there is no need to print the time (as it will have an arbitrary starting offset). That's orthogonal to the changes here, though.

[TheBlueMatt]

I think we need to think more about how we use time everywhere, see eg the issues turned up in #9606 - do we want our own types for time to enforce that we dont compare mock and non-mock time? Do we want multiple types of mock times? etc.

[theuni]

@TheBlueMatt This is intended to just be a replacement of what we have now. The behavioral changes can come next.

I have an impl of a clock/time_point that I've made that is explicitly non-convertable from one type to another. Happy to PR that for discussion as well. I expect that it would replace our current timestamp usage, but I figured it'd be best to get rid of boost first, since the boost condvars use boost::chrono.

[jtimon]

Concept ACK. With C++ std or with C time functions. Seems like removing boost's dependency here is better irrespective of next steps.
Needs rebase.

[laanwj]

Why the \ at the end of the line?

[laanwj]

nit: bracing style: we decided to always use braces unless the if and the statement can be on one line: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md

[laanwj]

Needs rebase/nit fix.

[laanwj]

Is this still in your plans @theuni?

[theuni]

Thanks for the reminder, I'll circle back to this.

[sipa]

What is the status here? Going to make it for 0.15?

[theuni]

I'll go ahead and rebase/clean this up now, but it's not a priority for 0.15.