Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: Add Instant and SystemTime to std::time #29894

Merged
merged 1 commit into from
Nov 20, 2015

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of RFC 1288 which adds two new unstable
types to the std::time module. The Instant type is used to represent
measurements of a monotonically increasing clock suitable for measuring time
withing a process for operations such as benchmarks or just the elapsed time to
do something. An Instant favors panicking when bugs are found as the bugs are
programmer errors rather than typical errors that can be encountered.

The SystemTime type is used to represent a system timestamp and is not
monotonic. Very few guarantees are provided about this measurement of the system
clock, but a fixed point in time (UNIX_EPOCH) is provided to learn about the
relative distance from this point for any particular time stamp.

This PR takes the same implementation strategy as the time crate on crates.io,
namely:

Platform Instant SystemTime
Windows QueryPerformanceCounter GetSystemTimeAsFileTime
OSX mach_absolute_time gettimeofday
Unix CLOCK_MONOTONIC CLOCK_REALTIME

These implementations can perhaps be refined over time, but they currently
satisfy the requirements of the Instant and SystemTime types while also
being portable across implementations and revisions of each platform.

cc #29866

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@bors
Copy link
Contributor

bors commented Nov 18, 2015

☔ The latest upstream changes (presumably #29083) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member Author

r? @brson

@rust-highfive rust-highfive assigned brson and unassigned aturon Nov 18, 2015
@sfackler
Copy link
Member

lgtm after a quick pass

@brson
Copy link
Contributor

brson commented Nov 19, 2015

The Debug implementations delegate to the platform-specific implementations, which have platform-specific representations. Seems like the abstract versions should display something consistent across platforms.

The implementations contain plattform-specific types, none of which look like the pub struct Instant { secs: u64, nanos: u32 } types in the RFC. Why is this ok?

This commit is an implementation of [RFC 1288][rfc] which adds two new unstable
types to the `std::time` module. The `Instant` type is used to represent
measurements of a monotonically increasing clock suitable for measuring time
withing a process for operations such as benchmarks or just the elapsed time to
do something. An `Instant` favors panicking when bugs are found as the bugs are
programmer errors rather than typical errors that can be encountered.

[rfc]: rust-lang/rfcs#1288

The `SystemTime` type is used to represent a system timestamp and is not
monotonic. Very few guarantees are provided about this measurement of the system
clock, but a fixed point in time (`UNIX_EPOCH`) is provided to learn about the
relative distance from this point for any particular time stamp.

This PR takes the same implementation strategy as the `time` crate on crates.io,
namely:

|  Platform  |  Instant                 |  SystemTime              |
|------------|--------------------------|--------------------------|
| Windows    | QueryPerformanceCounter  | GetSystemTimeAsFileTime  |
| OSX        | mach_absolute_time       | gettimeofday             |
| Unix       | CLOCK_MONOTONIC          | CLOCK_REALTIME           |

These implementations can perhaps be refined over time, but they currently
satisfy the requirements of the `Instant` and `SystemTime` types while also
being portable across implementations and revisions of each platform.
@alexcrichton
Copy link
Member Author

I personally prefer to retain the platform-specific representation for as long as possible so I opted to avoid the cross-platform representation via the secs/nanos repr, but this does have the downside of different Debug representations as well as requiring each type to ensure overflow logic is handled correctly. To me, however, keeping the representation clean and close to the platform seems worth it.

@alexcrichton
Copy link
Member Author

I'm also not too worried about cross-platform Debug representations, types like File already do some platform-specific-ness in terms of their output (although it's not as radical as this)

@brson
Copy link
Contributor

brson commented Nov 19, 2015

I have some concern that math on SystemTime will have different overflow behavior based on platform, but I think it's probably unlikely to bite anybody anytime soon, and if it does can probably be fixed as a bugfix without much fallout.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 19, 2015

📌 Commit c6eb852 has been approved by brson

@bors
Copy link
Contributor

bors commented Nov 19, 2015

⌛ Testing commit c6eb852 with merge b289892...

bors added a commit that referenced this pull request Nov 19, 2015
This commit is an implementation of [RFC 1288][rfc] which adds two new unstable
types to the `std::time` module. The `Instant` type is used to represent
measurements of a monotonically increasing clock suitable for measuring time
withing a process for operations such as benchmarks or just the elapsed time to
do something. An `Instant` favors panicking when bugs are found as the bugs are
programmer errors rather than typical errors that can be encountered.

[rfc]: rust-lang/rfcs#1288

The `SystemTime` type is used to represent a system timestamp and is not
monotonic. Very few guarantees are provided about this measurement of the system
clock, but a fixed point in time (`UNIX_EPOCH`) is provided to learn about the
relative distance from this point for any particular time stamp.

This PR takes the same implementation strategy as the `time` crate on crates.io,
namely:

|  Platform  |  Instant                 |  SystemTime              |
|------------|--------------------------|--------------------------|
| Windows    | QueryPerformanceCounter  | GetSystemTimeAsFileTime  |
| OSX        | mach_absolute_time       | gettimeofday             |
| Unix       | CLOCK_MONOTONIC          | CLOCK_REALTIME           |

These implementations can perhaps be refined over time, but they currently
satisfy the requirements of the `Instant` and `SystemTime` types while also
being portable across implementations and revisions of each platform.

cc #29866
@sfackler
Copy link
Member

@brson I was concerned about overflow behavior as well when making the libtime version of SteadyTime. From looking at boost/C++11's implementations, it seems like everyone assumes that overflow doesn't occur. Windows guarantees that overflow won't happen for at least a century or two of uptime, and I bet the same is roughly true in practice for OSX and Linux. Leaving time information in the native formats for as long as possible should help as well with overflow behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants