Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Trim fat #1143

Merged
merged 10 commits into from
Jan 9, 2018
Merged

Trim fat #1143

merged 10 commits into from
Jan 9, 2018

Conversation

kbknapp
Copy link
Member

@kbknapp kbknapp commented Jan 9, 2018 •
edited
Loading

Pre commit, cago bloat --release --crates for ripgrep:

kevin@laptop: ~/Projects/ripgrep 
➜ cargo bloat --release --crates
    Updating registry `https://github.com/rust-lang/crates.io-index`
 Downloading clap v2.29.0                                                       
   Compiling clap v2.29.0                                                       
   Compiling ripgrep v0.7.1 (file:///home/kevin/Projects/ripgrep)
    Finished release [optimized + debuginfo] target(s) in 61.72 secs
 27.7% 574.3KiB [Unknown]
 27.3% 565.4KiB std
 22.0% 457.3KiB clap   <-------- HERE
  9.4% 196.0KiB regex
  3.4%  70.0KiB ignore
  3.1%  65.0KiB regex_syntax
  1.5%  32.1KiB encoding_rs
  1.5%  31.6KiB globset
  0.8%  16.0KiB aho_corasick
  0.7%  13.7KiB grep
  0.5%   9.5KiB crossbeam
  0.4%   9.0KiB walkdir
  0.2%   5.1KiB termcolor
  0.2%   4.9KiB textwrap
  0.2%   4.2KiB env_logger
  0.2%   3.3KiB thread_local
  0.2%   3.2KiB ansi_term
  0.1%   2.6KiB same_file
  0.1%   2.1KiB strsim
  0.1%   2.0KiB vec_map
100.0%   2.0MiB Total

Post commit:

kevin@laptop: ~/Projects/ripgrep 
➜ cargo bloat --release --crates
   Compiling clap v2.29.1 (file:///home/kevin/Projects/clap-rs)
   Compiling ripgrep v0.7.1 (file:///home/kevin/Projects/ripgrep)
    Finished release [optimized + debuginfo] target(s) in 23.1 secs
 31.8% 574.0KiB [Unknown]
 31.3% 564.9KiB std
 10.9% 196.0KiB regex
 10.5% 189.1KiB clap   <------ HERE
  3.9%  70.0KiB ignore
  3.6%  65.0KiB regex_syntax
  1.8%  32.1KiB encoding_rs
  1.7%  31.6KiB globset
  0.9%  16.0KiB aho_corasick
  0.8%  13.7KiB grep
  0.5%   9.5KiB crossbeam
  0.5%   9.0KiB walkdir
  0.3%   5.1KiB termcolor
  0.3%   4.9KiB textwrap
  0.2%   4.2KiB env_logger
  0.2%   3.3KiB thread_local
  0.2%   3.2KiB ansi_term
  0.1%   2.6KiB same_file
  0.1%   2.1KiB strsim
  0.1%   2.0KiB vec_map
100.0%   1.8MiB Total

This is also with no functionality lost and a slight perf increase. Nearly a 57% decrease in size. Wins all around 💯

This change is Reviewable

@kbknapp
chore: adds .idea project folder to ignore list
6f25b28
@kbknapp
tests: adds a check_output2 function to compare output to one of two …
79c3639 
…possible solutions
@kbknapp
fix: fixes the ripgrep benchmark by adding a value to a flag that exp…
d26ab2b 
…ects it
@kbknapp
perf: debloats clap by deduplicating logic and refactors
03e413d 
This commit removes heavy use of macros in certain functions which
drastically increased code size. Some of the macros could be turned
into functions, while others could be removed entirely.

Examples were removing arg_post_processing! which did things like
checked overrides, requirements, groups, etc. This would happen
after every argument was parsed. This macro also had several other
macros inside it, and would expand to several tens or hundreds of
lines of code.

Then add that due to borrowck and branch issues, this macro may be
included in multiple parts of a function. Unlike traditional functions
each of these uses expanded into TONS of code (just like agressive
inlining).

This commit primarily removes those arg_post_processing! calls and
breaks up the functionality into two types. The first must happen at
ever new argument (not new value, but new argument). This is pretty
cheap. The next type was moved to the end of parsing validation section
which is more expensive, but only happens once.

i.e. clap was validating each argument/value as it saw them, now it's
lazy and validates them all at once at the end. This MUCH more
efficient!
@kbknapp
tests: cleans up certain test cases litle by little
7ac5a5a
@kbknapp
perf: further debloating by removing generics from error cases
eb8d919
@mention-bot
Copy link

@kbknapp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kbknapp to be a potential reviewer.

This was referenced Jan 9, 2018
Closed
Closed
@kbknapp kbknapp merged commit d78341f into master Jan 9, 2018
@kbknapp kbknapp deleted the trim-fat branch January 15, 2018 17:01
@eddyb eddyb mentioned this pull request Feb 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kbknapp @mention-bot