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
This commit is contained in:
Kevin K 2017-02-28 12:43:24 -05:00
parent 0e1e6dced6
commit 24e3839220
No known key found for this signature in database
GPG key ID: 17218E4B3692F01A
4 changed files with 27 additions and 88 deletions

View file

@ -1634,7 +1634,7 @@ impl<'a, 'b> Parser<'a, 'b>
where A: AnyArg<'a, 'b> + Display where A: AnyArg<'a, 'b> + Display
{ {
debugln!("Parser::validate_values: arg={:?}", arg.name()); 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() { if self.is_set(AS::StrictUtf8) && val.to_str().is_none() {
debugln!("Parser::validate_values: invalid UTF-8 found in val {:?}", debugln!("Parser::validate_values: invalid UTF-8 found in val {:?}",
val); val);
@ -1838,14 +1838,11 @@ impl<'a, 'b> Parser<'a, 'b>
debugln!("Parser::validate_arg_num_vals: max_vals set...{}", num); debugln!("Parser::validate_arg_num_vals: max_vals set...{}", num);
if (ma.vals.len() as u64) > num { if (ma.vals.len() as u64) > num {
debugln!("Parser::validate_arg_num_vals: Sending error TooManyValues"); debugln!("Parser::validate_arg_num_vals: Sending error TooManyValues");
return Err(Error::too_many_values(ma.vals return Err(Error::too_many_values(ma.vals.iter()
.get(ma.vals .last()
.keys() .expect(INTERNAL_ERROR_MSG)
.last() .to_str()
.expect(INTERNAL_ERROR_MSG)) .expect(INVALID_UTF8),
.expect(INTERNAL_ERROR_MSG)
.to_str()
.expect(INVALID_UTF8),
a, a,
&*self.create_current_usage(matcher, None), &*self.create_current_usage(matcher, None),
self.color())); self.color()));
@ -1882,7 +1879,7 @@ impl<'a, 'b> Parser<'a, 'b>
if let Some(a_reqs) = a.requires() { if let Some(a_reqs) = a.requires() {
for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) { for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) {
if ma.vals if ma.vals
.values() .iter()
.any(|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name)) { .any(|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name)) {
return self.missing_required_error(matcher, None); return self.missing_required_error(matcher, None);
} }
@ -1942,8 +1939,8 @@ impl<'a, 'b> Parser<'a, 'b>
// Validate the conditionally required args // Validate the conditionally required args
for &(a, v, r) in &self.r_ifs { for &(a, v, r) in &self.r_ifs {
if let Some(ma) = matcher.get(a) { if let Some(ma) = matcher.get(a) {
for val in ma.vals.values() { if matcher.get(r).is_none() {
if v == val && matcher.get(r).is_none() { if ma.vals.iter().any(|val| val == v) {
return self.missing_required_error(matcher, Some(r)); 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() { for &(arg, val, default) in vm.values() {
let add = if let Some(a) = $m.get(arg) { let add = if let Some(a) = $m.get(arg) {
if let Some(v) = val { if let Some(v) = val {
a.vals.values().any(|value| v == value) a.vals.iter().any(|value| v == value)
} else { } else {
true true
} }

View file

@ -4,9 +4,6 @@ use std::ffi::OsStr;
use std::ops::Deref; use std::ops::Deref;
use std::mem; use std::mem;
// Third Party
use vec_map::VecMap;
// Internal // Internal
use args::{ArgMatches, MatchedArg, SubCommand}; use args::{ArgMatches, MatchedArg, SubCommand};
use args::AnyArg; use args::AnyArg;
@ -25,7 +22,7 @@ impl<'a> ArgMatcher<'a> {
pub fn propagate(&mut self, arg: &'a str) { pub fn propagate(&mut self, arg: &'a str) {
debugln!("ArgMatcher::propagate: arg={}", arg); 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() ma.vals.clone()
} else { } else {
debugln!("ArgMatcher::propagate: arg wasn't used"); 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 sma = (*sc).matches.args.entry(arg).or_insert_with(|| {
let mut gma = MatchedArg::new(); let mut gma = MatchedArg::new();
gma.occurs += 1; gma.occurs += 1;
for (i, v) in &vals { gma.vals = vals.clone();
gma.vals.insert(i, v.clone());
}
gma gma
}); });
if sma.vals.is_empty() { if sma.vals.is_empty() {
for (i, v) in &vals { sma.vals = vals.clone();
sma.vals.insert(i, v.clone());
}
} }
} }
let mut am = ArgMatcher(mem::replace(&mut sc.matches, ArgMatches::new())); 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) { pub fn add_val_to(&mut self, arg: &'a str, val: &OsStr) {
let ma = self.entry(arg).or_insert(MatchedArg { let ma = self.entry(arg).or_insert(MatchedArg {
occurs: 0, occurs: 0,
vals: VecMap::new(), vals: Vec::with_capacity(1),
}); });
let len = ma.vals.len() + 1; // let len = ma.vals.len() + 1;
ma.vals.insert(len, val.to_owned()); ma.vals.push(val.to_owned());
} }
pub fn needs_more_vals<'b, A>(&self, o: &A) -> bool pub fn needs_more_vals<'b, A>(&self, o: &A) -> bool

View file

@ -3,10 +3,7 @@ use std::borrow::Cow;
use std::collections::HashMap; use std::collections::HashMap;
use std::ffi::{OsStr, OsString}; use std::ffi::{OsStr, OsString};
use std::iter::Map; use std::iter::Map;
use std::slice; use std::slice::Iter;
// Third Party
use vec_map;
// Internal // Internal
use INVALID_UTF8; use INVALID_UTF8;
@ -113,7 +110,7 @@ impl<'a> ArgMatches<'a> {
/// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html
pub fn value_of<S: AsRef<str>>(&self, name: S) -> Option<&str> { pub fn value_of<S: AsRef<str>>(&self, name: S) -> Option<&str> {
if let Some(arg) = self.args.get(name.as_ref()) { 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)); 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 /// [`Arg::values_of_lossy`]: ./struct.ArgMatches.html#method.values_of_lossy
pub fn value_of_lossy<S: AsRef<str>>(&'a self, name: S) -> Option<Cow<'a, str>> { pub fn value_of_lossy<S: AsRef<str>>(&'a self, name: S) -> Option<Cow<'a, str>> {
if let Some(arg) = self.args.get(name.as_ref()) { 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()); return Some(v.to_string_lossy());
} }
} }
@ -182,7 +179,7 @@ impl<'a> ArgMatches<'a> {
pub fn value_of_os<S: AsRef<str>>(&self, name: S) -> Option<&OsStr> { pub fn value_of_os<S: AsRef<str>>(&self, name: S) -> Option<&OsStr> {
self.args self.args
.get(name.as_ref()) .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 /// 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()) { if let Some(arg) = self.args.get(name.as_ref()) {
fn to_str_slice(o: &OsString) -> &str { o.to_str().expect(INVALID_UTF8) } 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 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 None
} }
@ -246,7 +243,7 @@ impl<'a> ArgMatches<'a> {
pub fn values_of_lossy<S: AsRef<str>>(&'a self, name: S) -> Option<Vec<String>> { pub fn values_of_lossy<S: AsRef<str>>(&'a self, name: S) -> Option<Vec<String>> {
if let Some(arg) = self.args.get(name.as_ref()) { if let Some(arg) = self.args.get(name.as_ref()) {
return Some(arg.vals return Some(arg.vals
.values() .iter()
.map(|v| v.to_string_lossy().into_owned()) .map(|v| v.to_string_lossy().into_owned())
.collect()); .collect());
} }
@ -288,7 +285,7 @@ impl<'a> ArgMatches<'a> {
fn to_str_slice(o: &OsString) -> &OsStr { &*o } fn to_str_slice(o: &OsString) -> &OsStr { &*o }
let to_str_slice: fn(&'a OsString) -> &'a OsStr = to_str_slice; // coerce to fn pointer 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()) { 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 None
} }
@ -554,7 +551,7 @@ impl<'a> ArgMatches<'a> {
#[derive(Clone)] #[derive(Clone)]
#[allow(missing_debug_implementations)] #[allow(missing_debug_implementations)]
pub struct Values<'a> { pub struct Values<'a> {
iter: Map<vec_map::Values<'a, OsString>, fn(&'a OsString) -> &'a str>, iter: Map<Iter<'a, OsString>, fn(&'a OsString) -> &'a str>,
} }
impl<'a> Iterator for Values<'a> { impl<'a> Iterator for Values<'a> {
@ -570,51 +567,6 @@ impl<'a> DoubleEndedIterator for Values<'a> {
impl<'a> ExactSizeIterator 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<V>>,
}
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<usize>) { (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`] /// 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 /// method. Usage of this iterator allows values which contain invalid UTF-8 code points unlike
/// [`Values`]. /// [`Values`].
@ -639,7 +591,7 @@ impl<'a, V> DoubleEndedIterator for Iter<'a, V> {
#[derive(Clone)] #[derive(Clone)]
#[allow(missing_debug_implementations)] #[allow(missing_debug_implementations)]
pub struct OsValues<'a> { pub struct OsValues<'a> {
iter: Map<vec_map::Values<'a, OsString>, fn(&'a OsString) -> &'a OsStr>, iter: Map<Iter<'a, OsString>, fn(&'a OsString) -> &'a OsStr>,
} }
impl<'a> Iterator for OsValues<'a> { impl<'a> Iterator for OsValues<'a> {

View file

@ -1,23 +1,20 @@
// Std // Std
use std::ffi::OsString; use std::ffi::OsString;
// Third Party
use vec_map::VecMap;
#[doc(hidden)] #[doc(hidden)]
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct MatchedArg { pub struct MatchedArg {
#[doc(hidden)] #[doc(hidden)]
pub occurs: u64, pub occurs: u64,
#[doc(hidden)] #[doc(hidden)]
pub vals: VecMap<OsString>, pub vals: Vec<OsString>,
} }
impl Default for MatchedArg { impl Default for MatchedArg {
fn default() -> Self { fn default() -> Self {
MatchedArg { MatchedArg {
occurs: 1, occurs: 1,
vals: VecMap::new(), vals: Vec::with_capacity(1),
} }
} }
} }