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

Deprecate --keep-debug flag #3876

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

daxpedda
Copy link
Collaborator

@daxpedda daxpedda commented Mar 8, 2024

Debug information is already controlled by Rustc/Cargo, but the reason this flag was introduced is to control debug information coming from the pre-compiled Std libraries, see #466.

This can now be done by the user instead in Cargo with the strip option, which is available since v1.59 (or directly with Rustc -C strip since v1.58).

Additionally, this will require no user action at all when v1.77 releases, which will strip Std's debug information automatically when no debug information is requested (e.g. the release profile), see rust-lang/cargo#13257.

This should significantly improve the stacktraces with the downside of significantly increased Wasm file sizes in release mode for users who don't strip debug information or are using Cargo versions below v1.77.

We might want to hold off merging this until Rust v1.77 release, which would be 21th March, but considering how close this is (13 days) I think its alright.

See #3483, which made this finally possible.
See #3698 for tracking other debug information related improvements.

@daxpedda daxpedda requested a review from Liamolucko March 8, 2024 22:15
@Liamolucko
Copy link
Collaborator

Debug information is already controlled by Rustc/Cargo, but the reason this flag was introduced is to control debug information coming from the pre-compiled Std libraries, see #466.

Interesting, I didn't know that history.

This should significantly improve the stacktraces with the downside of significantly increased Wasm file sizes in release mode for users who don't strip debug information or are using Cargo versions below v1.77.

I didn't think debug info affected stack traces. At least at a first glance, browsers only seem to use the name section for that, which is already included without --keep-debug.

It should still be useful for actually debugging, though.

We might want to hold off merging this until Rust v1.77 release, which would be 21th March, but considering how close this is (13 days) I think its alright.

Yeah, this probably won't be getting included in a wasm-bindgen release until after that point anyway.

@daxpedda
Copy link
Collaborator Author

daxpedda commented Mar 8, 2024

This should significantly improve the stacktraces with the downside of significantly increased Wasm file sizes in release mode for users who don't strip debug information or are using Cargo versions below v1.77.

I didn't think debug info affected stack traces. At least at a first glance, browsers only seem to use the name section for that, which is already included without --keep-debug.

Strange, it does improve the stack trace for me significantly.

@daxpedda daxpedda force-pushed the deprecate-debug branch 2 times, most recently from dc6d2ba to 19a6b59 Compare March 8, 2024 23:26
@daxpedda
Copy link
Collaborator Author

daxpedda commented Mar 9, 2024

So after some digging, I found that wasm-opt doesn't currently support support our DWARF output, for whatever reason. I opened WebAssembly/binaryen#6391 for that.

The main worry here is that wasm-pack suddenly stops working. This shouldn't be a problem as soon as Rust v1.77 hits, as then debug information will be stripped by default anyway.

I will update the PR when that happens.

@daxpedda daxpedda marked this pull request as draft March 9, 2024 00:42
@daxpedda
Copy link
Collaborator Author

daxpedda commented Mar 9, 2024

This should significantly improve the stacktraces with the downside of significantly increased Wasm file sizes in release mode for users who don't strip debug information or are using Cargo versions below v1.77.

I didn't think debug info affected stack traces. At least at a first glance, browsers only seem to use the name section for that, which is already included without --keep-debug.

Strange, it does improve the stack trace for me significantly.

You are right, I had the C/C++ DevTools Support extension enabled, which is why I was getting better stacktraces.

@daxpedda
Copy link
Collaborator Author

Apparently wasm-opt still fails unless we use -Cstrip=symbols, I'm happy to still move forwards with this as it isn't really a wasm-bindgen issue.

WDYT @Liamolucko.

@daxpedda
Copy link
Collaborator Author

daxpedda commented Mar 27, 2024

I'm unsure why wasm-opt fails in CI, seems to work locally for me ...

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

2 participants