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

perf: strip release binaries to minimize size, but download debug info on panic to get useful stack traces #27811

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nathanwhit
Copy link
Member

This saves about 26 MB on macOS, should be similar on linux (stripping makes no difference on windows because symbols are already in a separate file).

This PR strips symbols from the release binaries, but separates and uploads the debug info for debugging.

To make sure stack traces on panics are still useful, we lazily download the debug info when panicking (but only if running with RUST_BACKTRACE set). Since most users should not hit panics, this means that most people will get the benefits of smaller binary size with no drawbacks.

An additional benefit is that now that debug info is separate, stack traces are actually more useful because we can included more detailed debug info without worrying about the binary size.

This also resolves an issue on windows where stack traces on panics are incorrect, as the symbol information is already separate but we don't distribute it.


Remaining questions / things to resolve:

  • I pulled in ureq for a simple synchronous http client, but maybe we could reuse our own client? just seemed tricky to do in a panic hook context
  • how to ensure versioning is correct. right now in this PR we remove the debug info on upgrades, and it'll just get lazily downloaded again for the new version. is this enough?
  • how to test this. the debug info download only happens on panic, so how do we get deno to panic reliably in a test (maybe a cargo feature that has an explicit panic somewhere reachable? but feels wrong)

@littledivy
Copy link
Member

right now in this PR we remove the debug info on upgrades, and it'll just get lazily downloaded again for the new version. is this enough?

ideally we could tell by reading something from the binary that identifies its debug info. probably fine for now.

how to test this. the debug info download only happens on panic, so how do we get deno to panic reliably in a test (maybe a cargo feature that has an explicit panic somewhere reachable? but feels wrong)

Maybe DENO_TEST_PANIC=1?

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.

2 participants