From b44b52b69f1cfec786fff5aabc13d190d7e6ed92 Mon Sep 17 00:00:00 2001 From: Christopher Serr Date: Sat, 26 Aug 2023 18:08:30 +0200 Subject: [PATCH] Remove `num-rational` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This removes `num-rational` as a dependency. As it turns out, `num-rational`, based on which features are active, can very easily end up on the critical path during compilation, meaning that it is the crate that determines how long it takes to compile the `image` crate. This is because `cargo` can parallelize compilation of many crates, but crates with a long dependency chains (rather than wide), are the ones that in the end determine the compilation speed. This is one of the learnings of the `serde` drama, where it has been noticed that allowing `serde` and `serde_derive` to compile in parallel reduces its long dependency chain, resulting in faster compile times. This introduces the same kind of optimization here, where for all the following features, `num-rational` turns out to be the longest dependency chain: `png,jpeg,gif,bmp,ico,pnm,tga,tiff,webp,hdr,dxt,dds,farbfeld,qoi` The critical path is the following: `autocfg` → `num-traits` compile build.rs → `num-traits` run build.rs → `num-traits` → `num-integer` → `num-rational` → `image` As it turns out, the only thing really used in `num-rational` is `Ratio`, which can be easily replicated. This cuts out both `num-integer` and `num-rational` cutting compile times by around 0.5 seconds on a debug build. --- Cargo.toml | 1 - src/animation.rs | 113 ++++++++++++++++++++++++++++++++++++++++++---- src/codecs/gif.rs | 3 +- src/codecs/png.rs | 3 +- 4 files changed, 106 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8e7f2a246d..c7218bcc5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,6 @@ path = "./src/lib.rs" [dependencies] bytemuck = { version = "1.7.0", features = ["extern_crate_alloc"] } # includes cast_vec byteorder = "1.3.2" -num-rational = { version = "0.4", default-features = false } num-traits = "0.2.0" gif = { version = "0.12", optional = true } jpeg = { package = "jpeg-decoder", version = "0.3.0", default-features = false, optional = true } diff --git a/src/animation.rs b/src/animation.rs index aad57b4a5d..1681cfbd28 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -1,8 +1,7 @@ +use std::cmp::Ordering; use std::iter::Iterator; use std::time::Duration; -use num_rational::Ratio; - use crate::error::ImageResult; use crate::RgbaImage; @@ -49,14 +48,14 @@ pub struct Frame { /// The delay of a frame relative to the previous one. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd)] pub struct Delay { - ratio: Ratio, + ratio: Ratio, } impl Frame { /// Constructs a new frame without any delay. pub fn new(buffer: RgbaImage) -> Frame { Frame { - delay: Delay::from_ratio(Ratio::from_integer(0)), + delay: Delay::from_ratio(Ratio::new_raw(0, 1)), left: 0, top: 0, buffer, @@ -163,14 +162,14 @@ impl Delay { /// This is guaranteed to be an exact conversion if the `Delay` was previously created with the /// `from_numer_denom_ms` constructor. pub fn numer_denom_ms(self) -> (u32, u32) { - (*self.ratio.numer(), *self.ratio.denom()) + (self.ratio.numer(), self.ratio.denom()) } - pub(crate) fn from_ratio(ratio: Ratio) -> Self { + pub(crate) fn from_ratio(ratio: Ratio) -> Self { Delay { ratio } } - pub(crate) fn into_ratio(self) -> Ratio { + pub(crate) fn into_ratio(self) -> Ratio { self.ratio } @@ -179,7 +178,7 @@ impl Delay { /// Note that `denom_bound` bounds nominator and denominator of all intermediate /// approximations and the end result. fn closest_bounded_fraction(denom_bound: u32, nom: u32, denom: u32) -> (u32, u32) { - use std::cmp::Ordering::{self, *}; + use std::cmp::Ordering::*; assert!(0 < denom); assert!(0 < denom_bound); assert!(nom < denom); @@ -272,11 +271,107 @@ impl From for Duration { let ratio = delay.into_ratio(); let ms = ratio.to_integer(); let rest = ratio.numer() % ratio.denom(); - let nanos = (u64::from(rest) * 1_000_000) / u64::from(*ratio.denom()); + let nanos = (u64::from(rest) * 1_000_000) / u64::from(ratio.denom()); Duration::from_millis(ms.into()) + Duration::from_nanos(nanos) } } +#[derive(Copy, Clone, Debug)] +pub(crate) struct Ratio { + numer: u32, + denom: u32, +} + +impl Ratio { + pub(crate) fn new(numerator: u32, denominator: u32) -> Self { + assert_ne!(denominator, 0); + Self { + numer: numerator, + denom: denominator, + } + } + + fn new_raw(numerator: u32, denominator: u32) -> Self { + Self { + numer: numerator, + denom: denominator, + } + } + + fn numer(&self) -> u32 { + self.numer + } + + fn denom(&self) -> u32 { + self.denom + } + + pub(crate) fn to_integer(&self) -> u32 { + self.numer / self.denom + } +} + +impl PartialEq for Ratio { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl Eq for Ratio {} + +impl PartialOrd for Ratio { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Ratio { + fn cmp(&self, other: &Self) -> Ordering { + // Based on: + // https://github.com/rust-num/num-rational/blob/d89ce12bf9692a83bbe99141f179c8e1f035bbea/src/lib.rs#L337 + + if self.denom == other.denom { + return self.numer.cmp(&other.numer); + } + + if self.numer == other.numer { + return if self.numer == 0 { + Ordering::Equal + } else { + other.denom.cmp(&self.denom) + }; + } + + if let Some((a, b)) = self + .numer + .checked_mul(other.denom) + .and_then(|a| Some((a, other.numer.checked_mul(self.denom)?))) + { + return a.cmp(&b); + } + + let self_int = self.numer / self.denom; + let self_rem = self.numer % self.denom; + let other_int = other.numer / other.denom; + let other_rem = other.numer % other.denom; + + match self_int.cmp(&other_int) { + Ordering::Greater => Ordering::Greater, + Ordering::Less => Ordering::Less, + Ordering::Equal => match (self_rem == 0, other_rem == 0) { + (true, true) => Ordering::Equal, + (true, false) => Ordering::Less, + (false, true) => Ordering::Greater, + (false, false) => { + let self_recip = Ratio::new_raw(self.denom, self_rem); + let other_recip = Ratio::new_raw(other.denom, other_rem); + other_recip.cmp(&self_recip) + } + }, + } + } +} + #[cfg(test)] mod tests { use super::{Delay, Duration, Ratio}; diff --git a/src/codecs/gif.rs b/src/codecs/gif.rs index dcbd84154e..ae0b98e7eb 100644 --- a/src/codecs/gif.rs +++ b/src/codecs/gif.rs @@ -34,9 +34,8 @@ use std::mem; use gif::ColorOutput; use gif::{DisposalMethod, Frame}; -use num_rational::Ratio; -use crate::animation; +use crate::animation::{self, Ratio}; use crate::color::{ColorType, Rgba}; use crate::error::{ DecodingError, EncodingError, ImageError, ImageResult, ParameterError, ParameterErrorKind, diff --git a/src/codecs/png.rs b/src/codecs/png.rs index b9f98ce882..9384451ce1 100644 --- a/src/codecs/png.rs +++ b/src/codecs/png.rs @@ -10,10 +10,9 @@ use std::convert::TryFrom; use std::fmt; use std::io::{self, Read, Write}; -use num_rational::Ratio; use png::{BlendOp, DisposeOp}; -use crate::animation::{Delay, Frame, Frames}; +use crate::animation::{Delay, Frame, Frames, Ratio}; use crate::color::{Blend, ColorType, ExtendedColorType}; use crate::error::{ DecodingError, EncodingError, ImageError, ImageResult, LimitError, LimitErrorKind,