Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upImprovements to the Time APIs #1288
Conversation
nrc
added
the
T-libs
label
Sep 21, 2015
nrc
assigned
wycats
Sep 21, 2015
nagisa
reviewed
Sep 21, 2015
|
|
||
| It provides convenience methods for constructing Durations from larger units | ||
| of time (minutes, hours, days), but gives them names like | ||
| `Duration.from_standard_hour`. A standard hour is 3600 seconds, regardless of |
This comment has been minimized.
This comment has been minimized.
nagisa
reviewed
Sep 21, 2015
| Asking for an amount of time elapsed from a given `SystemTime` is a very common | ||
| operation that is guaranteed to produce a positive `Duration`. Asking for the | ||
| difference between an earlier and a later `SystemTime` also produces a positive | ||
| `Duration` when used correctly. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nagisa
reviewed
Sep 21, 2015
| optimizes for the broadly useful positive `Duration`. | ||
|
|
||
| ```rs | ||
| impl SystemTime { |
This comment has been minimized.
This comment has been minimized.
nagisa
reviewed
Sep 21, 2015
| /// Panics if `earlier` is later than &self. | ||
| /// Because SystemTime is monotonic, the only ime that `earlier` should be | ||
| /// a later time is a bug in your code. | ||
| pub fn duration_from_earlier(&self, earlier: SystemTime) -> SystemTime; |
This comment has been minimized.
This comment has been minimized.
nagisa
reviewed
Sep 21, 2015
| pub fn elapsed(&self) -> Duration; | ||
| } | ||
|
|
||
| impl Add<Duration> for SystemTime { |
This comment has been minimized.
This comment has been minimized.
nagisa
reviewed
Sep 21, 2015
| type Output = SystemTime; | ||
| } | ||
|
|
||
| impl Sub<Duration> for SystemTime { |
This comment has been minimized.
This comment has been minimized.
nagisa
Sep 21, 2015
Contributor
ProcessTime
While I have nothing against it; just noting inconsistency, in earlier paragraphs you allow
subtract a
ProcessTimefrom a laterProcessTime, producing aDuration
and not this.
This comment has been minimized.
This comment has been minimized.
wycats
Sep 21, 2015
Author
Contributor
That is referring to duration_from_earlier, not the Sub operator
This comment has been minimized.
This comment has been minimized.
nagisa
reviewed
Sep 21, 2015
| pub fn duration_from_earlier(&self, earlier: SystemTime) -> Result<Duration, ()>; | ||
|
|
||
| /// Returns an `Err` if &self is later than the current system time. | ||
| pub fn elapsed(&self) -> Result<Duration, ()>; |
This comment has been minimized.
This comment has been minimized.
nagisa
Sep 21, 2015
Contributor
Err = () is not sufficient, because system time APIs (on UNIX) might error-out for their own reasons.
This comment has been minimized.
This comment has been minimized.
|
@nagisa thanks for the early feedback. I'll correct these typos ASAP! |
sfackler
reviewed
Sep 21, 2015
| the timeframe of the process it was created in. | ||
|
|
||
| > In this context, monotonic means that a timestamp created later in real-world | ||
| > time will always be larger than a timestamp created earlier in real-world |
This comment has been minimized.
This comment has been minimized.
sfackler
Sep 21, 2015
Member
Pedantically speaking, "no smaller than", since you might pull the time twice within the same tick.
sfackler
reviewed
Sep 21, 2015
| impl SystemTime { | ||
| /// Panics if `earlier` is later than &self. | ||
| /// Because SystemTime is monotonic, the only ime that `earlier` should be | ||
| /// a later time is a bug in your code. |
This comment has been minimized.
This comment has been minimized.
sfackler
Sep 21, 2015
Member
Are we going to guarantee that the time will "never" overflow? Windows guarantees that overflow won't happen for at least ~200 years of uptime, but I'm not sure about OSX and Linux.
leodasvacas
reviewed
Sep 21, 2015
| # Alternatives | ||
|
|
||
| One alternative design would be to attempt to have a single unified time | ||
| type. The rationale for now doing so is explained under Drawbacks. |
This comment has been minimized.
This comment has been minimized.
jmesmon
reviewed
Sep 21, 2015
| types and operations. | ||
|
|
||
| First of all, with the exception of instants created in the rutime of a single | ||
| process, instants are not monotonic. A simple example of this is that if a |
This comment has been minimized.
This comment has been minimized.
jmesmon
Sep 21, 2015
Ok, so we're talking about the proposed API here? Because both Posix and windows provide functions to get a monotonic system time (clock_gettime + CLOCK_MONOTONIC or GetTickCount64()/GetTickCount()).
Might want to clearly delineate where we're talking about actual system restrictions vs restrictions that just occur in the Rust api.
The simple example happens to be true because (generally) one cannot specify the clock used to generate a file timestamp.
Edit: my confusion here might be due to thinking ProcessTime refers to the amount of time a particular process has run (ie: cpu time) rather than just being monotonic time. If that is the intention, it might make sense to avoid the proposed naming.
Edit2: to illustrate why this is confusing (assuming ProcessTime is meant to mean "some type of monotonic time"), consider what posix says about (CLOCK_MONOTONIC)[http://pubs.opengroup.org/onlinepubs/9699919799/]:
This clock represents the monotonic clock for the system
And also consdier what windows says about (GetTickCount64())[https://msdn.microsoft.com/en-us/library/windows/desktop/ms724411(v=vs.85).aspx]:
milliseconds that have elapsed since the system was started.
No mention of it being per-process there.
Does rust intend to have it's monotonic time be per-process? If so, why?
If not (and I haven't completely misinterpreted things), I strongly recommend a different name.
This comment has been minimized.
This comment has been minimized.
eternaleye
Sep 23, 2015
There's also that there are multiple kinds of monotonic time on some systems.
In particular, while CLOCK_MONOTONIC generates comparable and orderable instants, it does not generate comparable durations because it is still affected by NTP's ability to alter the length of a second. Linux at least has CLOCK_MONOTONIC_RAW (whose ticks are of constant length), but it makes no guarantee of the relationship between its units and human-meaningful ones.
In addition, suspend/hibernate is not counted for CLOCK_MONOTONIC, so durations from it are even more suspect - but Linux has the (underdocumented) CLOCK_BOOTTIME, which does count that... however, it isn't clear what its guarantees on the length of a tick are (consistent vs. meaningful).
Yes, this is in fact worth curling up in a corner over. No, it likely doesn't get better.
EDIT:
Personally, what I wish the platform provided was something akin to three calls:
booted() -> TAI nanoseconds
offset(kind) -> Consistent-length ticks since booted()
scale() -> Ratio ns/ticks
where 'kind' is one of "run" or "boot", respectively meaning actual runtime (CLOCK_MONOTONIC_RAW) or external time since boot (a hypothetical CLOCK_BOOTTIME_RAW) respectively.
The workflow would then be:
// take measurements
let start = offset(kind);
do_thing();
let end = offset(kind);
// Get a comparable duration
let duration = end - start;
// Get _coherent_ view of what that means in real units
let ratio = scale();
println!("do_thing() took {} nanoseconds", duration * ratio);
// Get the beginning of time, to format as human-readable instants
let base = booted();
println!(
"It began at {:date} and finished at {:date}",
base + start * ratio; base + end * ratio
);One can at least approximate those on Linux systems in some cases:
booted():CLOCK_TAI - CLOCK_MONOTONICoffset(run):CLOCK_MONOTONIC_RAWscale(): possiblyCLOCK_MONOTONIC / CLOCK_MONOTONIC_RAW, but that relies on NTP's second-slewing averaging out to a second, which is likely optimistic when it also uses it to correct small initial errors. It also doesn't work forBOOTTIMEbecause there is no_RAWvariant. Eugh. :/
jmesmon
reviewed
Sep 21, 2015
| program creates two files sequentially, it cannot assume that the creation time | ||
| of the second file is later than the creation time of the first file. | ||
|
|
||
| This is because NTP (the network time protocol) can arbitrarily change the |
This comment has been minimized.
This comment has been minimized.
jmesmon
Sep 21, 2015
Might be a good idea to clarify to:
NTP (the network time protocol) and any process/user with sufficient privileges
jmesmon
reviewed
Sep 21, 2015
| unlikely programmers are to consider "12:00:60" as a valid time. | ||
|
|
||
| Certain kinds of seemingly simple operations may not make sense in | ||
| all cases. For example, adding "1 year" to February 29, 2012 would produce |
This comment has been minimized.
This comment has been minimized.
jmesmon
Sep 21, 2015
I'd replace would with could or might, unless we intend to have the Rust api generate invalid dates. (The subtext here is that there are other ways to resolve these than generating invalid dates)
jmesmon
reviewed
Sep 21, 2015
| * ask for an amount of time elapsed since a `ProcessTime`, producing a `Duration` | ||
|
|
||
| Asking for an amount of time elapsed from a given `ProcessTime` is a very common | ||
| operation that is guaranteed to produce a positive `Duration`. Asking for the |
This comment has been minimized.
This comment has been minimized.
jmesmon
Sep 21, 2015
Should this really be positive? Or just non-negative? (ie: might help to clarify if a elapsed time can be 0)
jmesmon
reviewed
Sep 21, 2015
|
|
||
| The most important caveat of `SystemTime` is that it is **not monotonic**. This | ||
| means that you can save a file to the file system, then save another file to | ||
| the file system, **and the second file has an `mtime` earlier than the second**. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Where should By the way, should |
This comment has been minimized.
This comment has been minimized.
|
I like having the distinction between monotonic and system time. |
This comment has been minimized.
This comment has been minimized.
|
cc @lifthrasiir |
This comment has been minimized.
This comment has been minimized.
asb
commented
Sep 22, 2015
|
Might it be worth contrasting this API with that offered by e.g. Jodatime, On 22 September 2015 at 00:05, Aaron Turon notifications@github.com wrote:
|
This comment has been minimized.
This comment has been minimized.
yazaddaruvala
commented
Sep 23, 2015
|
Hopefully not just The article is clearly really old, but might help this design: |
This comment has been minimized.
This comment has been minimized.
yazaddaruvala
commented
Sep 23, 2015
|
@wycats from the wording in the current draft it seems like the biggest problem isn't actually monotonicity of time but the creation of negative durations. If there are no other issues with time lacking monotonicity, one alternative I haven't seen discussed but would like to mention is to instead split up
Apart from naming conventions, I kinda like the consistency it has with numbers. I don't think it even changes all that much:
Pros: Cons: Let me know what downsides I'm not aware of or am currently missing. Thanks. P.S. After writing this up, if it helps, I did find: https://internals.rust-lang.org/t/unsigned-version-of-duration/893 |
This comment has been minimized.
This comment has been minimized.
Veedrac
commented
Sep 23, 2015
|
I have nothing against a positive-first API, but one should not ignore negative durations either. That said, I can't really think of a case where you'd want signed durations and not want to branch off their direction. So my suggestion is that the |
This comment has been minimized.
This comment has been minimized.
|
I don't think libstd should contain a calendar - these contain lots of subjective edge cases. However, we probably want some form of absolute time for cross-system timestamps (we would need to choose between classic UNIX, UTS, or TAI). Leap-second-aware format conversions would be nice. |
This comment has been minimized.
This comment has been minimized.
eternaleye
commented
Sep 24, 2015
|
Personally, I strongly favor TAI for cross-system times, for the following reasons:
|
This comment has been minimized.
This comment has been minimized.
cristicbz
commented
Sep 25, 2015
|
Two minor suggestions:
pub struct StdHours(pub u64);
pub struct StdMinutes(pub u64);
// ...
impl From<StdHours> for Duration {
// ...
}
// ...Then conversions become as simple as: What do you think? Edit: One worry might be panicking |
This comment has been minimized.
This comment has been minimized.
|
Replying to @cristicbz (more later on other comments): I don't love MonotonicTime since I expect this to be the primary API developers use for things like benchmarks, and an obscure name might put them off. The point about being allowed to use BOOT_TIME is good, though. Can you think of a friendly name that communicates monotonicity? TimeThatObeysTheLawsOfPhysics :p I don't object to conversions at first glance and agree that they read nicely, provide nice ergonomics for mixing, and make it easy to avoid mistakes. I'll let others weigh in on the panicking issue before I form a strong opinion though :) @aturon? |
This comment has been minimized.
This comment has been minimized.
eternaleye
commented
Sep 25, 2015
|
I think that C++'s
For As far as use cases go, what should each be used for?
Notably not covered:
|
This comment has been minimized.
This comment has been minimized.
That’s not a valid solution, is it? Regarding @vadimcn’s link, virtualised environments are a pretty interesting use case/test which might uncover a whole can of worms with every platform we support (and possibly various different processors that might or might not have “interesting” clocks, if any), since they might not allow virtualised kernel to access hardware directly and other small details. |
This comment has been minimized.
This comment has been minimized.
eternaleye
commented
Nov 4, 2015
|
I was hoping it was like the architectural constraints/guarantees for |
This comment has been minimized.
This comment has been minimized.
|
@eternaleye: I don't recall seeing the 2-3s number anywhere. I would guess that non-monotonicity is on the order of as few ticks (as returned by |
This comment has been minimized.
This comment has been minimized.
eternaleye
commented
Nov 4, 2015
It's a comment by the original reporter in response to a question about the magnitude.
|
This comment has been minimized.
This comment has been minimized.
|
@eternaleye: he probably meant 'ticks', not 'milliseconds'. |
This comment has been minimized.
This comment has been minimized.
eternaleye
commented
Nov 4, 2015
|
ElapsedMilliseconds is consistently used - both in the original post's code example and the comments. |
This comment has been minimized.
This comment has been minimized.
|
Yeah - having a |
This comment has been minimized.
This comment has been minimized.
kryptan
commented
Nov 4, 2015
|
Seems that no one mentioned but according to wikipedia the desicion whether to keep or abolish leap seconds will be made at World Radiocommunication Conference which is scheduled for November 2–27 (i.e. it is happening now). Indeed, their page states:
Lets hope that they will do the right thing. |
alexcrichton
referenced this pull request
Nov 5, 2015
Closed
Tracking issue for Duration::span #27799
boxofrox
reviewed
Nov 8, 2015
| `Duration` when used correctly. | ||
|
|
||
| This design does not assume that negative `Duration`s are never useful, but | ||
| rather than the most common uses of `Duration` do not have a meaningful |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The libs team discussed this RFC during triage last week and after some final clarifications we've decided to merge. The last pieces were indicating that For now we've decided to take the conservative route of not trying to exhaustively specify all aspects of the two types here, but as usual there will be a period of Thanks again for all the discussion everyone! |
alexcrichton
merged commit d8133de
into
rust-lang:master
Nov 16, 2015
This comment has been minimized.
This comment has been minimized.
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 17, 2015
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 17, 2015
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 17, 2015
alexcrichton
referenced this pull request
Nov 17, 2015
Merged
std: Add Instant and SystemTime to std::time #29894
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 17, 2015
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 17, 2015
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 17, 2015
This comment has been minimized.
This comment has been minimized.
|
I've now got an implementation of this RFC, so if you'd like to take a look over it please feel free to leave a comment there! |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 18, 2015
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Nov 19, 2015
bors
added a commit
to rust-lang/rust
that referenced
this pull request
Nov 19, 2015
This comment has been minimized.
This comment has been minimized.
tioover
commented
Jan 31, 2016
|
Why not implement For example use std::time::Instant;
#[derive(PartialEq, Eq, Hash, PartialOrd, Ord)]
struct ID {
random: u32,
instant: Instant,
} |
This comment has been minimized.
This comment has been minimized.
|
@tioover Duration now implements Hash: rust-lang/rust#30818. The lack of an implementation for Instant is just an oversight - feel free to make a PR adding the impl. Since this RFC has been merged, rust-lang/rust#29866 is the new place for discussion, by the way. |
wycats commentedSep 21, 2015
Rendered