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

strip debug info from generated wasm if minify is true #3671

Merged

Conversation

AurelienRichez
Copy link

↪️ Pull Request

Disclaimer : I am not a wasm or rust expert.

I tinkered with parceljs, and managed to reduce the generated wasm file from 1.6MB to 116B for the usuall add example.

After trying to compile rust code with parcel, I noticed that the wasm file was really big ( #3235 shows the problem). More than 1.5MB 😨. Using twiggy shows that it is essentially bloat from debugs symbols.

I tried to figure out how wasm-pack optimize wasm and then tried different solutions to shrink the size on the simple add.rs file :

  • -Copt-level=s (should optimize for size): no visible effect, but it makes sense for such a simple function
  • -Clto=thin: it reduced the size to 44kB A big win but still a lot for a simple add function. It seems some debug symbol are still there
  • -Cdebuginfo=0 (should remove debug info): no visible effect, so I may have misunderstood this option.
  • -Clink-arg=-s (option passed to the linker to strip the binary): it reduced the size to 116B.

As a result I kept the last one (-Clto=thin did not show any difference once -Clink-arg=-s is added) . It is difficult to know if the others are useful for a more complicated case. I tried this on a mac.

I think wasm-pack actually uses wasm-opt to do this, so this might be another way to do it.

💻 Examples

For reference, the rust code used.
add.rs :

#[no_mangle]
pub fn add(a: i32, b: i32) -> i32 {
  return a + b
}

building with parcel version 1.12.4 :

✨  Built in 669ms.

dist/add.wasm    ⚠️  1.65 MB    615ms 

building with --no-cache with the modified version :

✨  Built in 502ms.

dist/add.wasm    116 B    448ms

🚨 Test instructions

I did not add unit tests for this (this is not really a bug). A good way to test this would be to compile the rust function mentionned above by calling parcel (or repo mentionned in #3235).

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

Copy link
Member

@DeMoorJasper DeMoorJasper left a comment

Choose a reason for hiding this comment

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

This looks amazing.

Not entirely sure if we'll merge this as Parcel 1 is currently not really receiving any more updates.
Even if this doesn't end up in Parcel 1 it will definitely be an big help when implementing Rust for Parcel 2

@devongovett devongovett merged commit cf83b72 into parcel-bundler:master Oct 26, 2019
This was referenced Mar 16, 2021
This was referenced Mar 16, 2021
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.

3 participants