Merge pull request #1554 from nbraud/factor/faster/montgomery32

factor: Refactor and improve performance (plus a few bug fixes)

src/uu/factor/Cargo.toml

@@ -11,12 +11,18 @@ keywords = ["coreutils", "uutils", "cross-platform", "cli", "utility"]
 categories = ["command-line-utilities"]
 edition = "2018"
 
+[build-dependencies]
+num-traits = "0.2" # used in src/numerics.rs, which is included by build.rs
+
+
 [dependencies]
+num-traits = "0.2"
 rand = "0.5"
 uucore = { version="0.0.4", package="uucore", git="https://github.com/uutils/uucore.git", branch="canary" }
 uucore_procs = { version="0.0.4", package="uucore_procs", git="https://github.com/uutils/uucore.git", branch="canary" }
 
 [dev-dependencies]
+paste = "0.1.18"
 quickcheck = "0.9.2"
 
 [[bin]]

src/uu/factor/build.rs

@@ -29,7 +29,7 @@ use miller_rabin::is_prime;
 
 #[path = "src/numeric.rs"]
 mod numeric;
-use numeric::inv_mod_u64;
+use numeric::modular_inverse;
 
 mod sieve;
 
@@ -57,7 +57,7 @@ fn main() {
     let mut x = primes.next().unwrap();
     for next in primes {
         // format the table
-        let outstr = format!("({}, {}, {}),", x, inv_mod_u64(x), std::u64::MAX / x);
+        let outstr = format!("({}, {}, {}),", x, modular_inverse(x), std::u64::MAX / x);
         if cols + outstr.len() > MAX_WIDTH {
             write!(file, "\n    {}", outstr).unwrap();
             cols = 4 + outstr.len();

src/uu/factor/src/factor.rs

@@ -54,12 +54,16 @@ impl fmt::Display for Factors {
     }
 }
 
-fn _factor<A: Arithmetic>(num: u64, f: Factors) -> Factors {
+fn _factor<A: Arithmetic + miller_rabin::Basis>(num: u64, f: Factors) -> Factors {
     use miller_rabin::Result::*;
+
     // Shadow the name, so the recursion automatically goes from “Big” arithmetic to small.
     let _factor = |n, f| {
-        // TODO: Optimise with 32 and 64b versions
-        _factor::<A>(n, f)
+        if n < (1 << 32) {
+            _factor::<Montgomery<u32>>(n, f)
+        } else {
+            _factor::<A>(n, f)
+        }
     };
 
     if num == 1 {
@@ -101,8 +105,11 @@ pub fn factor(mut n: u64) -> Factors {
 
     let (factors, n) = table::factor(n, factors);
 
-    // TODO: Optimise with 32 and 64b versions
-    _factor::<Montgomery>(n, factors)
+    if n < (1 << 32) {
+        _factor::<Montgomery<u32>>(n, factors)
+    } else {
+        _factor::<Montgomery<u64>>(n, factors)
+    }
 }
 
 #[cfg(test)]

src/uu/factor/src/miller_rabin.rs

@@ -2,10 +2,27 @@
 
 use crate::numeric::*;
 
-// Small set of bases for the Miller-Rabin prime test, valid for all 64b integers;
-//  discovered by Jim Sinclair on 2011-04-20, see miller-rabin.appspot.com
-#[allow(clippy::unreadable_literal)]
-const BASIS: [u64; 7] = [2, 325, 9375, 28178, 450775, 9780504, 1795265022];
+pub(crate) trait Basis {
+    const BASIS: &'static [u64];
+}
+
+impl Basis for Montgomery<u64> {
+    // Small set of bases for the Miller-Rabin prime test, valid for all 64b integers;
+    //  discovered by Jim Sinclair on 2011-04-20, see miller-rabin.appspot.com
+    #[allow(clippy::unreadable_literal)]
+    const BASIS: &'static [u64] = &[2, 325, 9375, 28178, 450775, 9780504, 1795265022];
+}
+
+impl Basis for Montgomery<u32> {
+    // Small set of bases for the Miller-Rabin prime test, valid for all 32b integers;
+    //  discovered by Steve Worley on 2013-05-27, see miller-rabin.appspot.com
+    #[allow(clippy::unreadable_literal)]
+    const BASIS: &'static [u64] = &[
+        4230279247111683200,
+        14694767155120705706,
+        16641139526367750375,
+    ];
+}
 
 #[derive(Eq, PartialEq)]
 pub(crate) enum Result {
@@ -23,16 +40,12 @@ impl Result {
 // Deterministic Miller-Rabin primality-checking algorithm, adapted to extract
 // (some) dividers; it will fail to factor strong pseudoprimes.
 #[allow(clippy::many_single_char_names)]
-pub(crate) fn test<A: Arithmetic>(m: A) -> Result {
+pub(crate) fn test<A: Arithmetic + Basis>(m: A) -> Result {
     use self::Result::*;
 
     let n = m.modulus();
-    if n < 2 {
-        return Pseudoprime;
-    }
-    if n % 2 == 0 {
-        return if n == 2 { Prime } else { Composite(2) };
-    }
+    debug_assert!(n > 1);
+    debug_assert!(n % 2 != 0);
 
     // n-1 = r 2ⁱ
     let i = (n - 1).trailing_zeros();
@@ -41,10 +54,10 @@ pub(crate) fn test<A: Arithmetic>(m: A) -> Result {
     let one = m.one();
     let minus_one = m.minus_one();
 
-    for _a in BASIS.iter() {
+    for _a in A::BASIS.iter() {
         let _a = _a % n;
         if _a == 0 {
-            break;
+            continue;
         }
 
         let a = m.from_u64(_a);
@@ -87,48 +100,113 @@ pub(crate) fn test<A: Arithmetic>(m: A) -> Result {
 // Used by build.rs' tests and debug assertions
 #[allow(dead_code)]
 pub(crate) fn is_prime(n: u64) -> bool {
-    if n % 2 == 0 {
+    if n < 2 {
+        false
+    } else if n % 2 == 0 {
         n == 2
     } else {
-        test::<Montgomery>(Montgomery::new(n)).is_prime()
+        test::<Montgomery<u64>>(Montgomery::new(n)).is_prime()
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::is_prime;
+    use super::*;
+    use crate::numeric::{Arithmetic, Montgomery};
+    use quickcheck::quickcheck;
+    use std::iter;
     const LARGEST_U64_PRIME: u64 = 0xFFFFFFFFFFFFFFC5;
 
+    fn primes() -> impl Iterator<Item = u64> {
+        iter::once(2).chain(odd_primes())
+    }
+
+    fn odd_primes() -> impl Iterator<Item = u64> {
+        use crate::table::{NEXT_PRIME, P_INVS_U64};
+        P_INVS_U64
+            .iter()
+            .map(|(p, _, _)| *p)
+            .chain(iter::once(NEXT_PRIME))
+    }
+
     #[test]
     fn largest_prime() {
         assert!(is_prime(LARGEST_U64_PRIME));
     }
 
     #[test]
-    fn first_primes() {
-        use crate::table::{NEXT_PRIME, P_INVS_U64};
-        for (p, _, _) in P_INVS_U64.iter() {
-            assert!(is_prime(*p), "{} reported composite", p);
+    fn largest_composites() {
+        for i in LARGEST_U64_PRIME + 1..=u64::MAX {
+            assert!(!is_prime(i), "2⁶⁴ - {} reported prime", u64::MAX - i + 1);
         }
-        assert!(is_prime(NEXT_PRIME));
     }
 
+    #[test]
+    fn two() {
+        assert!(is_prime(2));
+    }
+
+    // TODO: Deduplicate with macro in numeric.rs
+    macro_rules! parametrized_check {
+        ( $f:ident ) => {
+            paste::item! {
+                #[test]
+                fn [< $f _ u32 >]() {
+                    $f::<Montgomery<u32>>()
+                }
+                #[test]
+                fn [< $f _ u64 >]() {
+                    $f::<Montgomery<u64>>()
+                }
+            }
+        };
+    }
+
+    fn first_primes<A: Arithmetic + Basis>() {
+        for p in odd_primes() {
+            assert!(test(A::new(p)).is_prime(), "{} reported composite", p);
+        }
+    }
+    parametrized_check!(first_primes);
+
+    #[test]
+    fn one() {
+        assert!(!is_prime(1));
+    }
+    #[test]
+    fn zero() {
+        assert!(!is_prime(0));
+    }
+
+    fn first_composites<A: Arithmetic + Basis>() {
+        for (p, q) in primes().zip(odd_primes()) {
+            for i in p + 1..q {
+                assert!(!is_prime(i), "{} reported prime", i);
+            }
+        }
+    }
+    parametrized_check!(first_composites);
+
     #[test]
     fn issue_1556() {
         // 10 425 511 = 2441 × 4271
         assert!(!is_prime(10_425_511));
     }
 
-    #[test]
-    fn small_composites() {
-        use crate::table::P_INVS_U64;
-
-        for i in 0..P_INVS_U64.len() {
-            let (p, _, _) = P_INVS_U64[i];
-            for (q, _, _) in &P_INVS_U64[0..i] {
+    fn small_semiprimes<A: Arithmetic + Basis>() {
+        for p in odd_primes() {
+            for q in odd_primes().take_while(|q| *q <= p) {
                 let n = p * q;
-                assert!(!is_prime(n), "{} = {} × {} reported prime", n, p, q);
+                let m = A::new(n);
+                assert!(!test(m).is_prime(), "{} = {} × {} reported prime", n, p, q);
             }
         }
     }
+    parametrized_check!(small_semiprimes);
+
+    quickcheck! {
+        fn composites(i: u64, j: u64) -> bool {
+            i < 2 || j < 2 || !is_prime(i*j)
+        }
+    }
 }