Expand the std::time module

[alexcrichton]

Tracking issue for rust-lang/rfcs#1288

[alexcrichton]

One question raised during review is what representation these types should have on each platform. Currently they all have the native representation (e.g. libc::timeval, LARGE_INTEGER, etc) as this keeps lossless information as long as possible.

As a consequence, however, issues like #29970 come up where the types have the property that basic algebra does not work on them. For example b + 1ns == b may be true (along with other various properties). The reason for this is that the resolution of the native representation varies among platforms, meaning that small increments aren't actually representable.

I think that having a (u64, u32) representation would fix this, but it is a divergence from the platform types and now if a SystemTime is ever handed back to the standard library it'd have to do more conversions. May be worth it though?

[sgrif]

Is there intended to be a public constructor for any of these, or make Timespec public as well? This API as is doesn't provide any way to represent a time that isn't relative to now. It'd be great if there was a std library type that Diesel could return for timestamp database columns.

[SimonSapin]

It’s a bit convoluted, but you can construct a SystemTime with UNIX_EPOCH + Duration::from_secs(unix_timestamp_in_seconds), for example.

[sgrif]

Does SystemTime make sense as the type to return from a database column? It seems to imply things which are incorrect. Instant seems more appropriate.

[alexcrichton]

Yeah as @SimonSapin mentioned construction is intended to not be possible for the opaque Instant type and done via the epoch anchor for SystemTime. I'd probably suggest using SystemTime for database interactions (are those stored as seconds-since-epoch?) or perhaps using an external library which also tracks extra information like a time zone.

[sgrif]

They're stored as microseconds since 2000, so there would potentially be 2037 problems when converting.

[alexcrichton]

