From 24e383922016fb525227f93a8ad021fbe3a1d7db Mon Sep 17 00:00:00 2001 From: Kevin K Date: Tue, 28 Feb 2017 12:43:24 -0500 Subject: [PATCH] perf: changes internal use of VecMap to Vec for matched values of Args This makes a very big difference for CLIs that parse a large number of values (think ripgrep over a large directory). This commit improved the ripgrep parsing of ~2,000 values, simulating giving ripgrep a bunch of files. Parsing when from ~1,200,000 ns to ~400,000 ns! This was conducted a i7-5600U 2.6GHz --- src/app/parser.rs | 23 +++++++------- src/args/arg_matcher.rs | 19 ++++-------- src/args/arg_matches.rs | 66 ++++++----------------------------------- src/args/matched_arg.rs | 7 ++--- 4 files changed, 27 insertions(+), 88 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 1294df47..b66289a8 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -1634,7 +1634,7 @@ impl<'a, 'b> Parser<'a, 'b> where A: AnyArg<'a, 'b> + Display { debugln!("Parser::validate_values: arg={:?}", arg.name()); - for (_, val) in &ma.vals { + for val in &ma.vals { if self.is_set(AS::StrictUtf8) && val.to_str().is_none() { debugln!("Parser::validate_values: invalid UTF-8 found in val {:?}", val); @@ -1838,14 +1838,11 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::validate_arg_num_vals: max_vals set...{}", num); if (ma.vals.len() as u64) > num { debugln!("Parser::validate_arg_num_vals: Sending error TooManyValues"); - return Err(Error::too_many_values(ma.vals - .get(ma.vals - .keys() - .last() - .expect(INTERNAL_ERROR_MSG)) - .expect(INTERNAL_ERROR_MSG) - .to_str() - .expect(INVALID_UTF8), + return Err(Error::too_many_values(ma.vals.iter() + .last() + .expect(INTERNAL_ERROR_MSG) + .to_str() + .expect(INVALID_UTF8), a, &*self.create_current_usage(matcher, None), self.color())); @@ -1882,7 +1879,7 @@ impl<'a, 'b> Parser<'a, 'b> if let Some(a_reqs) = a.requires() { for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) { if ma.vals - .values() + .iter() .any(|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name)) { return self.missing_required_error(matcher, None); } @@ -1942,8 +1939,8 @@ impl<'a, 'b> Parser<'a, 'b> // Validate the conditionally required args for &(a, v, r) in &self.r_ifs { if let Some(ma) = matcher.get(a) { - for val in ma.vals.values() { - if v == val && matcher.get(r).is_none() { + if matcher.get(r).is_none() { + if ma.vals.iter().any(|val| val == v) { return self.missing_required_error(matcher, Some(r)); } } @@ -2196,7 +2193,7 @@ impl<'a, 'b> Parser<'a, 'b> for &(arg, val, default) in vm.values() { let add = if let Some(a) = $m.get(arg) { if let Some(v) = val { - a.vals.values().any(|value| v == value) + a.vals.iter().any(|value| v == value) } else { true } diff --git a/src/args/arg_matcher.rs b/src/args/arg_matcher.rs index 3052acd9..12e58fc1 100644 --- a/src/args/arg_matcher.rs +++ b/src/args/arg_matcher.rs @@ -4,9 +4,6 @@ use std::ffi::OsStr; use std::ops::Deref; use std::mem; -// Third Party -use vec_map::VecMap; - // Internal use args::{ArgMatches, MatchedArg, SubCommand}; use args::AnyArg; @@ -25,7 +22,7 @@ impl<'a> ArgMatcher<'a> { pub fn propagate(&mut self, arg: &'a str) { debugln!("ArgMatcher::propagate: arg={}", arg); - let vals: VecMap<_> = if let Some(ma) = self.get(arg) { + let vals: Vec<_> = if let Some(ma) = self.get(arg) { ma.vals.clone() } else { debugln!("ArgMatcher::propagate: arg wasn't used"); @@ -36,15 +33,11 @@ impl<'a> ArgMatcher<'a> { let sma = (*sc).matches.args.entry(arg).or_insert_with(|| { let mut gma = MatchedArg::new(); gma.occurs += 1; - for (i, v) in &vals { - gma.vals.insert(i, v.clone()); - } + gma.vals = vals.clone(); gma }); if sma.vals.is_empty() { - for (i, v) in &vals { - sma.vals.insert(i, v.clone()); - } + sma.vals = vals.clone(); } } let mut am = ArgMatcher(mem::replace(&mut sc.matches, ArgMatches::new())); @@ -105,10 +98,10 @@ impl<'a> ArgMatcher<'a> { pub fn add_val_to(&mut self, arg: &'a str, val: &OsStr) { let ma = self.entry(arg).or_insert(MatchedArg { occurs: 0, - vals: VecMap::new(), + vals: Vec::with_capacity(1), }); - let len = ma.vals.len() + 1; - ma.vals.insert(len, val.to_owned()); + // let len = ma.vals.len() + 1; + ma.vals.push(val.to_owned()); } pub fn needs_more_vals<'b, A>(&self, o: &A) -> bool diff --git a/src/args/arg_matches.rs b/src/args/arg_matches.rs index 12364fdc..3a60c773 100644 --- a/src/args/arg_matches.rs +++ b/src/args/arg_matches.rs @@ -3,10 +3,7 @@ use std::borrow::Cow; use std::collections::HashMap; use std::ffi::{OsStr, OsString}; use std::iter::Map; -use std::slice; - -// Third Party -use vec_map; +use std::slice::Iter; // Internal use INVALID_UTF8; @@ -113,7 +110,7 @@ impl<'a> ArgMatches<'a> { /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html pub fn value_of>(&self, name: S) -> Option<&str> { if let Some(arg) = self.args.get(name.as_ref()) { - if let Some(v) = arg.vals.values().nth(0) { + if let Some(v) = arg.vals.iter().nth(0) { return Some(v.to_str().expect(INVALID_UTF8)); } } @@ -145,7 +142,7 @@ impl<'a> ArgMatches<'a> { /// [`Arg::values_of_lossy`]: ./struct.ArgMatches.html#method.values_of_lossy pub fn value_of_lossy>(&'a self, name: S) -> Option> { if let Some(arg) = self.args.get(name.as_ref()) { - if let Some(v) = arg.vals.values().nth(0) { + if let Some(v) = arg.vals.iter().nth(0) { return Some(v.to_string_lossy()); } } @@ -182,7 +179,7 @@ impl<'a> ArgMatches<'a> { pub fn value_of_os>(&self, name: S) -> Option<&OsStr> { self.args .get(name.as_ref()) - .map_or(None, |arg| arg.vals.values().nth(0).map(|v| v.as_os_str())) + .map_or(None, |arg| arg.vals.iter().nth(0).map(|v| v.as_os_str())) } /// Gets a [`Values`] struct which implements [`Iterator`] for values of a specific argument @@ -214,7 +211,7 @@ impl<'a> ArgMatches<'a> { if let Some(arg) = self.args.get(name.as_ref()) { fn to_str_slice(o: &OsString) -> &str { o.to_str().expect(INVALID_UTF8) } let to_str_slice: fn(&OsString) -> &str = to_str_slice; // coerce to fn pointer - return Some(Values { iter: arg.vals.values().map(to_str_slice) }); + return Some(Values { iter: arg.vals.iter().map(to_str_slice) }); } None } @@ -246,7 +243,7 @@ impl<'a> ArgMatches<'a> { pub fn values_of_lossy>(&'a self, name: S) -> Option> { if let Some(arg) = self.args.get(name.as_ref()) { return Some(arg.vals - .values() + .iter() .map(|v| v.to_string_lossy().into_owned()) .collect()); } @@ -288,7 +285,7 @@ impl<'a> ArgMatches<'a> { fn to_str_slice(o: &OsString) -> &OsStr { &*o } let to_str_slice: fn(&'a OsString) -> &'a OsStr = to_str_slice; // coerce to fn pointer if let Some(arg) = self.args.get(name.as_ref()) { - return Some(OsValues { iter: arg.vals.values().map(to_str_slice) }); + return Some(OsValues { iter: arg.vals.iter().map(to_str_slice) }); } None } @@ -554,7 +551,7 @@ impl<'a> ArgMatches<'a> { #[derive(Clone)] #[allow(missing_debug_implementations)] pub struct Values<'a> { - iter: Map, fn(&'a OsString) -> &'a str>, + iter: Map, fn(&'a OsString) -> &'a str>, } impl<'a> Iterator for Values<'a> { @@ -570,51 +567,6 @@ impl<'a> DoubleEndedIterator for Values<'a> { impl<'a> ExactSizeIterator for Values<'a> {} -/// An iterator over the key-value pairs of a map. -#[derive(Clone)] -pub struct Iter<'a, V: 'a> { - front: usize, - back: usize, - iter: slice::Iter<'a, Option>, -} - -impl<'a, V> Iterator for Iter<'a, V> { - type Item = &'a V; - - #[inline] - fn next(&mut self) -> Option<&'a V> { - while self.front < self.back { - if let Some(elem) = self.iter.next() { - if let Some(x) = elem.as_ref() { - self.front += 1; - return Some(x); - } - } - self.front += 1; - } - None - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { (0, Some(self.back - self.front)) } -} - -impl<'a, V> DoubleEndedIterator for Iter<'a, V> { - #[inline] - fn next_back(&mut self) -> Option<&'a V> { - while self.front < self.back { - if let Some(elem) = self.iter.next_back() { - if let Some(x) = elem.as_ref() { - self.back -= 1; - return Some(x); - } - } - self.back -= 1; - } - None - } -} - /// An iterator for getting multiple values out of an argument via the [`ArgMatches::values_of_os`] /// method. Usage of this iterator allows values which contain invalid UTF-8 code points unlike /// [`Values`]. @@ -639,7 +591,7 @@ impl<'a, V> DoubleEndedIterator for Iter<'a, V> { #[derive(Clone)] #[allow(missing_debug_implementations)] pub struct OsValues<'a> { - iter: Map, fn(&'a OsString) -> &'a OsStr>, + iter: Map, fn(&'a OsString) -> &'a OsStr>, } impl<'a> Iterator for OsValues<'a> { diff --git a/src/args/matched_arg.rs b/src/args/matched_arg.rs index e852f26c..9a73af9d 100644 --- a/src/args/matched_arg.rs +++ b/src/args/matched_arg.rs @@ -1,23 +1,20 @@ // Std use std::ffi::OsString; -// Third Party -use vec_map::VecMap; - #[doc(hidden)] #[derive(Debug, Clone)] pub struct MatchedArg { #[doc(hidden)] pub occurs: u64, #[doc(hidden)] - pub vals: VecMap, + pub vals: Vec, } impl Default for MatchedArg { fn default() -> Self { MatchedArg { occurs: 1, - vals: VecMap::new(), + vals: Vec::with_capacity(1), } } }