Skip to content

Commit

Permalink
Remove num-rational
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
CryZe committed Aug 26, 2023
1 parent 065c848 commit b44b52b
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 14 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
113 changes: 104 additions & 9 deletions src/animation.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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<u32>,
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,
Expand Down Expand Up @@ -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<u32>) -> Self {
pub(crate) fn from_ratio(ratio: Ratio) -> Self {
Delay { ratio }
}

pub(crate) fn into_ratio(self) -> Ratio<u32> {
pub(crate) fn into_ratio(self) -> Ratio {
self.ratio
}

Expand All @@ -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);
Expand Down Expand Up @@ -272,11 +271,107 @@ impl From<Delay> 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<Ordering> {
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};
Expand Down
3 changes: 1 addition & 2 deletions src/codecs/gif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions src/codecs/png.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit b44b52b

Please sign in to comment.