From 2869248318a44db411c236058c8e332451ae60e9 Mon Sep 17 00:00:00 2001 From: nicoo Date: Sat, 30 May 2020 11:40:21 +0200 Subject: [PATCH 1/2] factor::Factors: Use a tree-based map internally This eliminate the need for sorting the prime factors for display. 25% performance improvement after the changes from factor/montgomery. --- src/uu/factor/src/factor.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 457bed6bb..4dd0fa5a7 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -11,7 +11,7 @@ extern crate rand; #[macro_use] extern crate uucore; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::fmt; use std::io::{stdin, BufRead}; use std::ops; @@ -27,12 +27,12 @@ static SUMMARY: &str = "Print the prime factors of the given number(s). static LONG_HELP: &str = ""; struct Factors { - f: HashMap, + f: BTreeMap, } impl Factors { fn new() -> Factors { - Factors { f: HashMap::new() } + Factors { f: BTreeMap::new() } } fn add(&mut self, prime: u64, exp: u8) { @@ -63,12 +63,8 @@ impl ops::MulAssign for Factors { impl fmt::Display for Factors { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // TODO: Use a representation with efficient in-order iteration - let mut primes: Vec<&u64> = self.f.keys().collect(); - primes.sort(); - - for p in primes { - for _ in 0..self.f[&p] { + for (p, exp) in self.f.iter() { + for _ in 0..*exp { write!(f, " {}", p)? } } From ef12991ee7afc48c573e0a9f5585aa6534b5d73c Mon Sep 17 00:00:00 2001 From: nicoo Date: Tue, 16 Jun 2020 15:06:28 +0200 Subject: [PATCH 2/2] factors: Avoid repeatedly locking and flushing stdout By default, stdout's LineWriter results one syscall per line, i.e. a billion syscalls when factoring a billion numbers... Buffering the output yields a ~28% speedup. --- src/uu/factor/src/factor.rs | 38 +++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 4dd0fa5a7..2445585fb 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -12,8 +12,9 @@ extern crate rand; extern crate uucore; use std::collections::BTreeMap; +use std::error::Error; use std::fmt; -use std::io::{stdin, BufRead}; +use std::io::{self, stdin, stdout, BufRead, Write}; use std::ops; mod miller_rabin; @@ -101,35 +102,40 @@ fn factor(mut n: u64) -> Factors { factors } -fn print_factors(num: u64) { - print!("{}:{}", num, factor(num)); - println!(); -} - -fn print_factors_str(num_str: &str) { - if let Err(e) = num_str.parse::().and_then(|x| { - print_factors(x); - Ok(()) - }) { - show_warning!("{}: {}", num_str, e); - } +fn print_factors_str(num_str: &str, w: &mut impl io::Write) -> Result<(), Box> { + num_str + .parse::() + .map_err(|e| e.into()) + .and_then(|x| writeln!(w, "{}:{}", x, factor(x)).map_err(|e| e.into())) } pub fn uumain(args: impl uucore::Args) -> i32 { let matches = app!(SYNTAX, SUMMARY, LONG_HELP).parse(args.collect_str()); + let stdout = stdout(); + let mut w = io::BufWriter::new(stdout.lock()); if matches.free.is_empty() { let stdin = stdin(); + for line in stdin.lock().lines() { for number in line.unwrap().split_whitespace() { - print_factors_str(number); + if let Err(e) = print_factors_str(number, &mut w) { + show_warning!("{}: {}", number, e); + } } } } else { - for num_str in &matches.free { - print_factors_str(num_str); + for number in &matches.free { + if let Err(e) = print_factors_str(number, &mut w) { + show_warning!("{}: {}", number, e); + } } } + + if let Err(e) = w.flush() { + show_error!("{}", e); + } + 0 }