perf: vastly reduces the amount of cloning when adding non-global args minus when they're added from App::args which is forced to clone

This commit is contained in:
Kevin K 2017-02-27 23:37:25 -05:00
parent 677f323bce
commit 8da0303bc0
No known key found for this signature in database
GPG key ID: 17218E4B3692F01A
8 changed files with 129 additions and 51 deletions

View file

@ -6,7 +6,6 @@ mod meta;
mod help; mod help;
// Std // Std
use std::borrow::Borrow;
use std::env; use std::env;
use std::ffi::{OsStr, OsString}; use std::ffi::{OsStr, OsString};
use std::fmt; use std::fmt;
@ -639,8 +638,8 @@ impl<'a, 'b> App<'a, 'b> {
/// # ; /// # ;
/// ``` /// ```
/// [argument]: ./struct.Arg.html /// [argument]: ./struct.Arg.html
pub fn arg<A: Borrow<Arg<'a, 'b>> + 'a>(mut self, a: A) -> Self { pub fn arg<A: Into<Arg<'a, 'b>>>(mut self, a: A) -> Self {
self.p.add_arg(a.borrow()); self.p.add_arg(a.into());
self self
} }
@ -660,7 +659,7 @@ impl<'a, 'b> App<'a, 'b> {
/// [arguments]: ./struct.Arg.html /// [arguments]: ./struct.Arg.html
pub fn args(mut self, args: &[Arg<'a, 'b>]) -> Self { pub fn args(mut self, args: &[Arg<'a, 'b>]) -> Self {
for arg in args { for arg in args {
self.p.add_arg(arg); self.p.add_arg_ref(arg);
} }
self self
} }
@ -683,7 +682,7 @@ impl<'a, 'b> App<'a, 'b> {
/// [`Arg`]: ./struct.Arg.html /// [`Arg`]: ./struct.Arg.html
/// [`Arg::from_usage`]: ./struct.Arg.html#method.from_usage /// [`Arg::from_usage`]: ./struct.Arg.html#method.from_usage
pub fn arg_from_usage(mut self, usage: &'a str) -> Self { pub fn arg_from_usage(mut self, usage: &'a str) -> Self {
self.p.add_arg(&Arg::from_usage(usage)); self.p.add_arg(Arg::from_usage(usage));
self self
} }
@ -715,7 +714,7 @@ impl<'a, 'b> App<'a, 'b> {
if l.is_empty() { if l.is_empty() {
continue; continue;
} }
self.p.add_arg(&Arg::from_usage(l)); self.p.add_arg(Arg::from_usage(l));
} }
self self
} }

View file

