-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Gzip artifacts #4972
Gzip artifacts #4972
Conversation
Hmm, actually I think we might need to compile xtask in release mode. |
I guess we could use a profile override to compile flate2 with optimizations. |
Hmm, that's a nice idea |
e2eb661
to
70da811
Compare
Squashed the changes |
70da811
to
e223830
Compare
FTR, I was about to say that github most likely already uses deflate as HTTP encoding -- but then I checked and according to |
Not sure if we need to do this -- I'd rather keep this minimal. The end-goal is distribution via rustup, so additional complexity here won't be worthwhile over the long run. This will also make life harder for folks who maintain other editor plugins. |
What would help long term is pushing this PR over the finish line: rust-lang/rust#72978 (it currently misses the bit about vendoring). |
@matklad do you plan to keep offering binary releases after rust-lang/rust#72978? |
@lnicola for at least medium term -- yes |
Hm, I am thinking that maybe we should enable |
Do we even get backtraces? I think we strip the binaries. |
Maybe we should enable debug info for the binary releases, but not for source builds. |
So that nightly users have sane stack traces. I think it makes sense to
extend this to everybody, if, after gziping, the binary size is within
reason.
…On Wed, 1 Jul 2020 at 18:10, Veetaha ***@***.***> wrote:
@matklad <https://github.com/matklad> this is the reason we don't strip
binaries for nightlies, haven't found a PR, but here is the commit:
ffb7ea6
<ffb7ea6>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4972 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANB3MYJGUWRMHXSHBCRRU3RZNNYDANCNFSM4OD5RDSQ>
.
|
5697716
to
97f6f8c
Compare
Removed stripping. |
run!( | ||
"cargo build --manifest-path ./crates/rust-analyzer/Cargo.toml --bin rust-analyzer --release" | ||
// We'd want to add, but that requires setting the right linker somehow | ||
// --features=jemalloc | ||
)?; | ||
if !nightly { | ||
run!("strip ./target/release/rust-analyzer")?; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
I also wonder whether we really need to have a special case |
Tested this on host Ubuntu and VirtualBox windows, both work. |
$ size -A -x rust-analyzer-linux
section size addr
.interp 0x1c 0x270
.note.ABI-tag 0x20 0x28c
.note.gnu.build-id 0x24 0x2ac
.gnu.hash 0x523c 0x2d0
.dynsym 0x13c68 0x5510
.dynstr 0x4b686 0x19178
.gnu.version 0x1a5e 0x647fe
.gnu.version_r 0x190 0x66260
.rela.dyn 0xe1900 0x663f0
.init 0x1a 0x147cf0
.plt 0x10 0x147d10
.plt.got 0xf0 0x147d20
.text 0x16dc110 0x147e10
.fini 0x9 0x1823f20
.rodata 0xe5de3 0x1823f40
.eh_frame_hdr 0x6f574 0x1909d24
.eh_frame 0x33ca64 0x1979298
.gcc_except_table 0x17f8dc 0x1cb5cfc
.tdata 0x60 0x2035700
.tbss 0x458 0x2035760
.init_array 0x10 0x2035760
.fini_array 0x8 0x2035770
.jcr 0x8 0x2035778
.data.rel.ro 0x98c10 0x2035780
.dynamic 0x220 0x20ce390
.got 0x6a38 0x20ce5b0
.data 0x209 0x20d5000
.bss 0xf08 0x20d5210
.comment 0x7f 0x0
.debug_aranges 0x8860 0x0
.debug_pubnames 0x31761 0x0
.debug_info 0xa1f13 0x0
.debug_abbrev 0x197e 0x0
.debug_line 0x5ee2f 0x0
.debug_frame 0xf18 0x0
.debug_str 0xa9bdf 0x0
.debug_loc 0x27d48 0x0
.debug_macinfo 0x19 0x0
.debug_pubtypes 0x132 0x0
.debug_ranges 0x6ea70 0x0
Total 0x2153c8d
$ size -A -x ~/Downloads/rust-analyzer-linux | grep debug | tr -s ' ' | cut -d' ' -f2 | tr '\n' '+'
0x8860+0x31761+0xa1f13+0x197e+0x5ee2f+0xf18+0xa9bdf+0x27d48+0x19+0x132+0x6ea70+
$ echo $((0x8860+0x31761+0xa1f13+0x197e+0x5ee2f+0xf18+0xa9bdf+0x27d48+0x19+0x132+0x6ea70)) # debuginfo (sum(.debug_*))
2611835
# = 0x27da7b = ~2.5MiB
$ echo $((0x6f574+0x33ca64+0x17f8dc)) # unwind info (.eh_frame_hdr + .eh_frame + .gcc_except_table)
5421236
# 0 0x52b8b4 = ~5.2MiB
$ echo $((0x2153c8d-0x16dc110-0xe5de3)) # total - .text - .rodata
10034586
# = 0x991d9a = ~9.6MiB Most of the non code/data part seems to be debuginfo and unwind info. |
@bjorn3 is it possible to get these stats for windows/mac? |
For MachO files |
|
bors d+ (some conflicts in cargo.lock) (miniz_oxide will be deduped after rust-lang/backtrace-rs#354) |
✌️ Veetaha can now approve this pull request. To approve and merge a pull request, simply reply with |
#5250 updates miniz |
Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com> Override miniz_oxide to build it with optimizations Building this crate with optimizations decreases the gzipping part of `cargo xtask dist` from `30-40s` down to `3s`, the overhead for `rustc` to apply optimizations is miserable on this background
97f6f8c
to
f92bfb5
Compare
bors r+ |
Here is the test release
Change in size:
~ 25 MB -> ~ 8 MB (gzipped)
The time to gzip during the dist build takes a somewhat considerable amount of time tho.
Having already compiled artifacts this takes in debug mode:
In release mode this is much faster:
[UPD] adding a profile override for
miniz_oxide
does the thing to ensure good performranceWe might need to notify all other ra plugins' maintainers about the change in our GH releases if we merge this PR, or we could leave uncompressed files along with gzipped for a while until everyone migrates.