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

[Merged by Bors] - fix: Reduce wasm size by stripping symbols #2774

Closed

Conversation

davidbeesley
Copy link
Contributor

stripping symbols has a large impact on wasm binary size. Is there a reason it is necessary to keep them?

Before

 248K target/wasm32-unknown-unknown/release/fluvio_smartmodule_aggregate.wasm*
 428K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_array.wasm*
 428K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_object.wasm*
 244K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_init.wasm*
 236K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_map.wasm*
 240K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_param.wasm*
 236K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter.wasm*
 192K target/wasm32-unknown-unknown/release/fluvio_smartmodule_map.wasm*
 432K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_average.wasm*
 404K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_json.wasm*
 252K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_sum.wasm*
 432K target/wasm32-unknown-unknown/release/fluvio_wasm_array_map_reddit.wasm*
 404K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_json.wasm*
 240K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_odd.wasm*
 956K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_regex.wasm*
 236K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_with_parameters_v1.wasm*
 248K target/wasm32-unknown-unknown/release/fluvio_wasm_join.wasm*
 240K target/wasm32-unknown-unknown/release/fluvio_wasm_map_double.wasm*
 440K target/wasm32-unknown-unknown/release/fluvio_wasm_map_json.wasm*
1020K target/wasm32-unknown-unknown/release/fluvio_wasm_map_regex.wasm*

After

 72K target/wasm32-unknown-unknown/release/fluvio_smartmodule_aggregate.wasm*
160K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_array.wasm*
160K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_object.wasm*
 76K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_init.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_map.wasm*
 76K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_param.wasm*
 68K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter.wasm*
 52K target/wasm32-unknown-unknown/release/fluvio_smartmodule_map.wasm*
168K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_average.wasm*
140K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_json.wasm*
 76K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_sum.wasm*
168K target/wasm32-unknown-unknown/release/fluvio_wasm_array_map_reddit.wasm*
140K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_json.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_odd.wasm*
736K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_regex.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_with_parameters_v1.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_join.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_map_double.wasm*
200K target/wasm32-unknown-unknown/release/fluvio_wasm_map_json.wasm*
796K target/wasm32-unknown-unknown/release/fluvio_wasm_map_regex.wasm*
                                                                                                                                                                                                                                                                                                                                              

@sehz
Copy link
Contributor

sehz commented Nov 3, 2022

What's the impact on the troubleshooting experience? (stack trace, etc). If not much impact, this is big improvement!

@EstebanBorai
Copy link
Contributor

What's the impact on the troubleshooting experience? (stack trace, etc). If not much impact, this is big improvement!

I think that for release builds is not that bad to have these optimizations, there's also debuginfo available. I wonder which are the drawbacks on debugging and build size optimizations.

@davidbeesley
Copy link
Contributor Author

davidbeesley commented Nov 4, 2022

What's the impact on the troubleshooting experience? (stack trace, etc). If not much impact, this is big improvement!

Good question. I'm not sure what most people use to debug wasm but attached below is the output I saw from running WASMTIME_BACKTRACE_DETAILS=1 smdk test on a module with a runtime error. It certainly is easier to debug with the symbols. I agree with @EstebanBorai that it's good for release builds unless there are other considerations I'm unaware of.

Some options would be:

  1. Have smdk test build a separate profile (cargo test rebuilds as well so that matches) and take a --release flag to run tests with the release mode, and have smdk build default to stripping symbols.
  2. Leave it in the Cargo.toml but add a comment explaining the significance.
  3. Have smdk build take a flag that modifies building to either include or exclude the symbols.
With symbols
loading module at: /home/dave/work/tmp/target/wasm32-unknown-unknown/release-lto/tmp.wasm
Error: wasm trap: wasm `unreachable` instruction executed
wasm backtrace:
    0: 0x98a2 - panic_abort::__rust_start_panic::abort::hc2c8aed43d968189
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/panic_abort/src/lib.rs:83:17
              - __rust_start_panic
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/panic_abort/src/lib.rs:37:5
              - rust_panic
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:746:9
    1: 0x9c38 - std::panicking::rust_panic_with_hook::hb09154fa23e06c37
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:716:5
    2: 0x9bc7 - std::panicking::begin_panic_handler::{{closure}}::h6091c197f0d08bf0
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:586:13
    3: 0x9b96 - std::sys_common::backtrace::__rust_end_short_backtrace::h004afb3e6a867c40
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/sys_common/backtrace.rs:138:18
    4: 0x42ee - rust_begin_unwind
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:584:5
    5: 0x2fdd - core::panicking::panic_fmt::h9e229748e3ae9f9d
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:142:14
    6: 0x98ec - std::sys::wasm::time::Instant::now::h3d9d01686938ff0a
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/sys/wasm/../unsupported/time.rs:13:9
              - std::time::Instant::now::hd37bf69e0e83bfb7
                    at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/time.rs:276:17
    7: 0xcb52 - <unknown>!filter


