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.

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

binary size is very large #330

Closed
kabirz opened this issue Jun 21, 2021 · 14 comments
Closed

binary size is very large #330

kabirz opened this issue Jun 21, 2021 · 14 comments
Labels
A-plugin Area: Plugin system C-enhancement Category: Improvements

Comments

@kabirz
Copy link
Contributor

kabirz commented Jun 21, 2021

The binary size about 24MBytes, it's too large. Why?

$ cargo bloat --release --package=helix-term
Finished release [optimized] target(s) in 0.30s
Analyzing target/release/hx
File  .text     Size          Crate Name
 0.4%   2.5% 136.0KiB      [Unknown] ts_lex
 0.4%   2.4% 132.1KiB      [Unknown] ts_lex
 0.4%   2.2% 121.5KiB      [Unknown] ts_lex
 0.1%   0.8%  44.6KiB          regex <regex::exec::ExecNoSync as regex::re_trait::RegularExpression>::captures_read_at
 0.1%   0.8%  44.3KiB      [Unknown] ts_lex
 0.1%   0.7%  37.1KiB      [Unknown] ts_lex
 0.1%   0.6%  32.5KiB      [Unknown] ts_lex
 0.1%   0.6%  31.2KiB      [Unknown] ts_lex
 0.1%   0.5%  28.3KiB      [Unknown] ts_lex
 0.1%   0.5%  27.6KiB     helix_term <toml::de::MapVisitor as serde::de::Deserializer>::deserialize_struct
 0.1%   0.5%  26.5KiB      [Unknown] ts_lex
 0.1%   0.5%  25.0KiB          regex regex::exec::ExecBuilder::build
 0.1%   0.4%  19.9KiB      [Unknown] ts_lex
 0.1%   0.4%  19.8KiB      [Unknown] ts_lex
 0.1%   0.4%  19.3KiB            std addr2line::Context<R>::from_dwarf
 0.1%   0.3%  19.1KiB      [Unknown] ts_lex
 0.1%   0.3%  18.9KiB      [Unknown] ts_lex
 0.1%   0.3%  18.8KiB pulldown_cmark pulldown_cmark::parse::Parser::handle_inline_pass1
 0.1%   0.3%  18.2KiB     helix_term <toml::de::ValueDeserializer as serde::de::Deserializer>::deserialize_struct
 0.1%   0.3%  18.2KiB            std std::backtrace_rs::symbolize::gimli::resolve::{{closure}}
14.9%  83.0%   4.5MiB                And 12663 smaller methods. Use -n N to show more.
17.9% 100.0%   5.4MiB                .text section size, the file size is 30.0MiB
@archseer
Copy link
Member

All the embedded grammar parsers, that's what happens when everything is statically linked into one binary. Additionally you're looking at the binary size before stripping debug info. Rust still includes debug info on --release builds because it makes for nicer backtraces.

https://github.com/johnthagen/min-sized-rust

@kirawi
Copy link
Member

kirawi commented Jun 21, 2021

Wait, why aren't we dynamically linking?

@archseer
Copy link
Member

archseer commented Jun 21, 2021

Rust doesn't dynamically link (by default), no. (Even stdlib is statically linked.)

Ideally we want to compile the grammars separately, then load them with libloading, that way the user isn't forced to compile all grammars. tree-sitter has a TODO upstream to compile the grammars optionally to wasm so we could compile once and distribute.

@archseer
Copy link
Member

(Note: strip trims about 6MB for me, 31->25MB)

@kirawi
Copy link
Member

kirawi commented Jun 21, 2021

I don't think we need to wait for it to compile to Wasm, it can already be compiled as a dynamic library and we can link to it with libloading just like NeoVim does.

@archseer
Copy link
Member

Building the C code via cc-rs as a dynamic lib isn't supported. tree-sitter-cli has some workarounds but they're platform specific.

https://github.com/tree-sitter/tree-sitter/blob/0fea8c02ee50756a0c493ccc30a0b9749b3bab59/cli/loader/src/lib.rs#L310-L394

@archseer
Copy link
Member

Ignore the numbers here since they're before optimization and stripping, but you can see the code size proportions of each crate:

.rw-r--r--  2.2M speed users 21 Jun 10:11  libhelix_core.rlib
.rw-r--r--  4.2M speed users 21 Jun 10:11  libhelix_lsp.rlib
.rw-r--r--   19M speed users 21 Jun 10:11  libhelix_syntax.rlib
.rw-r--r--  5.9M speed users 21 Jun 10:11  libhelix_term.rlib
.rw-r--r--  1.3M speed users 21 Jun 10:11  libhelix_tui.rlib
.rw-r--r--  2.4M speed users 21 Jun 10:11  libhelix_view.rlib

@pickfire
Copy link
Contributor

Doesn't that make it part of plugin if we are to move them?

@pickfire pickfire added A-plugin Area: Plugin system C-enhancement Category: Improvements labels Jun 21, 2021
@CBenoit
Copy link
Member

CBenoit commented Jun 21, 2021

Doesn't that make it part of plugin if we are to move them?

It definitely could, especially if we can compile everything into a .wasm file and just load that at runtime. rust-syntax.wasm, go-syntax.wasm, …

As an aside, it would also help with publishing on crates.io

@kirawi
Copy link
Member

kirawi commented Jun 21, 2021

Helix plugin manager when? Haha, joking.

@CBenoit
Copy link
Member

CBenoit commented Jun 21, 2021

Helix plugin manager when? Haha, joking.

Plugins system is not even a reality yet, plugins manager is science fiction !!
More seriously though, the plugins system is probably my next PR. I just want to ask a few more questions to wasmer/wasmtime developers before the end of the week.

@Kethku
Copy link
Contributor

Kethku commented Jun 22, 2021

Just my 2 cents: bending over backwards to shrink the binary size this early in development can make life harder. At this stage its often better to take the size hit in favor of simpler design and then reconsider dependencies and such later.

@archseer
Copy link
Member

See #432 for a draft.

@kabirz
Copy link
Contributor Author

kabirz commented Jul 14, 2021

Good job. Now about 7.8MB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-plugin Area: Plugin system C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

6 participants