@ -110,8 +110,8 @@ impl<'a, 'b> Parser<'a, 'b>
self.gen_completions_to(for_shell, &mut file) self.gen_completions_to(for_shell, &mut file)
} }
// actually adds the arguments #[inline]
pub fn add_arg(&mut self, a: &Arg<'a, 'b>) { fn debug_asserts(&self, a: &Arg) {
debug_assert!(!arg_names!(self).any(|name| name == a.b.name), debug_assert!(!arg_names!(self).any(|name| name == a.b.name),
format!("Non-unique argument name: {} is already in use", a.b.name)); format!("Non-unique argument name: {} is already in use", a.b.name));
if let Some(l) = a.s.long { if let Some(l) = a.s.long {
@ -124,11 +124,33 @@ impl<'a, 'b> Parser<'a, 'b>
format!("Argument short must be unique\n\n\t-{} is already in use", format!("Argument short must be unique\n\n\t-{} is already in use",
s)); s));
} }
let i = if a.index.is_none() {
(self.positionals.len() + 1)
} else {
a.index.unwrap() as usize
};
debug_assert!(!self.positionals.contains_key(i),
format!("Argument \"{}\" has the same index as another positional \
argument\n\n\tPerhaps try .multiple(true) to allow one positional argument \
to take multiple values",
a.b.name));
debug_assert!(!(a.is_set(ArgSettings::Required) && a.is_set(ArgSettings::Global)),
format!("Global arguments cannot be required.\n\n\t'{}' is marked as \
global and required",
a.b.name));
}
#[inline]
fn add_conditional_reqs(&mut self, a: &Arg<'a, 'b>) {
if let Some(ref r_ifs) = a.r_ifs { if let Some(ref r_ifs) = a.r_ifs {
for &(arg, val) in r_ifs { for &(arg, val) in r_ifs {
self.r_ifs.push((arg, val, a.b.name)); self.r_ifs.push((arg, val, a.b.name));
} }
} }
}
#[inline]
fn add_arg_groups(&mut self, a: &Arg<'a, 'b>) {
if let Some(ref grps) = a.b.groups { if let Some(ref grps) = a.b.groups {
for g in grps { for g in grps {
let mut found = false; let mut found = false;
@ -143,24 +165,64 @@ impl<'a, 'b> Parser<'a, 'b>
} }
} }
} }
}
#[inline]
fn add_reqs(&mut self, a: &Arg<'a, 'b>) {
if a.is_set(ArgSettings::Required) { if a.is_set(ArgSettings::Required) {
// If the arg is required, add all it's requirements to master required list
if let Some(ref areqs) = a.b.requires {
for name in areqs.iter().filter(|&&(val, _)| val.is_none()).map(|&(_, name)| name) {
self.required.push(name);
}
}
self.required.push(a.b.name); self.required.push(a.b.name);
} }
}
// actually adds the arguments
pub fn add_arg(&mut self, a: Arg<'a, 'b>) {
// if it's global we have to clone anyways
if a.is_set(ArgSettings::Global) {
return self.add_arg_ref(&a);
}
self.debug_asserts(&a);
self.add_conditional_reqs(&a);
self.add_arg_groups(&a);
self.add_reqs(&a);
if a.index.is_some() || (a.s.short.is_none() && a.s.long.is_none()) { if a.index.is_some() || (a.s.short.is_none() && a.s.long.is_none()) {
let i = if a.index.is_none() { let i = if a.index.is_none() {
(self.positionals.len() + 1) (self.positionals.len() + 1)
} else { } else {
a.index.unwrap() as usize a.index.unwrap() as usize
}; };
debug_assert!(!self.positionals.contains_key(i), self.positionals.insert(i, PosBuilder::from_arg(a, i as u64));
format!("Argument \"{}\" has the same index as another positional \ } else if a.is_set(ArgSettings::TakesValue) {
argument\n\n\tPerhaps try .multiple(true) to allow one positional argument \ let mut ob = OptBuilder::from(a);
to take multiple values", ob.s.unified_ord = self.flags.len() + self.opts.len();
a.b.name)); self.opts.push(ob);
let pb = PosBuilder::from_arg(a, i as u64, &mut self.required); } else {
let mut fb = FlagBuilder::from(a);
fb.s.unified_ord = self.flags.len() + self.opts.len();
self.flags.push(fb);
}
}
// actually adds the arguments but from a borrow (which means we have to do some clonine)
pub fn add_arg_ref(&mut self, a: &Arg<'a, 'b>) {
self.debug_asserts(&a);
self.add_conditional_reqs(&a);
self.add_arg_groups(&a);
self.add_reqs(&a);
if a.index.is_some() || (a.s.short.is_none() && a.s.long.is_none()) {
let i = if a.index.is_none() {
(self.positionals.len() + 1)
} else {
a.index.unwrap() as usize
};
let pb = PosBuilder::from_arg_ref(a, i as u64);
self.positionals.insert(i, pb); self.positionals.insert(i, pb);
} else if a.is_set(ArgSettings::TakesValue) { } else if a.is_set(ArgSettings::TakesValue) {
let mut ob = OptBuilder::from_arg(a, &mut self.required); let mut ob = OptBuilder::from(a);
ob.s.unified_ord = self.flags.len() + self.opts.len(); ob.s.unified_ord = self.flags.len() + self.opts.len();
self.opts.push(ob); self.opts.push(ob);
} else { } else {
@ -169,10 +231,6 @@ impl<'a, 'b> Parser<'a, 'b>
self.flags.push(fb); self.flags.push(fb);
} }
if a.is_set(ArgSettings::Global) { if a.is_set(ArgSettings::Global) {
debug_assert!(!a.is_set(ArgSettings::Required),
format!("Global arguments cannot be required.\n\n\t'{}' is marked as \
global and required",
a.b.name));
self.global_args.push(a.into()); self.global_args.push(a.into());
} }
} }
@ -589,7 +647,7 @@ impl<'a, 'b> Parser<'a, 'b>
// done and to recursively call this method // done and to recursively call this method
{ {
for a in &self.global_args { for a in &self.global_args {
sc.p.add_arg(a); sc.p.add_arg_ref(a);
} }
} }
sc.p.propogate_globals(); sc.p.propogate_globals();

