From 6ae014f13a0ff996ff519f015837ad8a4aba768e Mon Sep 17 00:00:00 2001 From: Mike Maley Date: Sun, 15 Feb 2026 22:24:24 -0500 Subject: [PATCH 1/3] A new implementation of Ord::cmp This implementation seeks to minimize the about of division and modulus operations to improve efficiency --- src/lib.rs | 153 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 114 insertions(+), 39 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b1cf2a4..c80cf95 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -341,50 +341,125 @@ where impl Ord for Ratio { #[inline] fn cmp(&self, other: &Self) -> cmp::Ordering { - // With equal denominators, the numerators can be directly compared - if self.denom == other.denom { - let ord = self.numer.cmp(&other.numer); - return if self.denom < T::zero() { - ord.reverse() - } else { - ord - }; - } + // self ?= other, returns the ordering where the ordering '?=' + // is either '<', '>', or '=='. - // With equal numerators, the denominators can be inversely compared - if self.numer == other.numer { - if self.numer.is_zero() { - return cmp::Ordering::Equal; - } - let ord = self.denom.cmp(&other.denom); - return if self.numer < T::zero() { - ord + use cmp::Ordering; + + // self = a/b, other = c/d + let (mut a, mut b) = (self.numer.clone(), self.denom.clone()); + let (mut c, mut d) = (other.numer.clone(), other.denom.clone()); + + // Keep denominators positive + // a/b ?= c/d => a*d ?= b*c, is valid if b > 0 and d > 0. + fn normalize(a: T, b: T) -> (T, T) { + if b < T::zero() { + (T::zero() - a, T::zero() - b) } else { - ord.reverse() - }; + (a, b) + } } - // Unfortunately, we don't have CheckedMul to try. That could sometimes avoid all the - // division below, or even always avoid it for BigInt and BigUint. - // FIXME- future breaking change to add Checked* to Integer? - - // Compare as floored integers and remainders - let (self_int, self_rem) = self.numer.div_mod_floor(&self.denom); - let (other_int, other_rem) = other.numer.div_mod_floor(&other.denom); - match self_int.cmp(&other_int) { - cmp::Ordering::Greater => cmp::Ordering::Greater, - cmp::Ordering::Less => cmp::Ordering::Less, - cmp::Ordering::Equal => { - match (self_rem.is_zero(), other_rem.is_zero()) { - (true, true) => cmp::Ordering::Equal, - (true, false) => cmp::Ordering::Less, - (false, true) => cmp::Ordering::Greater, - (false, false) => { - // Compare the reciprocals of the remaining fractions in reverse - let self_recip = Ratio::new_raw(self.denom.clone(), self_rem); - let other_recip = Ratio::new_raw(other.denom.clone(), other_rem); - self_recip.cmp(&other_recip).reverse() + (a, b) = normalize(a, b); + (c, d) = normalize(c, d); + + // reduces a/b to (a - q*c)/(b - q*d), where q = min(a/c, b/d) + fn reduce(a: T, b: T, c: &T, d: &T) -> (T, T) { + // use the min to prevent overflow + let q = cmp::min( + a.clone() / c.clone(), + b.clone() / d.clone() + ); + // a/b ?= c/d => a*d ?= b*c => + // a*d - q*c*d ?= b*c - q*c*d => + // (a - q*c)*d ?= c*(b - q*d) => + // (a - q*c)/(b - q*d) ?= c/d + (a - q.clone() * c.clone(), b - q * d.clone()) + } + + loop { + match a.cmp(&T::zero()) { + // 0 < a + Ordering::Greater => { + if c <= T::zero() { + // c <= 0 < a + return cmp::Ordering::Greater; + } + match a.cmp(&c) { + // 0 < c < a + Ordering::Greater => { + if b <= d { + // a > c && d >= b ∴ a*d > b*c + return cmp::Ordering::Greater; + } + (a, b) = reduce(a, b, &c, &d); + if b == T::zero() { + // a*d ?= c*b, b == 0, d > 0 => + // a ?= 0 + return a.cmp(&T::zero()); + } + } + // 0 < a < c + Ordering::Less => { + if b >= d { + // a < c && d <= b ∴ a*d < b*c + return cmp::Ordering::Less; + } + (c, d) = reduce(c, d, &a, &b); + if d == T::zero() { + // a*d ?= c*b, d == 0, b > 0 => + // 0 ?= c + return T::zero().cmp(&c); + } + } + Ordering::Equal => { + // a*d ?= b*c, a == c => d ?= b + return d.cmp(&b); + } + } + }, + // a < 0 + Ordering::Less => { + if c >= T::zero() { + // a < 0 <= c + return Ordering::Less + } + match a.cmp(&c) { + // c < a < 0 + Ordering::Greater => { + if b >= d { + // c < a < 0 && d <= b ∴ a*d > b*c + return cmp::Ordering::Greater; + } + (c, d) = reduce(c, d, &a, &b); + if d == T::zero() { + // a*d ?= c*b, d == 0, b > 0 => + // 0 ?= c + return T::zero().cmp(&c); + } + } + // a < c < 0 + Ordering::Less => { + if b <= d { + // a < c < 0 && d >= b ∴ a*d < b*c + return cmp::Ordering::Less; + } + (a, b) = reduce(a, b, &c, &d); + if b == T::zero() { + // a*d ?= c*b, b == 0, d > 0 => + // a ?= 0 + return a.cmp(&T::zero()); + } + } + Ordering::Equal => { + // a*d ?= b*c, a == c < 0 => b ?= d + return b.cmp(&d); + } } + }, + Ordering::Equal => { + // a*d ?= b*c, a == 0, b > 0 => 0 ?= c + return T::zero().cmp(&c); } } } From 7377b30b1fdf78bd9f6f2f72b5b7eb343f95bdbb Mon Sep 17 00:00:00 2001 From: Mike Maley Date: Wed, 18 Feb 2026 21:34:17 -0500 Subject: [PATCH 2/3] Optimized Ord::cmp Optimized the Ord::cmp function after benchmarking it against the original function. --- src/lib.rs | 192 ++++++++++++++++++++++------------------------------- 1 file changed, 80 insertions(+), 112 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c80cf95..05ec30d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -339,130 +339,98 @@ where // for those multiplications to overflow fixed-size integers, so we need to take care. impl Ord for Ratio { - #[inline] + fn cmp(&self, other: &Self) -> cmp::Ordering { // self ?= other, returns the ordering where the ordering '?=' // is either '<', '>', or '=='. use cmp::Ordering; - // self = a/b, other = c/d - let (mut a, mut b) = (self.numer.clone(), self.denom.clone()); - let (mut c, mut d) = (other.numer.clone(), other.denom.clone()); + let zero = T::zero(); - // Keep denominators positive - // a/b ?= c/d => a*d ?= b*c, is valid if b > 0 and d > 0. - fn normalize(a: T, b: T) -> (T, T) { - if b < T::zero() { - (T::zero() - a, T::zero() - b) - } else { - (a, b) + // makes self.denom > 0, and checks other.denom > 0 as a side effect + if self.denom <= zero { + if self.denom == zero { + panic!("denominator == 0"); + } + return (other).cmp(&Ratio::new_raw( + T::zero() - self.numer.clone(), + T::zero() - self.denom.clone(), + )).reverse(); + } + + // let a/b = lhs, c/d = rhs + // 0 < c < a and 0 < b and 0 < d + fn reduce( + lhs: &Ratio, + rhs: &Ratio + ) -> Ordering { + if lhs.denom <= rhs.denom { + return Ordering::Greater; } - } - (a, b) = normalize(a, b); - (c, d) = normalize(c, d); - - // reduces a/b to (a - q*c)/(b - q*d), where q = min(a/c, b/d) - fn reduce(a: T, b: T, c: &T, d: &T) -> (T, T) { - // use the min to prevent overflow - let q = cmp::min( - a.clone() / c.clone(), - b.clone() / d.clone() - ); - // a/b ?= c/d => a*d ?= b*c => - // a*d - q*c*d ?= b*c - q*c*d => - // (a - q*c)*d ?= c*(b - q*d) => - // (a - q*c)/(b - q*d) ?= c/d - (a - q.clone() * c.clone(), b - q * d.clone()) - } - - loop { - match a.cmp(&T::zero()) { - // 0 < a - Ordering::Greater => { - if c <= T::zero() { - // c <= 0 < a - return cmp::Ordering::Greater; - } - match a.cmp(&c) { - // 0 < c < a - Ordering::Greater => { - if b <= d { - // a > c && d >= b ∴ a*d > b*c - return cmp::Ordering::Greater; - } - (a, b) = reduce(a, b, &c, &d); - if b == T::zero() { - // a*d ?= c*b, b == 0, d > 0 => - // a ?= 0 - return a.cmp(&T::zero()); - } - } - // 0 < a < c - Ordering::Less => { - if b >= d { - // a < c && d <= b ∴ a*d < b*c - return cmp::Ordering::Less; - } - (c, d) = reduce(c, d, &a, &b); - if d == T::zero() { - // a*d ?= c*b, d == 0, b > 0 => - // 0 ?= c - return T::zero().cmp(&c); - } - } - Ordering::Equal => { - // a*d ?= b*c, a == c => d ?= b - return d.cmp(&b); - } - } - }, - // a < 0 - Ordering::Less => { - if c >= T::zero() { - // a < 0 <= c - return Ordering::Less - } - match a.cmp(&c) { - // c < a < 0 - Ordering::Greater => { - if b >= d { - // c < a < 0 && d <= b ∴ a*d > b*c - return cmp::Ordering::Greater; - } - (c, d) = reduce(c, d, &a, &b); - if d == T::zero() { - // a*d ?= c*b, d == 0, b > 0 => - // 0 ?= c - return T::zero().cmp(&c); - } - } - // a < c < 0 - Ordering::Less => { - if b <= d { - // a < c < 0 && d >= b ∴ a*d < b*c - return cmp::Ordering::Less; - } - (a, b) = reduce(a, b, &c, &d); - if b == T::zero() { - // a*d ?= c*b, b == 0, d > 0 => - // a ?= 0 - return a.cmp(&T::zero()); - } - } - Ordering::Equal => { - // a*d ?= b*c, a == c < 0 => b ?= d - return b.cmp(&d); - } - } - }, - Ordering::Equal => { - // a*d ?= b*c, a == 0, b > 0 => 0 ?= c - return T::zero().cmp(&c); + let zero = T::zero(); + let (mut a, mut b) = lhs.clone().into_raw(); + let (mut c, mut d) = rhs.clone().into_raw(); + let mut q_n; + let mut q_d; + + loop { + (q_n, a) = a.div_rem(&c); // 0 < c < a ∴ q_n ≥ 1 + (q_d, b) = b.div_rem(&d); // 0 < d < b ∴ q_d ≥ 1 + match q_n.cmp(&q_d) { // a/c ?= b/d + Ordering::Equal => (), + ord => return ord, + } + if a == zero { + // a*d ?= b*c, a = 0, 0 < c => 0 ?= b + return zero.cmp(&b); + } + if b == zero { + // 0 < a, b = 0, 0 < d ∴ a*d > 0 = b*c + return Ordering::Greater; + } + (q_n, c) = c.div_rem(&a); // 0 < a < c ∴ q_n ≥ 1 + (q_d, d) = d.div_rem(&b); // 0 < b < d ∴ q_d ≥ 1 + match q_d.cmp(&q_n) { // d/b ?= c/a + Ordering::Equal => (), + ord => return ord, + } + if c == zero { + // a*d ?= b*c, 0 < a, c = 0 => d ?= 0 + return d.cmp(&zero); + } + if d == zero { + // 0 < b, 0 < c, d = 0 ∴ a*d = 0 < b*c + return Ordering::Less; } } } + + match self.numer.cmp(&zero) { + Ordering::Greater => { + if other.numer <= zero { + return Ordering::Greater; + } + match self.numer.cmp(&other.numer) { + Ordering::Greater => reduce(self, other), + Ordering::Less => reduce(other, self).reverse(), + Ordering::Equal => other.denom.cmp(&self.denom), + } + }, + Ordering::Less => { + if zero <= other.numer { + return Ordering::Less; + } + match self.numer.cmp(&other.numer) { + Ordering::Greater => reduce(other, self), + Ordering::Less => reduce(self, other).reverse(), + Ordering::Equal => self.denom.cmp(&other.denom) + } + }, + Ordering::Equal => zero.cmp(&other.numer) + } + } } From 9f6dac3edf59d9c75709570d572e01f86fa3d7b1 Mon Sep 17 00:00:00 2001 From: Mike Maley Date: Sat, 21 Feb 2026 23:49:48 -0500 Subject: [PATCH 3/3] Optimized Ord::cmp Reduced the need to clone values --- src/lib.rs | 81 +++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 44 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 05ec30d..7da1519 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -341,27 +341,21 @@ where impl Ord for Ratio { fn cmp(&self, other: &Self) -> cmp::Ordering { - // self ?= other, returns the ordering where the ordering '?=' - // is either '<', '>', or '=='. use cmp::Ordering; let zero = T::zero(); - // makes self.denom > 0, and checks other.denom > 0 as a side effect - if self.denom <= zero { - if self.denom == zero { - panic!("denominator == 0"); - } - return (other).cmp(&Ratio::new_raw( - T::zero() - self.numer.clone(), - T::zero() - self.denom.clone(), - )).reverse(); + // makes self.denom > 0 and other.denom > 0 + if self.denom <= zero || other.denom <= zero { + return self.reduced().cmp(&other.reduced()); } + // return an `Ordering` for the following case // let a/b = lhs, c/d = rhs // 0 < c < a and 0 < b and 0 < d - fn reduce( + #[inline] + fn reduce( lhs: &Ratio, rhs: &Ratio ) -> Ordering { @@ -370,41 +364,41 @@ impl Ord for Ratio { } let zero = T::zero(); - let (mut a, mut b) = lhs.clone().into_raw(); - let (mut c, mut d) = rhs.clone().into_raw(); - let mut q_n; - let mut q_d; - - loop { - (q_n, a) = a.div_rem(&c); // 0 < c < a ∴ q_n ≥ 1 - (q_d, b) = b.div_rem(&d); // 0 < d < b ∴ q_d ≥ 1 - match q_n.cmp(&q_d) { // a/c ?= b/d - Ordering::Equal => (), - ord => return ord, - } - if a == zero { - // a*d ?= b*c, a = 0, 0 < c => 0 ?= b - return zero.cmp(&b); - } - if b == zero { - // 0 < a, b = 0, 0 < d ∴ a*d > 0 = b*c - return Ordering::Greater; + + let (q_n, mut a) = lhs.numer.div_rem(&rhs.numer); // 0 < c < a ∴ q_n ≥ 1 + let (q_d, mut b) = lhs.denom.div_rem(&rhs.denom); // 0 < d < b ∴ q_d ≥ 1 + let mut ord = q_n.cmp(&q_d); + if ord.is_ne() { + return ord; + } + match (a == zero, b == zero) { + // a = 0, b = 0 ∴ a*d = 0 = b*c + (true, true) => return Ordering::Equal, + // 0 = a, 0 < b, 0 < c ∴ a*d = 0 < b*c + (true, false) => return Ordering::Less, + // 0 < a, b = 0, 0 < d ∴ a*d > 0 = b*c + (false, true) => return Ordering::Greater, + (false, false) => (), + } + let (mut q_n, mut c) = rhs.numer.div_rem(&a); // 0 < a < c ∴ q_n ≥ 1 + let (mut q_d, mut d) = rhs.denom.div_rem(&b); // 0 < b < d ∴ q_d ≥ 1 + ord = q_d.cmp(&q_n); // d/b ?= c/a + while ord.is_eq() { + match (c == zero, d == zero) { + // c = 0, d = 0 ∴ a*d = 0 = b*c + (true, true) => return Ordering::Equal, + // 0 < a, c = 0, 0 < d ∴ a*d > 0 = b*c + (true, false) => return Ordering::Greater, + // 0 < b, 0 < c, d = 0 ∴ a*d = 0 < b*c + (false, true) => return Ordering::Less, + (false, false) => (), } + (a, b, c, d) = (d, c, b, a); (q_n, c) = c.div_rem(&a); // 0 < a < c ∴ q_n ≥ 1 (q_d, d) = d.div_rem(&b); // 0 < b < d ∴ q_d ≥ 1 - match q_d.cmp(&q_n) { // d/b ?= c/a - Ordering::Equal => (), - ord => return ord, - } - if c == zero { - // a*d ?= b*c, 0 < a, c = 0 => d ?= 0 - return d.cmp(&zero); - } - if d == zero { - // 0 < b, 0 < c, d = 0 ∴ a*d = 0 < b*c - return Ordering::Less; - } + ord = q_d.cmp(&q_n); // d/b ?= c/a } + ord } match self.numer.cmp(&zero) { @@ -430,7 +424,6 @@ impl Ord for Ratio { }, Ordering::Equal => zero.cmp(&other.numer) } - } }