Without symbols
loading module at: /home/dave/work/tmp/target/wasm32-unknown-unknown/release-lto/tmp.wasm
Error: wasm trap: wasm `unreachable` instruction executed
wasm backtrace:
    0: 0x98c1 - <unknown>!<wasm function 153>
    1: 0x9c57 - <unknown>!<wasm function 160>
    2: 0x9be6 - <unknown>!<wasm function 159>
    3: 0x9bb5 - <unknown>!<wasm function 158>
    4: 0x430d - <unknown>!<wasm function 59>
    5: 0x2ffc - <unknown>!<wasm function 29>
    6: 0x990b - <unknown>!<wasm function 154>
    7: 0xcb75 - <unknown>!<wasm function 169>

@sehz
Copy link
Contributor

sehz commented Nov 4, 2022

I think we can eliminate backtrace as well. We can add another release profile to include symbols, backtrace and etc.

Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

Good improvement. We can create separate issues to bring back symbol for debugging and testing.

@sehz
Copy link
Contributor

sehz commented Nov 6, 2022

bors r+

@sehz sehz added this to the 0.10.1 milestone Nov 6, 2022
bors bot pushed a commit that referenced this pull request Nov 6, 2022
stripping symbols has a large impact on wasm binary size. Is there a reason it is necessary to keep them?

#### Before
```
 248K target/wasm32-unknown-unknown/release/fluvio_smartmodule_aggregate.wasm*
 428K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_array.wasm*
 428K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_object.wasm*
 244K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_init.wasm*
 236K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_map.wasm*
 240K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_param.wasm*
 236K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter.wasm*
 192K target/wasm32-unknown-unknown/release/fluvio_smartmodule_map.wasm*
 432K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_average.wasm*
 404K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_json.wasm*
 252K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_sum.wasm*
 432K target/wasm32-unknown-unknown/release/fluvio_wasm_array_map_reddit.wasm*
 404K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_json.wasm*
 240K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_odd.wasm*
 956K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_regex.wasm*
 236K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_with_parameters_v1.wasm*
 248K target/wasm32-unknown-unknown/release/fluvio_wasm_join.wasm*
 240K target/wasm32-unknown-unknown/release/fluvio_wasm_map_double.wasm*
 440K target/wasm32-unknown-unknown/release/fluvio_wasm_map_json.wasm*
1020K target/wasm32-unknown-unknown/release/fluvio_wasm_map_regex.wasm*

```

#### After 
```
 72K target/wasm32-unknown-unknown/release/fluvio_smartmodule_aggregate.wasm*
160K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_array.wasm*
160K target/wasm32-unknown-unknown/release/fluvio_smartmodule_array_map_object.wasm*
 76K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_init.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_map.wasm*
 76K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter_param.wasm*
 68K target/wasm32-unknown-unknown/release/fluvio_smartmodule_filter.wasm*
 52K target/wasm32-unknown-unknown/release/fluvio_smartmodule_map.wasm*
168K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_average.wasm*
140K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_json.wasm*
 76K target/wasm32-unknown-unknown/release/fluvio_wasm_aggregate_sum.wasm*
168K target/wasm32-unknown-unknown/release/fluvio_wasm_array_map_reddit.wasm*
140K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_json.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_odd.wasm*
736K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_regex.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_filter_with_parameters_v1.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_join.wasm*
 72K target/wasm32-unknown-unknown/release/fluvio_wasm_map_double.wasm*
200K target/wasm32-unknown-unknown/release/fluvio_wasm_map_json.wasm*
796K target/wasm32-unknown-unknown/release/fluvio_wasm_map_regex.wasm*
                                                                                                                                                                                                                                                                                                                                              
```
@bors
Copy link

bors bot commented Nov 6, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix: Reduce wasm size by stripping symbols [Merged by Bors] - fix: Reduce wasm size by stripping symbols Nov 6, 2022
@bors bors bot closed this Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants