perf: Switch to &'static str by default

Originally, clap carried a lifetime parameter.  When moving away from
that, we took the approach that dynamically generated strings are always
supported and `&'static str` was just an optimization.

The problem is the code size increase from this is dramatic.  So we're
taking the opposite approach and making dynamic formatting opt-in under
the `string` feature flag.  When deciding on an implementation, I
favored the faster one rather than the one with smaller code size since
small code size can be gotten through other means.

Before: 567.2 KiB, 15.975 µs
After: 541.1 KiB, 9.7855 µs
With `string`: 576.6 KiB, 13.016 µs
This commit is contained in:
Ed Page 2022-09-16 11:52:55 -05:00
parent fe43f0c945
commit c165b601ac
11 changed files with 54 additions and 50 deletions

View file

@ -60,7 +60,7 @@ default = [
"suggestions",
]
debug = ["clap_derive/debug", "dep:backtrace"] # Enables debug messages
unstable-doc = ["derive", "cargo", "wrap_help", "env", "unicode", "perf", "unstable-replace", "unstable-grouped"] # for docs.rs
unstable-doc = ["derive", "cargo", "wrap_help", "env", "unicode", "string", "unstable-replace", "unstable-grouped"] # for docs.rs
# Used in default
std = [] # support for no_std in a backwards-compatible way
@ -74,7 +74,7 @@ cargo = ["dep:once_cell"] # Disable if you're not using Cargo, enables Cargo-env
wrap_help = ["dep:terminal_size"]
env = [] # Use environment variables during arg parsing
unicode = ["dep:unicode-width", "dep:unicase"] # Support for unicode characters in arguments and help messages
perf = [] # Optimize for runtime performance
string = [] # Allow runtime generated strings
# In-work features
unstable-replace = []

View file

@ -15,8 +15,8 @@ MSRV?=1.60.0
_FEATURES = minimal default wasm full debug release
_FEATURES_minimal = --no-default-features --features "std"
_FEATURES_default =
_FEATURES_wasm = --features "deprecated derive cargo env unicode perf unstable-replace unstable-grouped"
_FEATURES_full = --features "deprecated derive cargo env unicode perf unstable-replace unstable-grouped wrap_help"
_FEATURES_wasm = --features "deprecated derive cargo env unicode string unstable-replace unstable-grouped"
_FEATURES_full = --features "deprecated derive cargo env unicode string unstable-replace unstable-grouped wrap_help"
_FEATURES_next = ${_FEATURES_full} --features unstable-v5
_FEATURES_debug = ${_FEATURES_full} --features debug --features clap_complete/debug
_FEATURES_release = ${_FEATURES_full} --release

View file

