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

shrink moonfire-nvr binary #70

Open
4 of 7 tasks
scottlamb opened this issue Jul 20, 2019 · 5 comments
Open
4 of 7 tasks

shrink moonfire-nvr binary #70

scottlamb opened this issue Jul 20, 2019 · 5 comments
Labels
enhancement rust Rust backend work required
Milestone
wishlist

Comments

@scottlamb
Copy link
Owner

scottlamb commented Jul 20, 2019 •
edited

There's some significant bloat in the binary right now:

  • debug symbols. These are handy, but they're enormous. This is the vast majority of the reason for the large binaries. (Unstripped binary is 150M. Stripped is 9.2M.) When I release, it'd be nice to do split these to a separate file as Ubuntu does. See: debuginfo: Add support for split-debuginfo on platforms that allow it rust-lang/rust#34651
  • protobuf. It's surprisingly large; maybe there's some low-hanging fruit or we could switch to prost.
  • regex. This was a convenient way of doing my parsing, but it's not strictly necessary, and cargo bloat says it's pretty large.
  • cursive. It's a lovely library, but long-term I want a web config UI (web configuration UI #35) instead.
  • duplicate crypto libraries: openssl and ring. libpasta uses ring iirc. I could just stop using openssl, although that might not be a good choice when I actually have built-in https (good Internet-facing webserver #27) [Currently I've eliminated openssl in the new-schema branch. It's not yet merged to master.]
  • two arguments parsing libraries: clap and docopt. I use docopt. prettydiff is pulling in clap, which at first glance seems silly. I might convert to clap anyway though so maybe there's no point in messing with prettydiff.
  • two versions of parking_lot. (tokio currently uses 0.7; I use 0.9 in moonfire-{nvr,db,base} and in mylog.) tokio is likely to update soon-ish, so I can probably just wait for this to be fixed.

These aren't causing material problems right now; the big binaries are just a bit embarrassing. So no particular size we need to get under or anything.

$ cargo bloat --release --crates
Compiling ...
Analyzing target/release/moonfire-nvr

File  .text     Size Crate
0.8%  21.0%   1.0MiB std
0.4%  12.0% 602.5KiB moonfire_db
0.4%  10.3% 521.0KiB protobuf
0.2%   5.7% 286.6KiB regex
0.2%   5.3% 268.5KiB moonfire_nvr
0.2%   4.8% 242.2KiB cursive
0.2%   4.3% 215.5KiB regex_syntax
0.1%   3.7% 187.3KiB h2
0.1%   3.0% 148.8KiB docopt
0.1%   2.4% 121.9KiB hyper
0.1%   2.3% 116.2KiB futures
...
@scottlamb scottlamb added enhancement rust Rust backend work required labels Jul 20, 2019
@scottlamb scottlamb added this to the wishlist milestone Jul 20, 2019
scottlamb added a commit that referenced this issue Mar 21, 2020
@scottlamb
use Blake3 instead of SHA-1 or Blake2b
0099173 
Benefits:

* Blake3 is faster. This is most noticeable for the hashing of the
  sample file data.
* we no longer need OpenSSL, which helps with shrinking the binary size
  (#70). sha1 basically forced OpenSSL usage; ring deliberately doesn't
  support this old algorithm, and the pure-Rust sha1 crate is painfully
  slow. OpenSSL might still be a better choice than ring/rustls for TLS
  but it's nice to have the option.

For the video sample entries, I decided we don't need to hash at all. I
think the id number is sufficiently stable, and it's okay---perhaps even
desirable---if an existing init segment changes for fixes like e5b83c2.
@scottlamb
Copy link
Owner Author

The split debug symbols (aka "fission") would currently break backtraces, unfortunately:

rust-lang/backtrace-rs#287

scottlamb added a commit that referenced this issue Apr 18, 2020
@scottlamb
switch from docopt to structopt
e8eb764 
A couple reasons for this:

* the docopt crate is "unlikely to see significant future evolution",
  and the wider docopt project is "mostly unmaintained at this point".
  clap/structopt is more full-featured, has more natural subcommand
  support, etc.

* it may allow me to shrink the binary (#70). This change alone seems
  to be a slight regression, but it's a step toward getting rid of
  regex, which is pretty large. And I feel less ridiculous now that I
  don't have two parsing crates anyway; prettydiff was pulling in
  structopt.

There are some behavior changes here:

* misc --help output changes and such as you'd expect from switching
  argument-parsing libraries

* I properly used PathBuf and OsString for stuff that theoretically
  could be non-UTF-8. I haven't tested that it actually made any
  difference. I'm also still storing the sample file dirname as "text"
  in the database to avoid causing a diff when not doing a schema
  change.
scottlamb added a commit that referenced this issue Apr 18, 2020
@scottlamb
replace regex use with nom
af9e568 
This reduces the binary size noticeably on my macOS machine (#70):

                             unstripped  stripped
1  before switching to clap    11.1 MiB   6.7 MiB
2  after switching to clap     11.4 MiB   6.9 MiB
3  without regex               10.1 MiB   5.9 MiB
@IronOxidizer
Copy link
Contributor

IronOxidizer commented Sep 7, 2020 •
edited

There are many (10+) duplicate packages with different versions. This is likely increasing the size to some degree. These can be reduced by using dependencies with matching dependency versions.

Here's a quick list:

ansi_term v0.11.0
ansi_term v0.9.0
error-chain v0.11.0
error-chain v0.12.2
proc-macro2 v0.4.30
proc-macro2 v1.0.10
quote v0.6.13
quote v1.0.3
rand v0.3.23
rand v0.4.6
rand v0.5.6
rand v0.7.3
rand_core v0.3.1
rand_core v0.4.2
rand_core v0.5.1
strsim v0.8.0
strsim v0.9.3
structopt v0.2.18
structopt v0.3.13
structopt-derive v0.2.18
structopt-derive v0.4.6
syn v0.15.44
syn v1.0.17
time v0.1.43
time v0.2.10
unicode-xid v0.1.0
unicode-xid v0.2.0

For the depedency graph, use cargo tree -d

@scottlamb
Copy link
Owner Author

Definitely not ideal, although I suspect none of the ones in that list are a major contribution to the binary size given the cargo bloat output.

@scottlamb
Copy link
Owner Author

scottlamb commented Oct 27, 2021 •
edited

As of 0.7.0, no duplicate packages!

[slamb@slamb-workstation ~/git/moonfire-nvr/server]$ cargo bloat --release --crates
    Finished release [optimized + debuginfo] target(s) in 0.05s
    Analyzing target/release/moonfire-nvr

File  .text     Size Crate
0.6%  18.7% 982.2KiB std
0.6%  16.8% 884.4KiB moonfire_db
0.3%  10.4% 545.1KiB protobuf
0.2%   6.4% 335.4KiB moonfire_nvr
0.2%   6.2% 328.0KiB clap
0.2%   4.7% 248.3KiB cursive_core
0.2%   4.5% 235.2KiB retina
0.1%   4.1% 213.7KiB tokio
0.1%   3.2% 170.8KiB hyper
0.1%   1.8%  93.0KiB backtrace
0.0%   1.1%  60.2KiB http
0.0%   1.1%  60.2KiB cursive
0.0%   1.0%  53.9KiB hashbrown
0.0%   1.0%  52.7KiB serde_json
0.0%   1.0%  51.3KiB rustc_demangle
0.0%   0.9%  47.6KiB url
0.0%   0.8%  42.9KiB miniz_oxide
0.0%   0.8%  40.2KiB addr2line
0.0%   0.7%  36.7KiB idna
0.0%   0.7%  35.3KiB sha2
0.4%  11.2% 589.7KiB And 88 more crates. Use -n N to show more.
3.4% 100.0%   5.1MiB .text section size, the file size is 152.2MiB

Note: numbers above are a result of guesswork. They are not 100% correct and never will be.

Biggest likely-fixable contributors that are still around: debug info (file size minus .text section is 147 MiB, so still the vast majority), protobuf, and cursive:

  • debug info: the ecosystem seems to be slowly making progress on split debug info working properly, so I'm just waiting it out.
  • protobuf: I might try switching to prost. I'm likely to stop using the protobuf text format stuff anyway in favor of JSON. I'll likely use JSON-format permissions for JSON API for configuration #153 and config file for more flexible http server setup #133.
  • cursive: I still intend to obsolete this in favor of a web configuration UI.

moonfire-db has grown, I think largely as a result of using JSON stuff in the schema (serde_json code is large). The v6->v7 schema upgrade funcs in particular are pretty large. Maybe eventually I can drop older schema upgrade code.

scottlamb added a commit that referenced this issue Oct 27, 2021
@scottlamb
use newer cargo version resolver
973d2fb 
This trims ~700KiB off moonfire-nvr's text section, much of it by
eliminating h2. See #70.
scottlamb added a commit that referenced this issue Jan 5, 2022
@scottlamb
reduce debug output in release/bench builds
0406e09 
This reduces the binary size from 154 MiB to 70 MiB (#70 progress).
Tools like `cargo flamegraph` still work fine.

As suggested by "EarthFeet" on reddit:
https://www.reddit.com/r/rust/comments/rw0jad/cargos_strip_profile_option_has_been_stabilized/hra193k/
@scottlamb
Copy link
Owner Author

With 0406e09, the binary shrunk from 154 MiB to 70 MiB. I'm still hoping to reduce it further with split debug info. From this comment and this PR it looks like work is actively being done on that.

@scottlamb scottlamb mentioned this issue Dec 24, 2022
Open
scottlamb added a commit that referenced this issue Feb 11, 2023
@scottlamb
switch from ancient clap/structopt release to bpaf
e21f795 
Improves #70: this reduces binary size from 12.3 MiB to 11.9 MiB (3%) on
macOS/arm64.

The user experience is almost the same. (The help output's `Usage:`
lines lack the e.g. `moonfire-nvr run` prefix of argv[0] and subcommand,
which isn't ideal, but I guess it's pretty minor in the grand scheme of
things.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement rust Rust backend work required
Projects
None yet
Milestone
wishlist
Development

No branches or pull requests
2 participants
@scottlamb @IronOxidizer