clockworklabs / SpacetimeDB

Multiplayer at the speed of light
https://spacetimedb.com
Other
4.37k stars 110 forks source link

CLI - `spacetime publish` should have flags for configuring `wasm-opt` #846

Open gefjon opened 8 months ago

gefjon commented 8 months ago

Optimizing modules with wasm-opt, at least under the default wasm-opt config, strips their debug symbols, making profiler output (e.g. perf) incomprehensible. We need some way to disable this other than "uninstall wasm-opt."

Bare minimum:

Flag to spacetime publish like --no-wasm-opt which still builds the module in release mode, but does not run wasm-opt. (Currently the only way to publish without running wasm-opt is via --debug, which is obviously undesirable for profiling.)

Ideal:

RReverser commented 8 months ago
  • Currently we pass -O2 -all, though it's not clear to me how much thought went into that choice.

-O2 has been chosen after careful comparisons of different modes on some example STDB modules. It had the best balance between performance and compilation times (-O3 is rarely useful in wasm-opt).

-all was added because C# doesn't emit proper target_features section, without which wasm-opt errors out on post-MVP Wasm features like bulk memory operations.

RReverser commented 8 months ago

FWIW I think -g should be fine to always keep around - if input Wasm was already stripped, it won't add anything, and if it has debug info, then it will be simply preserved. But might be better to only add it in debug builds (which STDB CLI already distinguishes).

gefjon commented 8 months ago

FWIW I think -g should be fine to always keep around - if input Wasm was already stripped, it won't add anything, and if it has debug info, then it will be simply preserved. But might be better to only add it in debug builds (which STDB CLI already distinguishes).

Well, we'd like a way to have release builds with symbols; that's the impetus for this ticket.

bfops commented 2 weeks ago

We discussed this in https://github.com/clockworklabs/SpacetimeDB/pull/1743. Summary:

The original motivating use case was addressed by https://github.com/clockworklabs/SpacetimeDB/pull/1013.

@gefjon says:

So long as we're preserving debug symbols, I consider the original issue solved and see no need to remove wasm-opt. I also don't see any downside to providing the flag, but certainly it's not a priority. Long-term I'd like to have a mechanism for passing arbitrary wasm-opt flags through spacetime build/spacetime publish, but again, not a priority.