@ -170,7 +170,7 @@ pub fn generate_to<G, S, T>(
) -> Result<PathBuf, Error>
where
G: Generator,
S: Into<clap::builder::Str>,
S: Into<String>,
T: Into<OsString>,
{
cmd.set_bin_name(bin_name);
@ -223,7 +223,7 @@ where
pub fn generate<G, S>(gen: G, cmd: &mut clap::Command, bin_name: S, buf: &mut dyn Write)
where
G: Generator,
S: Into<clap::builder::Str>,
S: Into<String>,
{
cmd.set_bin_name(bin_name);
_generate::<G, S>(gen, cmd, buf)
@ -232,7 +232,7 @@ where
fn _generate<G, S>(gen: G, cmd: &mut clap::Command, buf: &mut dyn Write)
where
G: Generator,
S: Into<clap::builder::Str>,
S: Into<String>,
{
cmd.build();

View file

@ -16,7 +16,7 @@
//! * **env**: Turns on the usage of environment variables during parsing.
//! * **unicode**: Turns on support for unicode characters (including emoji) in arguments and help messages.
//! * **wrap_help**: Turns on the help text wrapping feature, based on the terminal size.
//! * **perf**: Optimized for runtime performance.
//! * **string**: Allow runtime generated strings
//!
//! #### Experimental features
//!

View file

@ -70,8 +70,8 @@ pub struct Command {
name: Str,
long_flag: Option<Str>,
short_flag: Option<char>,
display_name: Option<Str>,
bin_name: Option<Str>,
display_name: Option<String>,
bin_name: Option<String>,
author: Option<Str>,
version: Option<Str>,
long_version: Option<Str>,
@ -695,7 +695,7 @@ impl Command {
if let Some(f) = p.file_name() {
if let Some(s) = f.to_str() {
if self.bin_name.is_none() {
self.bin_name = Some(Str::from(s.to_owned()));
self.bin_name = Some(s.to_owned());
}
}
}
@ -1376,7 +1376,7 @@ impl Command {
/// # ;
/// ```
#[must_use]
pub fn bin_name(mut self, name: impl IntoResettable<Str>) -> Self {
pub fn bin_name(mut self, name: impl IntoResettable<String>) -> Self {
self.bin_name = name.into_resettable().into_option();
self
}
@ -1392,7 +1392,7 @@ impl Command {
/// # ;
/// ```
#[must_use]
pub fn display_name(mut self, name: impl IntoResettable<Str>) -> Self {
pub fn display_name(mut self, name: impl IntoResettable<String>) -> Self {
self.display_name = name.into_resettable().into_option();
self
}
@ -3156,7 +3156,7 @@ impl Command {
}
/// Set binary name. Uses `&mut self` instead of `self`.
pub fn set_bin_name(&mut self, name: impl Into<Str>) {
pub fn set_bin_name(&mut self, name: impl Into<String>) {
self.bin_name = Some(name.into());
}
@ -3889,7 +3889,7 @@ impl Command {
"Command::_build_subcommand Setting bin_name of {} to {:?}",
sc.name, bin_name
);
sc.bin_name = Some(Str::from(bin_name));
sc.bin_name = Some(bin_name);
if sc.display_name.is_none() {
let self_display_name = if is_multicall_set {
@ -3911,7 +3911,7 @@ impl Command {
"Command::_build_subcommand Setting display_name of {} to {:?}",
sc.name, display_name
);
sc.display_name = Some(Str::from(display_name));
sc.display_name = Some(display_name);
}
// Ensure all args are built and ready to parse
@ -3990,7 +3990,7 @@ impl Command {
"Command::_build_bin_names:iter: Setting bin_name of {} to {:?}",
sc.name, bin_name
);
sc.bin_name = Some(Str::from(bin_name));
sc.bin_name = Some(bin_name);
} else {
debug!(
"Command::_build_bin_names::iter: Using existing bin_name of {} ({:?})",
@ -4018,7 +4018,7 @@ impl Command {
"Command::_build_bin_names:iter: Setting display_name of {} to {:?}",
sc.name, display_name
);
sc.display_name = Some(Str::from(display_name));
sc.display_name = Some(display_name);
} else {
debug!(
"Command::_build_bin_names::iter: Using existing display_name of {} ({:?})",

View file

@ -7,12 +7,14 @@ pub struct OsStr {
}
impl OsStr {
#[cfg(feature = "string")]
pub(crate) fn from_string(name: std::ffi::OsString) -> Self {
Self {
name: Inner::from_string(name),
}
}
#[cfg(feature = "string")]
pub(crate) fn from_ref(name: &std::ffi::OsStr) -> Self {
Self {
name: Inner::from_ref(name),
@ -42,7 +44,7 @@ impl From<&'_ OsStr> for OsStr {
}
}
#[cfg(feature = "perf")]
#[cfg(feature = "string")]
impl From<Str> for OsStr {
fn from(id: Str) -> Self {
match id.into_inner() {
@ -52,10 +54,10 @@ impl From<Str> for OsStr {
}
}
#[cfg(not(feature = "perf"))]
#[cfg(not(feature = "string"))]
impl From<Str> for OsStr {
fn from(id: Str) -> Self {
Self::from_ref(std::ffi::OsStr::new(id.as_str()))
Self::from_static_ref(std::ffi::OsStr::new(id.into_inner().0))
}
}
@ -69,31 +71,34 @@ impl From<&'_ Str> for OsStr {
}
}
#[cfg(not(feature = "perf"))]
impl From<&'_ Str> for OsStr {
fn from(id: &'_ Str) -> Self {
Self::from_ref(std::ffi::OsStr::new(id.as_str()))
id.clone().into()
}
}
#[cfg(feature = "string")]
impl From<std::ffi::OsString> for OsStr {
fn from(name: std::ffi::OsString) -> Self {
Self::from_string(name)
}
}
#[cfg(feature = "string")]
impl From<&'_ std::ffi::OsString> for OsStr {
fn from(name: &'_ std::ffi::OsString) -> Self {
Self::from_ref(name.as_os_str())
}
}
#[cfg(feature = "string")]
impl From<std::string::String> for OsStr {
fn from(name: std::string::String) -> Self {
Self::from_string(name.into())
}
}
#[cfg(feature = "string")]
impl From<&'_ std::string::String> for OsStr {
fn from(name: &'_ std::string::String) -> Self {
Self::from_ref(name.as_str().as_ref())
@ -238,7 +243,7 @@ impl PartialEq<OsStr> for std::ffi::OsString {
}
}
#[cfg(feature = "perf")]
#[cfg(feature = "string")]
pub(crate) mod inner {
#[derive(Clone)]
pub(crate) enum Inner {
@ -272,26 +277,18 @@ pub(crate) mod inner {
}
}
#[cfg(not(feature = "perf"))]
#[cfg(not(feature = "string"))]
pub(crate) mod inner {
#[derive(Clone)]
pub(crate) struct Inner(Box<std::ffi::OsStr>);
pub(crate) struct Inner(&'static std::ffi::OsStr);
impl Inner {
pub(crate) fn from_string(name: std::ffi::OsString) -> Self {
Self(name.into_boxed_os_str())
}
pub(crate) fn from_ref(name: &std::ffi::OsStr) -> Self {
Self(Box::from(name))
}
pub(crate) fn from_static_ref(name: &'static std::ffi::OsStr) -> Self {
Self::from_ref(name)
Self(name)
}
pub(crate) fn as_os_str(&self) -> &std::ffi::OsStr {
&self.0
self.0
}
pub(crate) fn into_os_string(self) -> std::ffi::OsString {

View file

@ -162,6 +162,12 @@ impl<I: Into<ValueParser>> IntoResettable<ValueParser> for I {
}
}
impl<I: Into<String>> IntoResettable<String> for I {
fn into_resettable(self) -> Resettable<String> {
Resettable::Value(self.into())
}
}
impl<I: Into<StyledStr>> IntoResettable<StyledStr> for I {
fn into_resettable(self) -> Resettable<StyledStr> {
Resettable::Value(self.into())

View file

@ -5,12 +5,14 @@ pub struct Str {
}
impl Str {
#[cfg(feature = "string")]
pub(crate) fn from_string(name: std::string::String) -> Self {
Self {
name: Inner::from_string(name),
}
}
#[cfg(feature = "string")]
pub(crate) fn from_ref(name: &str) -> Self {
Self {
name: Inner::from_ref(name),
@ -23,7 +25,6 @@ impl Str {
}
}
#[cfg(feature = "perf")]
pub(crate) fn into_inner(self) -> Inner {
self.name
}
@ -40,12 +41,14 @@ impl From<&'_ Str> for Str {
}
}
#[cfg(feature = "string")]
impl From<std::string::String> for Str {
fn from(name: std::string::String) -> Self {
Self::from_string(name)
}
}
#[cfg(feature = "string")]
impl From<&'_ std::string::String> for Str {
fn from(name: &'_ std::string::String) -> Self {
Self::from_ref(name.as_str())
@ -211,7 +214,7 @@ impl PartialEq<Str> for std::string::String {
}
}
#[cfg(feature = "perf")]
#[cfg(feature = "string")]
pub(crate) mod inner {
#[derive(Clone)]
pub(crate) enum Inner {
@ -245,26 +248,18 @@ pub(crate) mod inner {
}
}
#[cfg(not(feature = "perf"))]
#[cfg(not(feature = "string"))]
pub(crate) mod inner {
#[derive(Clone)]
pub(crate) struct Inner(Box<str>);
pub(crate) struct Inner(pub(crate) &'static str);
impl Inner {
pub(crate) fn from_string(name: std::string::String) -> Self {
Self(name.into_boxed_str())
}
pub(crate) fn from_ref(name: &str) -> Self {
Self(Box::from(name))
}
pub(crate) fn from_static_ref(name: &'static str) -> Self {
Self::from_ref(name)
Self(name)
}
pub(crate) fn as_str(&self) -> &str {
&self.0
self.0
}
pub(crate) fn into_string(self) -> String {

View file

@ -45,12 +45,14 @@ impl From<&'_ Str> for Id {
}
}
#[cfg(feature = "string")]
impl From<std::string::String> for Id {
fn from(name: std::string::String) -> Self {
Self(name.into())
}
}
#[cfg(feature = "string")]
impl From<&'_ std::string::String> for Id {
fn from(name: &'_ std::string::String) -> Self {
Self(name.into())

View file

@ -18,6 +18,8 @@ fn example_tests() {
"suggestions",
#[cfg(feature = "unicode")]
"unicode",
#[cfg(feature = "string")]
"string",
#[cfg(feature = "wrap_help")]
"wrap_help",
#[cfg(feature = "unstable-replace")]

View file

@ -18,6 +18,8 @@ fn ui_tests() {
"suggestions",
#[cfg(feature = "unicode")]
"unicode",
#[cfg(feature = "string")]
"string",
#[cfg(feature = "wrap_help")]
"wrap_help",
#[cfg(feature = "unstable-replace")]