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 5139bf3
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 16 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
78 changes: 67 additions & 11 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 { numer: 0, denom: 1 }),
left: 0,
top: 0,
buffer,
Expand Down Expand Up @@ -115,7 +114,7 @@ impl Delay {
/// ```
pub fn from_numer_denom_ms(numerator: u32, denominator: u32) -> Self {
Delay {
ratio: Ratio::new_raw(numerator, denominator),
ratio: Ratio::new(numerator, denominator),
}
}

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 @@ -271,12 +270,69 @@ impl From<Delay> for Duration {
fn from(delay: Delay) -> Self {
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 rest = ratio.numer % 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 {
#[inline]
pub(crate) fn new(numerator: u32, denominator: u32) -> Self {
assert_ne!(denominator, 0);
Self {
numer: numerator,
denom: denominator,
}
}

#[inline]
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 {
// The following comparison can be simplified:
// a / b <cmp> c / d
// We multiply both sides by `b`:
// a <cmp> c * b / d
// We multiply both sides by `d`:
// a * d <cmp> c * b

let a: u32 = self.numer;
let b: u32 = self.denom;
let c: u32 = other.numer;
let d: u32 = other.denom;

// We cast the types from `u32` to `u64` in order
// to not overflow the multiplications.

(a as u64 * d as u64).cmp(&(c as u64 * b as u64))
}
}

#[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 5139bf3

Please sign in to comment.