std: make Instant a signed Duration on all platforms - #158368
Conversation
|
r? @Darksonn rustbot has assigned @Darksonn. Use Why was this reviewer chosen?The reviewer was selected based on:
|
There was a problem hiding this comment.
This does more than required for the conversion – but the existing implementation was both incorrect (the TSC-based time passed 1000 times slower than real time due to a faulty conversion) and lossy. Since our UEFI code isn't free from issues anyway I took the liberty of adjusting this to what I see as a better algorithm. I can split this out if you'd like.
This comment has been minimized.
This comment has been minimized.
d8f7cbd to
c8d3117
Compare
This comment has been minimized.
This comment has been minimized.
c8d3117 to
a35d18f
Compare
|
So to state this explicitly, this makes |
a35d18f to
a399bb4
Compare
Yes, exactly. |
Instant in the same way on all platformsInstant a signed Duration on all platforms
|
@rfcbot merge libs |
|
@Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
Since this changes the windows implementation to do more work it would be good to have a benchmark how this affects throughput/latency of |
It doesn't, the Windows implementation already converts to seconds/nanoseconds on every |
|
@rustbot label -S-waiting-on-review +S-waiting-on-fcp |
|
☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
This fixes #156142 for all targets using a very principled solution: representing
Instantthe same way on all platforms.Currently, not all platforms have the same range and precision of
Instant: while all UNIX platforms (and Hermit) representInstantusing a pair consisting of an 64-bit signed second field and a non-negative nanosecond offset and thus are able to represent timepoints well before the system epoch, most other platforms use aDurationwith the result that operations likeInstant::now() - Duration::from(/* 100 years */)do not succeed. SOLID even representsInstantusing a microsecond counter, which means that nanosecond arithmetic is lossy.To improve the portability of time arithmetic, I propose using the same representation for
Instanton all platforms: a 64-bit signed second field and a non-negative nanosecond offset (i.e. a signedDuration). This means that:Instant::nowcan never fail due to an out-of-bounds timestamp from the platform.Note that this does not add any significant conversion cost since all platforms currently convert to some seconds-based unit anyway.
I've split this PR into two parts: the first makes the actual changes to the implementation, the second adjusts the documentation to remove any mentions of platform-specific representations.