Auto merge of #50364 - LukasKalbertodt:improve-duration-debug-impl, r=KodrAus
Improve `Debug` impl of `time::Duration`
Hi there!
For a long time now, I was getting annoyed by the derived `Debug` impl of `Duration`. Usually, I use `Duration` to either do quick'n'dirty benchmarking or measuring the time of some operation in general. The output of the derived Debug impl is hard to parse for humans: is { secs: 0, nanos: 968360102 } or { secs: 0, nanos 98507324 } longer?
So after running into the annoyance several times (sometimes building my own function to print the Duration properly), I decided to tackle this. Now the output looks like this:
```
Duration::new(1, 0) => 1s
Duration::new(1, 1) => 1.000000001s
Duration::new(1, 300) => 1.0000003s
Duration::new(1, 4000) => 1.000004s
Duration::new(1, 600000) => 1.0006s
Duration::new(1, 7000000) => 1.007s
Duration::new(0, 0) => 0ns
Duration::new(0, 1) => 1ns
Duration::new(0, 300) => 300ns
Duration::new(0, 4001) => 4.001µs
Duration::new(0, 600300) => 600.3µs
Duration::new(0, 7000000) => 7ms
```
Note that I implemented the formatting manually and didn't use floats. No information is "lost" when printing. So `Duration::new(123_456_789_000, 900_000_001)` prints as `123456789000.900000001s`.
~~This is not yet finished~~, but I wanted to open the PR now already in order to get some feedback (maybe everyone likes the derived impl).
### Still ToDo:
- [x] Respect precision ~~and width~~ parameter of the formatter (see [this comment](https://github.com/rust-lang/rust/pull/50364#issuecomment-386107107))
### Alternatives/Decisions
- Should large durations displayed in minutes, hours, days, ...? For now, I decided not to because the current formatting is close the how a `Duration` is stored. From this new `Debug` output, you can still easily see what the values of `secs` and `nanos` are. A formatting like `3h 27m 12s 9ms` might be more appropriate for a `Display` impl?
- Should this rather be a `Display` impl and should `Debug` be derived? Maybe this formatting is too fancy for `Debug`? In my opinion it's not and, as already mentioned, from the current format one can still very easily determine the values for `secs` and `nanos`.
- Whitespace between the number and the unit?
### Notes for reviewers
- ~~The combined diff sucks. Rather review both commits individually.~~
- ~~In the unit test, I am building my own type implementing `fmt::Write` to test the output. Maybe there is already something like that which I can use?~~
- My `Debug` impl block is marked as `#[stable(...)]`... but that's fine since the derived Debug impl was stable already, right?
---
~~Apart from the main change, I moved all `time` unit tests into the `tests` directory. All other `libcore` tests are there, so I guess it was simply an oversight. Prior to this change, the `time` tests weren't run, so I guess this is kind of a bug fix. If my `Debug` impl is rejected, I can of course just send the fix as PR.~~ (this was already merged in #50466)
This commit is contained in:
bors
commit
444a9c3f1a
@ -160,3 +160,131 @@ fn checked_div() {
|
||||
assert_eq!(Duration::new(1, 0).checked_div(2), Some(Duration::new(0, 500_000_000)));
|
||||
assert_eq!(Duration::new(2, 0).checked_div(0), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_extreme_values() {
|
||||
assert_eq!(
|
||||
format!("{:?}", Duration::new(18_446_744_073_709_551_615, 123_456_789)),
|
||||
"18446744073709551615.123456789s"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_secs() {
|
||||
assert_eq!(format!("{:?}", Duration::new(7, 000_000_000)), "7s");
|
||||
assert_eq!(format!("{:?}", Duration::new(7, 100_000_000)), "7.1s");
|
||||
assert_eq!(format!("{:?}", Duration::new(7, 000_010_000)), "7.00001s");
|
||||
assert_eq!(format!("{:?}", Duration::new(7, 000_000_001)), "7.000000001s");
|
||||
assert_eq!(format!("{:?}", Duration::new(7, 123_456_789)), "7.123456789s");
|
||||
|
||||
assert_eq!(format!("{:?}", Duration::new(88, 000_000_000)), "88s");
|
||||
assert_eq!(format!("{:?}", Duration::new(88, 100_000_000)), "88.1s");
|
||||
assert_eq!(format!("{:?}", Duration::new(88, 000_010_000)), "88.00001s");
|
||||
assert_eq!(format!("{:?}", Duration::new(88, 000_000_001)), "88.000000001s");
|
||||
assert_eq!(format!("{:?}", Duration::new(88, 123_456_789)), "88.123456789s");
|
||||
|
||||
assert_eq!(format!("{:?}", Duration::new(999, 000_000_000)), "999s");
|
||||
assert_eq!(format!("{:?}", Duration::new(999, 100_000_000)), "999.1s");
|
||||
assert_eq!(format!("{:?}", Duration::new(999, 000_010_000)), "999.00001s");
|
||||
assert_eq!(format!("{:?}", Duration::new(999, 000_000_001)), "999.000000001s");
|
||||
assert_eq!(format!("{:?}", Duration::new(999, 123_456_789)), "999.123456789s");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_millis() {
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_000_000)), "7ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_100_000)), "7.1ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_000_001)), "7.000001ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_123_456)), "7.123456ms");
|
||||
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_000_000)), "88ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_100_000)), "88.1ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_000_001)), "88.000001ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_123_456)), "88.123456ms");
|
||||
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_000_000)), "999ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_100_000)), "999.1ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_000_001)), "999.000001ms");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_123_456)), "999.123456ms");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_micros() {
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_000)), "7µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_100)), "7.1µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_001)), "7.001µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 7_123)), "7.123µs");
|
||||
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_000)), "88µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_100)), "88.1µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_001)), "88.001µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88_123)), "88.123µs");
|
||||
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_000)), "999µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_100)), "999.1µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_001)), "999.001µs");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999_123)), "999.123µs");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_nanos() {
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 0)), "0ns");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 1)), "1ns");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 88)), "88ns");
|
||||
assert_eq!(format!("{:?}", Duration::new(0, 999)), "999ns");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_precision_zero() {
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 0)), "0ns");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 123)), "123ns");
|
||||
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_001)), "1µs");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_499)), "1µs");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_500)), "2µs");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_999)), "2µs");
|
||||
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_000_001)), "1ms");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_499_999)), "1ms");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_500_000)), "2ms");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(0, 1_999_999)), "2ms");
|
||||
|
||||
assert_eq!(format!("{:.0?}", Duration::new(1, 000_000_001)), "1s");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(1, 499_999_999)), "1s");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(1, 500_000_000)), "2s");
|
||||
assert_eq!(format!("{:.0?}", Duration::new(1, 999_999_999)), "2s");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_precision_two() {
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 0)), "0.00ns");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 123)), "123.00ns");
|
||||
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 1_000)), "1.00µs");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 7_001)), "7.00µs");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 7_100)), "7.10µs");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 7_109)), "7.11µs");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 7_199)), "7.20µs");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 1_999)), "2.00µs");
|
||||
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 1_000_000)), "1.00ms");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 3_001_000)), "3.00ms");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 3_100_000)), "3.10ms");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(0, 1_999_999)), "2.00ms");
|
||||
|
||||
assert_eq!(format!("{:.2?}", Duration::new(1, 000_000_000)), "1.00s");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(4, 001_000_000)), "4.00s");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(2, 100_000_000)), "2.10s");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(2, 104_990_000)), "2.10s");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(2, 105_000_000)), "2.11s");
|
||||
assert_eq!(format!("{:.2?}", Duration::new(8, 999_999_999)), "9.00s");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn debug_formatting_precision_high() {
|
||||
assert_eq!(format!("{:.5?}", Duration::new(0, 23_678)), "23.67800µs");
|
||||
|
||||
assert_eq!(format!("{:.9?}", Duration::new(1, 000_000_000)), "1.000000000s");
|
||||
assert_eq!(format!("{:.10?}", Duration::new(4, 001_000_000)), "4.0010000000s");
|
||||
assert_eq!(format!("{:.20?}", Duration::new(4, 001_000_000)), "4.00100000000000000000s");
|
||||
}
|
||||
|
||||
@ -21,6 +21,7 @@
|
||||
//! assert_eq!(Duration::new(5, 0), Duration::from_secs(5));
|
||||
//! ```
|
||||
|
||||
use fmt;
|
||||
use iter::Sum;
|
||||
use ops::{Add, Sub, Mul, Div, AddAssign, SubAssign, MulAssign, DivAssign};
|
||||
|
||||
@ -59,7 +60,7 @@ const MICROS_PER_SEC: u64 = 1_000_000;
|
||||
/// let ten_millis = Duration::from_millis(10);
|
||||
/// ```
|
||||
#[stable(feature = "duration", since = "1.3.0")]
|
||||
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash, Default)]
|
||||
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
|
||||
pub struct Duration {
|
||||
secs: u64,
|
||||
nanos: u32, // Always 0 <= nanos < NANOS_PER_SEC
|
||||
@ -485,3 +486,119 @@ impl<'a> Sum<&'a Duration> for Duration {
|
||||
iter.fold(Duration::new(0, 0), |a, b| a + *b)
|
||||
}
|
||||
}
|
||||
|
||||
#[stable(feature = "duration_debug_impl", since = "1.27.0")]
|
||||
impl fmt::Debug for Duration {
|
||||
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
|
||||
/// Formats a floating point number in decimal notation.
|
||||
///
|
||||
/// The number is given as the `integer_part` and a fractional part.
|
||||
/// The value of the fractional part is `fractional_part / divisor`. So
|
||||
/// `integer_part` = 3, `fractional_part` = 12 and `divisor` = 100
|
||||
/// represents the number `3.012`. Trailing zeros are omitted.
|
||||
///
|
||||
/// `divisor` must not be above 100_000_000. It also should be a power
|
||||
/// of 10, everything else doesn't make sense. `fractional_part` has
|
||||
/// to be less than `10 * divisor`!
|
||||
fn fmt_decimal(
|
||||
f: &mut fmt::Formatter,
|
||||
mut integer_part: u64,
|
||||
mut fractional_part: u32,
|
||||
mut divisor: u32,
|
||||
) -> fmt::Result {
|
||||
// Encode the fractional part into a temporary buffer. The buffer
|
||||
// only need to hold 9 elements, because `fractional_part` has to
|
||||
// be smaller than 10^9. The buffer is prefilled with '0' digits
|
||||
// to simplify the code below.
|
||||
let mut buf = [b'0'; 9];
|
||||
|
||||
// The next digit is written at this position
|
||||
let mut pos = 0;
|
||||
|
||||
// We keep writing digits into the buffer while there are non-zero
|
||||
// digits left and we haven't written enough digits yet.
|
||||
while fractional_part > 0 && pos < f.precision().unwrap_or(9) {
|
||||
// Write new digit into the buffer
|
||||
buf[pos] = b'0' + (fractional_part / divisor) as u8;
|
||||
|
||||
fractional_part %= divisor;
|
||||
divisor /= 10;
|
||||
pos += 1;
|
||||
}
|
||||
|
||||
// If a precision < 9 was specified, there may be some non-zero
|
||||
// digits left that weren't written into the buffer. In that case we
|
||||
// need to perform rounding to match the semantics of printing
|
||||
// normal floating point numbers. However, we only need to do work
|
||||
// when rounding up. This happens if the first digit of the
|
||||
// remaining ones is >= 5.
|
||||
if fractional_part > 0 && fractional_part >= divisor * 5 {
|
||||
// Round up the number contained in the buffer. We go through
|
||||
// the buffer backwards and keep track of the carry.
|
||||
let mut rev_pos = pos;
|
||||
let mut carry = true;
|
||||
while carry && rev_pos > 0 {
|
||||
rev_pos -= 1;
|
||||
|
||||
// If the digit in the buffer is not '9', we just need to
|
||||
// increment it and can stop then (since we don't have a
|
||||
// carry anymore). Otherwise, we set it to '0' (overflow)
|
||||
// and continue.
|
||||
if buf[rev_pos] < b'9' {
|
||||
buf[rev_pos] += 1;
|
||||
carry = false;
|
||||
} else {
|
||||
buf[rev_pos] = b'0';
|
||||
}
|
||||
}
|
||||
|
||||
// If we still have the carry bit set, that means that we set
|
||||
// the whole buffer to '0's and need to increment the integer
|
||||
// part.
|
||||
if carry {
|
||||
integer_part += 1;
|
||||
}
|
||||
}
|
||||
|
||||
// Determine the end of the buffer: if precision is set, we just
|
||||
// use as many digits from the buffer (capped to 9). If it isn't
|
||||
// set, we only use all digits up to the last non-zero one.
|
||||
let end = f.precision().map(|p| ::cmp::min(p, 9)).unwrap_or(pos);
|
||||
|
||||
// If we haven't emitted a single fractional digit and the precision
|
||||
// wasn't set to a non-zero value, we don't print the decimal point.
|
||||
if end == 0 {
|
||||
write!(f, "{}", integer_part)
|
||||
} else {
|
||||
// We are only writing ASCII digits into the buffer and it was
|
||||
// initialized with '0's, so it contains valid UTF8.
|
||||
let s = unsafe {
|
||||
::str::from_utf8_unchecked(&buf[..end])
|
||||
};
|
||||
|
||||
// If the user request a precision > 9, we pad '0's at the end.
|
||||
let w = f.precision().unwrap_or(pos);
|
||||
write!(f, "{}.{:0<width$}", integer_part, s, width = w)
|
||||
}
|
||||
}
|
||||
|
||||
// Print leading '+' sign if requested
|
||||
if f.sign_plus() {
|
||||
write!(f, "+")?;
|
||||
}
|
||||
|
||||
if self.secs > 0 {
|
||||
fmt_decimal(f, self.secs, self.nanos, 100_000_000)?;
|
||||
f.write_str("s")
|
||||
} else if self.nanos >= 1_000_000 {
|
||||
fmt_decimal(f, self.nanos as u64 / 1_000_000, self.nanos % 1_000_000, 100_000)?;
|
||||
f.write_str("ms")
|
||||
} else if self.nanos >= 1_000 {
|
||||
fmt_decimal(f, self.nanos as u64 / 1_000, self.nanos % 1_000, 100)?;
|
||||
f.write_str("µs")
|
||||
} else {
|
||||
fmt_decimal(f, self.nanos as u64, 0, 1)?;
|
||||
f.write_str("ns")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user