From 3743a3e1e7a87bdd0ea7b923dc42386bfc14ccd5 Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 29 Jul 2020 20:27:54 +0200 Subject: [PATCH] factor: Derecursify and refactor ~7% slowdown, paves the way for upcoming improvements --- src/uu/factor/src/factor.rs | 87 ++++++++++++++++++++++--------------- src/uu/factor/src/table.rs | 5 ++- 2 files changed, 54 insertions(+), 38 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 4e5322084c..dfe6e9f1ad 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -33,7 +33,14 @@ impl Decomposition { } } - #[cfg(test)] + fn is_one(&self) -> bool { + self.0.is_empty() + } + + fn pop(&mut self) -> Option<(u64, Exponent)> { + self.0.pop() + } + fn product(&self) -> u64 { self.0 .iter() @@ -77,11 +84,11 @@ impl Factors { self.0.borrow_mut().add(prime, exp) } + #[cfg(test)] pub fn push(&mut self, prime: u64) { self.add(prime, 1) } - #[cfg(test)] fn product(&self) -> u64 { self.0.borrow().product() } @@ -102,62 +109,70 @@ impl fmt::Display for Factors { } } -fn _factor(num: u64, f: Factors) -> Factors { +fn _find_factor(num: u64) -> Option { use miller_rabin::Result::*; - // Shadow the name, so the recursion automatically goes from “Big” arithmetic to small. - let _factor = |n, f| { - if n < (1 << 32) { - _factor::>(n, f) - } else { - _factor::(n, f) - } - }; - - if num == 1 { - return f; - } - let n = A::new(num); - let divisor = match miller_rabin::test::(n) { - Prime => { - let mut r = f; - r.push(num); - return r; - } - - Composite(d) => d, - Pseudoprime => rho::find_divisor::(n), - }; + match miller_rabin::test::(n) { + Prime => None, + Composite(d) => Some(d), + Pseudoprime => Some(rho::find_divisor::(n)), + } +} - let f = _factor(divisor, f); - _factor(num / divisor, f) +fn find_factor(num: u64) -> Option { + if num < (1 << 32) { + _find_factor::>(num) + } else { + _find_factor::>(num) + } } -pub fn factor(mut n: u64) -> Factors { +pub fn factor(num: u64) -> Factors { let mut factors = Factors::one(); - if n < 2 { + if num < 2 { return factors; } - let z = n.trailing_zeros(); + let mut n = num; + let z = num.trailing_zeros(); if z > 0 { factors.add(2, z as Exponent); n >>= z; } + debug_assert_eq!(num, n * factors.product()); if n == 1 { return factors; } - let (factors, n) = table::factor(n, factors); + table::factor(&mut n, &mut factors); + debug_assert_eq!(num, n * factors.product()); - if n < (1 << 32) { - _factor::>(n, factors) - } else { - _factor::>(n, factors) + if n == 1 { + return factors; + } + + let mut dec = Decomposition::one(); + dec.add(n, 1); + + while !dec.is_one() { + // Check correctness invariant + debug_assert_eq!(num, factors.product() * dec.product()); + + let (f, e) = dec.pop().unwrap(); + + if let Some(d) = find_factor(f) { + dec.add(f / d, e); + dec.add(d, e); + } else { + // f is prime + factors.add(f, e); + } } + + factors } #[cfg(test)] diff --git a/src/uu/factor/src/table.rs b/src/uu/factor/src/table.rs index d6ef796fc0..b62e801cbc 100644 --- a/src/uu/factor/src/table.rs +++ b/src/uu/factor/src/table.rs @@ -14,7 +14,8 @@ use crate::Factors; include!(concat!(env!("OUT_DIR"), "/prime_table.rs")); -pub(crate) fn factor(mut num: u64, mut factors: Factors) -> (Factors, u64) { +pub(crate) fn factor(n: &mut u64, factors: &mut Factors) { + let mut num = *n; for &(prime, inv, ceil) in P_INVS_U64 { if num == 1 { break; @@ -42,5 +43,5 @@ pub(crate) fn factor(mut num: u64, mut factors: Factors) -> (Factors, u64) { } } - (factors, num) + *n = num; }