-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add as_millis_{f64,f32}
helper functions for Duration
#349
Comments
as_*_f64
helper functions for Duration
for units other than secondsas_millis_{f64,f32}
helper functions for Duration
We discussed this in the libs-api meeting today. The first point that came up is that the original motivation (precision loss due to Regarding the Feel free to open a tracking issue and open a PR to rust-lang/rust to add it as an unstable feature. |
**Description** Add a lint which checks for a binop or cast where the expression is converting a Duration to a floating point number, and losing precision along the way. The lint won't fire for most cases involving `as_nanos()` since converting that to floating point already gives the max precision. The lint is also restricted to an MSRV of 1.38.0, since that is when the `as_secs_{f32,f64}()` methods were stabilized. **Motivation** This change is motivated by [a rust stdlib ACP which proposed `as_millis_{f64,f32}`][ACP-link]. As part of that I did some code searches on github (see ACP for links) that showed a lot of code which converted Duration values to floating point using methods other than `as_secs_{f32,64}()`, and were losing precision because of that. This lint seems like a good way to raise awareness and prompt using the existing methods. [ACP-link]: rust-lang/libs-team#349 **Testing Done** Added UI tests, ran `cargo test`, followed the Clippy manual
**Description** Add a lint which checks for a binop or cast where the expression is converting a Duration to a floating point number, and losing precision along the way. The lint won't fire for most cases involving `as_nanos()` since converting that to floating point already gives the max precision. The lint is also restricted to an MSRV of 1.38.0, since that is when the `as_secs_{f32,f64}()` methods were stabilized. **Motivation** This change is motivated by [a rust stdlib ACP which proposed `as_millis_{f64,f32}`][ACP-link]. As part of that I did some code searches on github (see ACP for links) that showed a lot of code which converted Duration values to floating point using methods other than `as_secs_{f32,64}()`, and were losing precision because of that. This lint seems like a good way to raise awareness and prompt using the existing methods. [ACP-link]: rust-lang/libs-team#349 **Testing Done** Added UI tests, ran `cargo test`, followed the Clippy manual
**Description** Add a lint which checks for a binop or cast where the expression is converting a Duration to a floating point number, and losing precision along the way. The lint won't fire for most cases involving `as_nanos()` since converting that to floating point already gives the max precision. The lint is also restricted to an MSRV of 1.38.0, since that is when the `as_secs_{f32,f64}()` methods were stabilized. **Motivation** This change is motivated by [a rust stdlib ACP which proposed `as_millis_{f64,f32}`][ACP-link]. As part of that I did some code searches on github (see ACP for links) that showed a lot of code which converted Duration values to floating point using methods other than `as_secs_{f32,64}()`, and were losing precision because of that. This lint seems like a good way to raise awareness and prompt using the existing methods. [ACP-link]: rust-lang/libs-team#349 **Testing Done** Added UI tests, ran `cargo test`, followed the Clippy manual
**Description** Add a lint which checks for a binop or cast where the expression is converting a Duration to a floating point number, and losing precision along the way. The lint won't fire for most cases involving `as_nanos()` since converting that to floating point already gives the max precision. The lint is also restricted to an MSRV of 1.38.0, since that is when the `as_secs_{f32,f64}()` methods were stabilized. **Motivation** This change is motivated by [a rust stdlib ACP which proposed `as_millis_{f64,f32}`][ACP-link]. As part of that I did some code searches on github (see ACP for links) that showed a lot of code which converted Duration values to floating point using methods other than `as_secs_{f32,64}()`, and were losing precision because of that. This lint seems like a good way to raise awareness and prompt using the existing methods. [ACP-link]: rust-lang/libs-team#349 **Testing Done** Added UI tests, ran `cargo test`, followed the Clippy manual
**Description** Add a lint which checks for a binop or cast where the expression is converting a Duration to a floating point number, and losing precision along the way. The lint won't fire for most cases involving `as_nanos()` since converting that to floating point already gives the max precision. The lint is also restricted to an MSRV of 1.38.0, since that is when the `as_secs_{f32,f64}()` methods were stabilized. **Motivation** This change is motivated by [a rust stdlib ACP which proposed `as_millis_{f64,f32}`][ACP-link]. As part of that I did some code searches on github (see ACP for links) that showed a lot of code which converted Duration values to floating point using methods other than `as_secs_{f32,64}()`, and were losing precision because of that. This lint seems like a good way to raise awareness and prompt using the existing methods. [ACP-link]: rust-lang/libs-team#349 **Testing Done** Added UI tests, ran `cargo test`, followed the Clippy manual
**Description** Add a lint which checks for a binop or cast where the expression is converting a Duration to a floating point number, and losing precision along the way. The lint won't fire for most cases involving `as_nanos()` since converting that to floating point already gives the max precision. The lint is also restricted to an MSRV of 1.38.0, since that is when the `as_secs_{f32,f64}()` methods were stabilized. **Motivation** This change is motivated by [a rust stdlib ACP which proposed `as_millis_{f64,f32}`][ACP-link]. As part of that I did some code searches on github (see ACP for links) that showed a lot of code which converted Duration values to floating point using methods other than `as_secs_{f32,64}()`, and were losing precision because of that. This lint seems like a good way to raise awareness and prompt using the existing methods. [ACP-link]: rust-lang/libs-team#349 **Testing Done** Added UI tests, ran `cargo test`, followed the Clippy manual
**Description** Add a lint which checks for a binop or cast where the expression is converting a Duration to a floating point number, and losing precision along the way. The lint won't fire for most cases involving `as_nanos()` since converting that to floating point already gives the max precision. The lint is also restricted to an MSRV of 1.38.0, since that is when the `as_secs_{f32,f64}()` methods were stabilized. **Motivation** This change is motivated by [a rust stdlib ACP which proposed `as_millis_{f64,f32}`][ACP-link]. As part of that I did some code searches on github (see ACP for links) that showed a lot of code which converted Duration values to floating point using methods other than `as_secs_{f32,64}()`, and were losing precision because of that. This lint seems like a good way to raise awareness and prompt using the existing methods. [ACP-link]: rust-lang/libs-team#349 **Testing Done** Added UI tests, ran `cargo test`, followed the Clippy manual
Proposal
Problem statement
We have a very common pattern in our code base when we're recording metrics that involve a span of time:
Reminding people to use the second case so that preserve sub-millisecond precision is not very difficult, but I'd like to make it an even easier pattern for people to use and simply express that they want "the number of milliseconds as a floating point number, including the sub-millisecond values for precision".
Motivating examples or use cases
As far as I can tell, the
.as_secs_f64() * 1000.0
is a very common pattern and it searching for it on Github returns hundreds of results (alt search for `*f32).Doing a little research for this I realized people also write this as
.as_nanos() as f64 / 1_000_000.0
, Github search yield some additional results.Solution sketch
Ideally the new methods would include:
I would also be interested in
as_{micros,nanos}_{f64,f32}
from a completeness perspective, but I haven't seen this pattern as commonly for conversion to those units.Alternatives
The main alternative I see is to continue using the
duration.as_secs_f64() * 1000.0
. This alternative is quick to type and easy to understand.Links and related work
A Github search provides some existing examples of Duration extension traits or other implementations, here are a couple:
https://github.com/bytedance/g3/blob/0b46b5597e6193da8c7ebd6c2e860590f3aa80ca/lib/g3-types/src/ext/duration.rs#L28-L30
https://github.com/cavedon/valixdrive/blob/e035ea47d4158e0fe0ec9eab6dca9f0f9c187d6a/src/main.rs#L57-L59
What happens now?
This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.
Possible responses
The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):
Second, if there's a concrete solution:
The text was updated successfully, but these errors were encountered: