I just discovered std::time::Duration has no implementation of std::ops::Rem.
Is there some nuanced reasons this is the case? Seems like low-hanging fruit to improve the std library.
Why should this be implemented?
e.g.
How would this look?
Duration::ZERO % Duration::ZERO PANIC
Duration::from_seconds(10) % Duration::from_seconds(3) = Duration::from_seconds(1)
This doesn't feel right. I would prefer integer like semantics where division/remainder by 0 results in a panic.
Reminder that whenever you propose an addition, you should say why you think it should exist.
The fact that it could exist is not sufficient.
This seems reasonable, though I agree with SkiFire that % Duration::ZERO should behave like % 0_u32 does.
Fair point, and agreed.
As a standard library construct, everything therein should have a high standard of usefulness, which includes eagerly implementing common traits for high operability. Rem, in my humble opinion, is a basic and common operation. Therefore I see Duration lacking basic operability.
To circumnavigate this oversight, one has to convert each duration to nanos, modulo the operands, and reconstruct a duration from the remaining nanos. This is rather poor ergonomics.
What about Duration::{pow, sqrt} too? I actually needed to use these recently to calculate the standard deviation of some durations. Despite me needing to use them, these feel too far away from the intended purpose of Duration and I think it's better that I had to fallback to converting to a f64 and back. It's hard to say exactly where the line should be drawn on the APIs that make sense for a Duration.
pow and sqrt cannot easily be written in a dimensionally correct way. A Duration represents an amount of time; square a Duration, and you get an amount of "time^2", which would not be a Duration, so there would need to be a new type for it, which clearly isn't worth it.
On the other hand, if I understand division-with-rounding right, the remainder when dividing a Duration by either a Duration or a dimensionless u32 is properly a Duration.
However, we don't currently have Duration: Div<Duration>, and it does seem odd to have Duration: Rem<Duration>, as this proposal suggests, without Duration: Div<Duration>.
Interesting prior art: .Net's TimeSpan (which is basically a signed Duration) turns out to have multiplication and division by both (EDIT: this was misleading, see @elidupree's comment below instead), but not remainder by either, per https://docs.microsoft.com/en-us/dotnet/api/system.timespan?view=net-6.0#operators.TimeSpan and f64
(I do agree, from a math and dimensional analysis perspective, that duration remainder is perfectly reasonable, though.)
Nitpick: It doesn't have TimeSpan * TimeSpan (which would be dimensionally incorrect), but does have
TimeSpan * f64 -> TimeSpanf64 * TimeSpan -> TimeSpanTimeSpan / f64 -> TimeSpan TimeSpan / TimeSpan -> f64all of which are correct. (And it doesn't have % because it doesn't interoperate with integers, I guess?)
Adding Duration / Duration -> u128 would make sense to me, and then Duration % Duration -> Duration and Duration % uX -> Duration would make sense
At a high level, Duration is an abstract wrapper of time. At a functional level, there is a level of indirection between the conversion to a discrete unit like milliseconds or nanoseconds. Therefore applying a power or square root to unit-less time doesn't really make sense.
Right, but sometimes you have to make e.g. exponentially increasing durations and it's nice to be able to operate within the type's natural precision (as of this writing a u64 seconds and a u32 nanoseconds). Ideally there'd be a (u64, u32/1e9) fixed point to do unitless operations with, but that's not currently the case.
Edit: which also means these nontrivial additions are equivalent to adding a weird (u64, u32/1e9) fixed point type to the language.
Note that you can do that with https://doc.rust-lang.org/std/time/struct.Duration.html#method.mul_f64 -- either by putting the exponent on the float, or by using that every time through the loop.
What is the standard procedure for making a PR for this addition?
Do I make a PR to the rust GitHub repository under a specific branch, like nightly?
PRs are done on “master” which is the branch from which nightly is built. New API is usually first introduced as unstable, but AFAIK that kind of thing doesn’t work for trait implementations[1], which are always instantly stabilized. So can open a PR now on master, then either there will be a formal procedure for accepting the PR on in the PR thread itself (via a “final commenting period”, “FCP”), or perhaps an RFC might be necessary though that seems unlikely to me, I think… (I’m not 100% sure what the criterions here are though).
For more feedback/questions you can ask here or also on https://rust-lang.zulipchat.com/.
This means, you’d need to add a #[(feature = "some_feature_name", since = "1.61.0")] attribute to the impl, where you invent some sensible (but in practice irrelevant) feature name for your PR instead of “some_feature_name”. ↩︎
Complete agreement here.
Not at all; we do, in fact, have a high standard of usefulness, and we don't automatically add every trait implementation that might make sense, especially if we don't actually have people presenting specific use cases.
Could you talk more about your use case for wanting a remainder operation on time? Not a way you think someone might use it; a way you concretely want to use it in production Rust code. No hypotheticals, please; that's how we can end up with functions that nobody actually uses.
My case is actually (I think) common.
I need a monitor thread to poll a value at routine times. Let's say I need to observe the CPU load 4 times a second. Thus, I need to determine (after a poll finishes) how long to sleep for the next poll time.
You would end up with some code like:
let start_time = Instant::now();
let frequency = Duration::from_seconds(0.25);
loop {
// Sleep until next poll time
let time_since_start = (Instant::now() - start_time).as_nanos();
let time_till_next = frequency.as_nanos() - (time_since_start % frequency.as_nanos());
let sleep_time = Duration::from_nanos(time_till_next as u64);
thread::sleep(sleep_time);
// Poll CPU here
}
With Rem<Duration> implemented, we could have:
let start_time = Instant::now();
let frequency = Duration::from_seconds(0.25);
loop {
// Sleep until next poll time
let time_since_start = Instant::now() - start_time;
let sleep_time = frequency - (time_since_start % frequency);
thread::sleep(sleep_time);
// Poll CPU here
}
Note: The naive thing to do would be to sleep for 0.25 seconds after polling in the loop, but this is incorrect, and will cause accumulating delay over time, especially if the procedure of polling your value is long.
I would probably write that by using a periodic timer, rather than a sleep, which would handle "wake on next expiry" automatically, and may also support aligning that timer with others on the system to reduce overall wakeups.
(I think there are equivalent timer crates for sync code, I just don't know which one to recommend.)
let timer = Timer::interval(Duration::from_millis(250));
while let Some(instant) = timer.next().await {
// Poll CPU here
}
Also, in your second example, I think you wanted time_since_start % frequency, rather than the other way around. You may also have wanted to drop the as_nanos() from the previous line, since this should be Duration % Duration giving a Duration.
I do appreciate the concrete use case, but it seems like one better served by a different approach, and by something wrapped into a reusable component rather than open-coded.
I don't think the two snippets are equivalent. In the second one if time_since_start is less than half the value of frequency then you won't sleep for long enough. You'll still need to do all the operations of the first version, you'll just be able to skip a couple of as_nanos() and Duration::from_nanos.
I have fixed the logical typo oversight in the second snippet.
I'm not entirely sure this is correct...
Here's why:
timer waits 250ms
polling takes 12ms
timer waits another 250ms
^ timer should have waited 238ms.
This implementation would antedate polling and have increasing inaccuracy over time.