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.

Sign up for GitHub

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

Jump to bottom

Implement lossless From methods for Duration conversions #342

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement lossless From methods for Duration conversions #342

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/duration/std.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ impl From<Duration> for std::time::Duration {
/// 1. If the duration is negative, this will return a std::time::Duration::ZERO.
/// 2. If the duration larger than the MAX duration, this will return std::time::Duration::MAX
fn from(hf_duration: Duration) -> Self {
let (sign, days, hours, minutes, seconds, milli, us, nano) = hf_duration.decompose();
if sign < 0 {
std::time::Duration::ZERO
} else {
// Build the seconds separately from the nanos.
let above_ns_f64: f64 =
Duration::compose(sign, days, hours, minutes, seconds, milli, us, 0).to_seconds();
std::time::Duration::new(above_ns_f64 as u64, nano as u32)
}
const NANOS_PER_SEC: u128 = std::time::Duration::from_secs(1).as_nanos();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a simpler way to implement this functionality is by using the to_unit function on the hf_duration.

I haven't tested the following, so consider this a draft.

let seconds = hf_duration.to_seconds().floor() as u64; // Somehow check that this won't fail
let subsec_nanos = (hf_duration - Unit::Second * seconds).to_unit(Unit::Nanoseconds).floor() as u32; // Also check that this won't fail, probably using try_from as you initially recommended

std::time::Duration(seconds, subsec_nanos)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a better way to handle the subsec_nanos than what I wrote above. After the subtraction, we know that the duration has zero centuries. So, we can safely ignore that part and operate directly on nanoseconds.

use crate::NANOSECONDS_PER_SECOND; // https://docs.rs/hifitime/latest/hifitime/constant.NANOSECONDS_PER_SECOND.html
let subsec_nanos: u64 = (hf_duration - Unit::Second * seconds).nanoseconds / NANOSECONDS_PER_SECOND;
// Finally convert this to u32 safely.

let nanos = hf_duration.total_nanoseconds();
let unsigned_nanos = u128::try_from(nanos).unwrap_or(0);

let secs: u64 = (unsigned_nanos / NANOS_PER_SEC)
.try_into()
.unwrap_or(u64::MAX);
let subsec_nanos = (unsigned_nanos % NANOS_PER_SEC) as u32;

std::time::Duration::new(secs, subsec_nanos)
}
}

Expand All @@ -40,6 +41,6 @@ impl From<std::time::Duration> for Duration {
/// 1. If the duration is negative, this will return a std::time::Duration::ZERO.
/// 2. If the duration larger than the MAX duration, this will return std::time::Duration::MAX
fn from(std_duration: std::time::Duration) -> Self {
std_duration.as_secs_f64() * Unit::Second
Duration::from_total_nanoseconds(std_duration.as_nanos().try_into().unwrap_or(i128::MAX))
}
}
Loading