Skip to content

perf: Switch to &'static str by default #4223

Merged
epage merged 6 commits into
clap-rs:masterfrom
epage:string
Sep 16, 2022
Merged

perf: Switch to &'static str by default #4223
epage merged 6 commits into
clap-rs:masterfrom
epage:string

Conversation

@epage

@epage epage commented Sep 16, 2022

Copy link
Copy Markdown
Member

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

The overall hope is to allow a `&'static str`-only version of clap, so
we need to move off of `Str` where dynamic strings are required.  We
need it here for subcommands due to external subcommands.

This had minimal impact on code size (567.2 to 567.5 KiB)
This is a more familiar type for people and it didn't cost us anything
(in fact it undid the code size change in the last commit).
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant