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

fix(cargo.toml): rm debug symbols in release build #1441

Merged
merged 3 commits into from Aug 9, 2022

Conversation

kskarthik
Copy link
Contributor

@kskarthik kskarthik commented Aug 1, 2022

Helps further slimming of the binary.

Info:

  1. https://doc.rust-lang.org/cargo/reference/profiles.html#strip
  2. https://github.com/johnthagen/min-sized-rust#optimize-for-size

What kind of change does this PR introduce?

  • Feature

Did this PR introduce a breaking change?

  • No

Copy link
Collaborator

@last-partizan last-partizan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@LoipesMas
Copy link
Member

I think we'd rather optimize for speed, not size.
And from my testing it looks like the difference is negligible anyway:
image
That's current profile, with strip = true added and with strip = true and opt-level = "z" added, respectively.
It's a 4mb difference at most, so I'm not sure if potential performance loss is worth it.
Maybe if we did a benchmark and there were no performance loss.

@kskarthik
Copy link
Contributor Author

@LoipesMas so strip=true should be good to go, if performace is the choice

@LoipesMas
Copy link
Member

Yeah, just stripping should be fine.

opt-level z is overkill, currently Neovide is facing more performance
than size issues.

This reverts commit 1d20613.
@MultisampledNight MultisampledNight merged commit 669a526 into neovide:main Aug 9, 2022
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.

None yet

4 participants