From 45eab385b727fab8417669b75b0da76a8bae5006 Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 29 Jul 2020 19:39:50 +0200 Subject: [PATCH 1/4] factor: Introduce a type alias for exponents This way, we can easily replace u8 with a larger type when moving to support larger integers. --- src/uu/factor/src/factor.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index dca485f37c7..c4745bde89a 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -13,9 +13,11 @@ use std::fmt; use crate::numeric::{Arithmetic, Montgomery}; use crate::{miller_rabin, rho, table}; +type Exponent = u8; + #[derive(Clone, Debug, Eq, PartialEq)] pub struct Factors { - f: BTreeMap, + f: BTreeMap, } impl Factors { @@ -23,7 +25,7 @@ impl Factors { Factors { f: BTreeMap::new() } } - pub fn add(&mut self, prime: u64, exp: u8) { + pub fn add(&mut self, prime: u64, exp: Exponent) { debug_assert!(miller_rabin::is_prime(prime)); debug_assert!(exp > 0); let n = *self.f.get(&prime).unwrap_or(&0); @@ -95,7 +97,7 @@ pub fn factor(mut n: u64) -> Factors { let z = n.trailing_zeros(); if z > 0 { - factors.add(2, z as u8); + factors.add(2, z as Exponent); n >>= z; } From 436ce2db212beb943ddb961fdf534d00e01d094d Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 29 Jul 2020 19:47:44 +0200 Subject: [PATCH 2/4] factor::Factors: Split off a Decomposition type The new type can be used to represent in-progress factorisations, which contain non-prime factors. --- src/uu/factor/src/factor.rs | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index c4745bde89a..661967a62b3 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -16,20 +16,38 @@ use crate::{miller_rabin, rho, table}; type Exponent = u8; #[derive(Clone, Debug, Eq, PartialEq)] -pub struct Factors { - f: BTreeMap, +struct Decomposition(BTreeMap); + +impl Decomposition { + fn one() -> Decomposition { + Decomposition(BTreeMap::new()) + } + + fn add(&mut self, factor: u64, exp: Exponent) { + debug_assert!(exp > 0); + let n = *self.0.get(&factor).unwrap_or(&0); + self.0.insert(factor, exp + n); + } + + #[cfg(test)] + fn product(&self) -> u64 { + self.0 + .iter() + .fold(1, |acc, (p, exp)| acc * p.pow(*exp as u32)) + } } +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Factors(Decomposition); + impl Factors { pub fn one() -> Factors { - Factors { f: BTreeMap::new() } + Factors(Decomposition::one()) } pub fn add(&mut self, prime: u64, exp: Exponent) { debug_assert!(miller_rabin::is_prime(prime)); - debug_assert!(exp > 0); - let n = *self.f.get(&prime).unwrap_or(&0); - self.f.insert(prime, exp + n); + self.0.add(prime, exp) } pub fn push(&mut self, prime: u64) { @@ -38,15 +56,13 @@ impl Factors { #[cfg(test)] fn product(&self) -> u64 { - self.f - .iter() - .fold(1, |acc, (p, exp)| acc * p.pow(*exp as u32)) + self.0.product() } } impl fmt::Display for Factors { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for (p, exp) in self.f.iter() { + for (p, exp) in (self.0).0.iter() { for _ in 0..*exp { write!(f, " {}", p)? } From 4e3005a1b8efb47c7156d6ac53833814304c0fff Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 29 Jul 2020 20:38:07 +0200 Subject: [PATCH 3/4] factor::Decomposition: Use a flat vector representation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ~18% faster than BTreeMap, and ~5% faster than “master”. --- src/uu/factor/src/factor.rs | 46 ++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 661967a62b3..07cad84342d 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -7,7 +7,6 @@ extern crate rand; -use std::collections::BTreeMap; use std::fmt; use crate::numeric::{Arithmetic, Montgomery}; @@ -15,18 +14,22 @@ use crate::{miller_rabin, rho, table}; type Exponent = u8; -#[derive(Clone, Debug, Eq, PartialEq)] -struct Decomposition(BTreeMap); +#[derive(Clone, Debug)] +struct Decomposition(Vec<(u64, Exponent)>); impl Decomposition { fn one() -> Decomposition { - Decomposition(BTreeMap::new()) + Decomposition(Vec::new()) } fn add(&mut self, factor: u64, exp: Exponent) { debug_assert!(exp > 0); - let n = *self.0.get(&factor).unwrap_or(&0); - self.0.insert(factor, exp + n); + + if let Some((_, e)) = self.0.iter_mut().find(|(f, _)| *f == factor) { + *e += exp; + } else { + self.0.push((factor, exp)) + } } #[cfg(test)] @@ -35,7 +38,30 @@ impl Decomposition { .iter() .fold(1, |acc, (p, exp)| acc * p.pow(*exp as u32)) } + + fn get(&self, p: u64) -> Option<&(u64, u8)> { + self.0.iter().find(|(q, _)| *q == p) + } +} + +impl PartialEq for Decomposition { + fn eq(&self, other: &Decomposition) -> bool { + for p in &self.0 { + if other.get(p.0) != Some(p) { + return false; + } + } + + for p in &other.0 { + if self.get(p.0) != Some(p) { + return false; + } + } + + true + } } +impl Eq for Decomposition {} #[derive(Clone, Debug, Eq, PartialEq)] pub struct Factors(Decomposition); @@ -62,7 +88,10 @@ impl Factors { impl fmt::Display for Factors { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for (p, exp) in (self.0).0.iter() { + let mut v = (self.0).0.clone(); + v.sort_unstable(); + + for (p, exp) in v.iter() { for _ in 0..*exp { write!(f, " {}", p)? } @@ -168,7 +197,8 @@ mod tests { } fn recombines_factors(f: Factors) -> bool { - factor(f.product()) == f + assert_eq!(factor(f.product()), f); + true } } } From 287a8b71b1a6d88dd8c18587f34cbe457f97a5b0 Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 29 Jul 2020 20:45:45 +0200 Subject: [PATCH 4/4] factor::Factors: Use a RefCell rather than copy data when printing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ~2.9% faster than the previous commit, ~11% faster than “master” overall. --- src/uu/factor/src/factor.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 07cad84342d..4e5322084cb 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -7,6 +7,7 @@ extern crate rand; +use std::cell::RefCell; use std::fmt; use crate::numeric::{Arithmetic, Montgomery}; @@ -64,16 +65,16 @@ impl PartialEq for Decomposition { impl Eq for Decomposition {} #[derive(Clone, Debug, Eq, PartialEq)] -pub struct Factors(Decomposition); +pub struct Factors(RefCell); impl Factors { pub fn one() -> Factors { - Factors(Decomposition::one()) + Factors(RefCell::new(Decomposition::one())) } pub fn add(&mut self, prime: u64, exp: Exponent) { debug_assert!(miller_rabin::is_prime(prime)); - self.0.add(prime, exp) + self.0.borrow_mut().add(prime, exp) } pub fn push(&mut self, prime: u64) { @@ -82,13 +83,13 @@ impl Factors { #[cfg(test)] fn product(&self) -> u64 { - self.0.product() + self.0.borrow().product() } } impl fmt::Display for Factors { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut v = (self.0).0.clone(); + let v = &mut (self.0).borrow_mut().0; v.sort_unstable(); for (p, exp) in v.iter() {