View file

@ -1,3 +1,4 @@
use args::{ArgSettings, Arg, ArgFlags}; use args::{ArgSettings, Arg, ArgFlags};
#[derive(Debug, Clone, Default)] #[derive(Debug, Clone, Default)]

View file

@ -4,6 +4,7 @@ use std::fmt::{Display, Formatter, Result};
use std::rc::Rc; use std::rc::Rc;
use std::result::Result as StdResult; use std::result::Result as StdResult;
use std::ffi::{OsStr, OsString}; use std::ffi::{OsStr, OsString};
use std::mem;
// Third Party // Third Party
use vec_map::{self, VecMap}; use vec_map::{self, VecMap};
@ -27,8 +28,6 @@ impl<'n, 'e> FlagBuilder<'n, 'e> {
impl<'a, 'b, 'z> From<&'z Arg<'a, 'b>> for FlagBuilder<'a, 'b> { impl<'a, 'b, 'z> From<&'z Arg<'a, 'b>> for FlagBuilder<'a, 'b> {
fn from(a: &'z Arg<'a, 'b>) -> Self { fn from(a: &'z Arg<'a, 'b>) -> Self {
// No need to check for index() or takes_value() as that is handled above
FlagBuilder { FlagBuilder {
b: Base::from(a), b: Base::from(a),
s: Switched::from(a), s: Switched::from(a),
@ -36,6 +35,15 @@ impl<'a, 'b, 'z> From<&'z Arg<'a, 'b>> for FlagBuilder<'a, 'b> {
} }
} }
impl<'a, 'b> From<Arg<'a, 'b>> for FlagBuilder<'a, 'b> {
fn from(mut a: Arg<'a, 'b>) -> Self {
FlagBuilder {
b: mem::replace(&mut a.b, Base::default()),
s: mem::replace(&mut a.s, Switched::default()),
}
}
}
impl<'n, 'e> Display for FlagBuilder<'n, 'e> { impl<'n, 'e> Display for FlagBuilder<'n, 'e> {
fn fmt(&self, f: &mut Formatter) -> Result { fn fmt(&self, f: &mut Formatter) -> Result {
if let Some(l) = self.s.long { if let Some(l) = self.s.long {

View file

@ -3,6 +3,7 @@ use std::fmt::{Display, Formatter, Result};
use std::rc::Rc; use std::rc::Rc;
use std::result::Result as StdResult; use std::result::Result as StdResult;
use std::ffi::{OsStr, OsString}; use std::ffi::{OsStr, OsString};
use std::mem;
// Third Party // Third Party
use vec_map::{self, VecMap}; use vec_map::{self, VecMap};
@ -23,23 +24,26 @@ pub struct OptBuilder<'n, 'e>
impl<'n, 'e> OptBuilder<'n, 'e> { impl<'n, 'e> OptBuilder<'n, 'e> {
pub fn new(name: &'n str) -> Self { OptBuilder { b: Base::new(name), ..Default::default() } } pub fn new(name: &'n str) -> Self { OptBuilder { b: Base::new(name), ..Default::default() } }
}
pub fn from_arg(a: &Arg<'n, 'e>, reqs: &mut Vec<&'n str>) -> Self { impl<'n, 'e, 'z> From<&'z Arg<'n, 'e>> for OptBuilder<'n, 'e> {
// No need to check for .index() as that is handled above fn from(a: &'z Arg<'n, 'e>) -> Self {
let ob = OptBuilder { OptBuilder {
b: Base::from(a), b: Base::from(a),
s: Switched::from(a), s: Switched::from(a),
v: Valued::from(a), v: Valued::from(a),
};
// If the arg is required, add all it's requirements to master required list
if a.is_set(ArgSettings::Required) {
if let Some(ref areqs) = a.b.requires {
for r in areqs.iter().filter(|r| r.0.is_none()) {
reqs.push(r.1);
}
}
} }
ob }
}
impl<'n, 'e> From<Arg<'n, 'e>> for OptBuilder<'n, 'e> {
fn from(mut a: Arg<'n, 'e>) -> Self {
a.v.fill_in();
OptBuilder {
b: mem::replace(&mut a.b, Base::default()),
s: mem::replace(&mut a.s, Switched::default()),
v: mem::replace(&mut a.v, Valued::default()),
}
} }
} }

View file

@ -4,6 +4,7 @@ use std::fmt::{Display, Formatter, Result};
use std::rc::Rc; use std::rc::Rc;
use std::result::Result as StdResult; use std::result::Result as StdResult;
use std::ffi::{OsStr, OsString}; use std::ffi::{OsStr, OsString};
use std::mem;
// Third Party // Third Party
use vec_map::{self, VecMap}; use vec_map::{self, VecMap};
@ -32,10 +33,7 @@ impl<'n, 'e> PosBuilder<'n, 'e> {
} }
} }
pub fn from_arg(a: &Arg<'n, 'e>, idx: u64, reqs: &mut Vec<&'n str>) -> Self { pub fn from_arg_ref(a: &Arg<'n, 'e>, idx: u64) -> Self {
// Create the Positional Argument Builder with each HashSet = None to only
// allocate
// those that require it
let mut pb = PosBuilder { let mut pb = PosBuilder {
b: Base::from(a), b: Base::from(a),
v: Valued::from(a), v: Valued::from(a),
@ -45,17 +43,21 @@ impl<'n, 'e> PosBuilder<'n, 'e> {
(a.v.num_vals.is_some() && a.v.num_vals.unwrap() > 1) { (a.v.num_vals.is_some() && a.v.num_vals.unwrap() > 1) {
pb.b.settings.set(ArgSettings::Multiple); pb.b.settings.set(ArgSettings::Multiple);
} }
// If the arg is required, add all it's requirements to master required list
if a.is_set(ArgSettings::Required) {
if let Some(ref areqs) = a.b.requires {
for name in areqs.iter().filter(|&&(val, _)| val.is_none()).map(|&(_, name)| name) {
reqs.push(name);
}
}
}
pb pb
} }
pub fn from_arg(mut a: Arg<'n, 'e>, idx: u64) -> Self {
if a.v.max_vals.is_some() || a.v.min_vals.is_some() ||
(a.v.num_vals.is_some() && a.v.num_vals.unwrap() > 1) {
a.b.settings.set(ArgSettings::Multiple);
}
PosBuilder {
b: mem::replace(&mut a.b, Base::default()),
v: mem::replace(&mut a.v, Valued::default()),
index: idx,
}
}
pub fn multiple_str(&self) -> &str { pub fn multiple_str(&self) -> &str {
if self.b.settings.is_set(ArgSettings::Multiple) && self.v.val_names.is_none() { if self.b.settings.is_set(ArgSettings::Multiple) && self.v.val_names.is_none() {
"..." "..."

View file

@ -1,3 +1,4 @@
use Arg; use Arg;
#[derive(Debug)] #[derive(Debug)]

View file

@ -41,6 +41,16 @@ impl<'n, 'e> Default for Valued<'n, 'e> {
} }
} }
impl<'n, 'e> Valued<'n, 'e> {
pub fn fill_in(&mut self) {
if let Some(ref vec) = self.val_names {
if vec.len() > 1 {
self.num_vals = Some(vec.len() as u64);
}
}
}
}
impl<'n, 'e, 'z> From<&'z Arg<'n, 'e>> for Valued<'n, 'e> { impl<'n, 'e, 'z> From<&'z Arg<'n, 'e>> for Valued<'n, 'e> {
fn from(a: &'z Arg<'n, 'e>) -> Self { fn from(a: &'z Arg<'n, 'e>) -> Self {
let mut v = a.v.clone(); let mut v = a.v.clone();
@ -49,11 +59,6 @@ impl<'n, 'e, 'z> From<&'z Arg<'n, 'e>> for Valued<'n, 'e> {
v.num_vals = Some(vec.len() as u64); v.num_vals = Some(vec.len() as u64);
} }
} }
if let Some(ref vec) = a.v.val_names {
if vec.len() > 1 {
v.num_vals = Some(vec.len() as u64);
}
}
v v
} }
} }