Ah ok, in that case (especially if there's no extra time zone data or anything like that) I'd just create something like:

let microseconds_since_2000 = db_value;
let t2000 = UNIX_EPOCH + thirty_years;
let duration_since_2000 = Duration::new(...);
let date = t2000 + duration_since_2000;

[sgrif]

I added support for this to Diesel in diesel-rs/diesel@dbcc1d2. A note on the API from that -- The lack of negative durations makes constructing arbitrary times really painful, though I understand the reasoning that went into that. However, using Result for time comparison felt odd to me. It makes sense if you only expect monotonic time for use with try!, but it feels odd to use otherwise. Perhaps it would make sense to add another method duration_since_time or similar name, which returns an enum of either Before or After?

[alexcrichton]

Very interesting! It's true that the APIs weren't necessarily designed initially with ergonomics as the primary feature, but rather correctness in terms of not omitting various oddities here and there. It's an interesting point though that duration_from_earlier and how it's more tailored towards timing operations rather than synthesizing new timestamps. There may be some slight tweaks that make it a little more amenable to this kind of operation.

[SimonSapin]

Duration being non-negative make sense (for the reasons given in the RFC), but I’m starting to think we should also have a type that represents a "signed" duration.

[aturon]

cc @wycats

[wycats]

My original design did indeed have a Signed Duration, but @aturon wanted to
see more concrete justification.

@aturon how do you feel now?

On Mon, Dec 7, 2015, 6:37 AM Aaron Turon notifications@github.com wrote:

cc @wycats https://github.com/wycats

—
Reply to this email directly or view it on GitHub
#29866 (comment).

[SimonSapin]

In short, the justification is: if the way to convert SystemTime to/from something the outside world can understand is to do Duration arithmetic with an epoch SystemTime constant, you’re gonna want to deal with (possibly) negative durations.

[aturon]

I'm not opposed to a sign duration type, and agree with @SimonSapin that we'll still have achieved the basic goal by making the "default" duration nonnegative.

Perhaps we can usefully prototype this out of tree.

[SimonSapin]

I don’t really like the name SignedDuration, but I don’t have another suggestion.

Although it would probably have to duplicate much of Duration’s own code, making that new type a struct that uses i64 internally instead of u64 is probably more natural than enum { PositiveOrZero(Duration), Negative(Duration) } which is roughly the return type of SystemTime::duration_from_earlier

[sfackler]

I experimented with converting r2d2 over to using Instant rather than libtime, and it went pretty smoothly: sfackler/r2d2@6b1f1dc.

One note I do have is that for whatever reason, I found Instant::duration_from_earlier hard to intuitively reason about with respect to which Instant should go where. Not totally sure if another name would be better though.

[aturon]

@sfackler Would duration_starting_from be better?

[sfackler]

Might be, yeah.

[Stebalien]

Instants are always guaranteed to be greater than any previously measured instant when created...

Greater than or equal to. There is no uniqueness guarantee.

[danburkert]

I'd like to re-raise the issue of renaming Instance that was brought up in the RFC discussion. My vote would be to rename Instance to MonotonicTime or MonoTime. My biggest concern with the Instance name is that it's not clear how it relates to SystemTime. In reality they are both 'times', or instances in time, but the names do not make that clear. Additionally, the C++ chrono API names them system_clock and steady_clock, it would be nice to be in line with that.

Another possibility is to rename Instance to SteadyTime, but in practice it's difficult to guarantee steady time. The C++ standard specifies the steady time guarantee, but in practice the implementations fall short. If it's determined in the future that a steady clock is possible, then SteadyTime could be added as an additional type.

[alexcrichton]

@danburkert note that the name is Instant rather than Instance, but I'm not sure if that affects what you're thinking much?

[danburkert]

Sorry about that. But yes, I have the same issues with Instant as I do with Instance.

[alexcrichton]

I'm also somewhat sympathetic to naming, specifically if we end up having a couple of clocks we'd probably want them all consistently named. Right now the names SystemTime and Instant don't share much similarity, and a pattern of FooTime seems reasonable-ish to start out at least.

[wycats]

@alexcrichton @danburkert I'm pretty uneasy with MonoTime or MonotonicTime, because we really want people to reach for this time first (say, when doing benchmarking), and MonoTime just feels more niche and less friendly than SystemTime.

Instead of trying to find a way to make the two types appear to have parity, I'd rather find a name for SystemTime that helps to indicate the caveats, and use Time for Instant.

[sgrif]

I agree with @wycats here. The name Time implies that it's monotonic. Time going backwards is the exception, not the rule (assuming the laws of physics as we know them hold. Dealing with computers which are traveling faster than the speed of light probably requires a separate RFC)

[danburkert]

Time is certainly a better name than Instant. That being said, I want to continue making the case for MonoTime.

I'm pretty uneasy with MonoTime or MonotonicTime, because we really want people to reach for this time first (say, when doing benchmarking), and MonoTime just feels more niche and less friendly than SystemTime.

This can go both ways. Ultimately you can't save all parties from their own ignorance by anointing one clock or another. It's better to put the two clock types on equal footing, and leave the education up to the documentation.

Instead of trying to find a way to make the two types appear to have parity

The two types do have parity. They are both clocks which provide measurements. Neither is perfect, neither is strictly superior, and in any given situation it's likely that only one of them can appropriately be used.

The name Time implies that it's monotonic.

It also implies other properties: steadiness, not jumping forwards, that measurements can be compared across hosts, etc. None of the properties are guaranteed or provided by MonoTime.

[danburkert]

One more thought. A monotonic clock is not sufficient for benchmarking, the clock should also be steady. As a trivial example, a logical clock is monotonic but would be inappropriate for benchmarking. Perhaps the type should be SteadyClock, with the guarantee that it is as steady as the best clock on the platform.

[alexcrichton]

although std::time::Time suffers from a stuttering problem, I'd be slightly more comfortable with the name over Instant indeed!

[sgrif]

Just re-export it from std/lib.rs!

[jsgf]

I've been using time2 in one of my projects, and I've been getting panics from "overflow when subtracting Durations". Unfortunately I haven't been able to repro this myself, but one of the users of my code has been getting it sporadically. I haven't been able to tell which subtraction could be causing them (they all look fine), and I've yet to get a backtrace back.

I think its a bit of an ergonomic issue for subtraction to panic; if we want to disallow negative Durations, then it seems to me that subtraction should return a Result or Option.

[alexcrichton]

The libs team discussed this during triage yesterday and we reached a few conclusions:

• We decided to stick with the name Instant over the alternatives of MonoTime or just Time. As @wycats pointed out the term MonoTime seemed a bit niche, and we felt that the name Time was a bit "too sweet" and also may convey the wrong perception of a wall clock time, for example. Additionally, we felt that Instant was actually pretty representative of what it was, an instant in time.
• We'll add Sub to Instant as it basically already supports it and is somewhat more readable than the method version.
• The duration_from_earlier method will be renamed to duration_since

[alexcrichton]

Ah, and the other conclusion was to also stabilize everything, which is probably worth mentioning!