Stop building with assertions enabled by default?

[kripken]

Early in Binaryen's history we wanted assertions in all builds in order to catch bugs. It was useful for a while but maybe today it is excessive. Profiling a 200MB wasm file now, I see just isDebugEnabled takes 2.64% of the time. While we could optimize that perhaps, there is surely lots of other overhead from assertions.

Having assertions in our main builds does mean user bug reports are potentially more helpful, but I can't remember them actually helping much? So I'm leaning towards setting BYN_ENABLE_ASSERTIONS to false by default.

[tlively]

I'd be fine with this 👍

[kripken]

Update:

• isDebugEnabled that is mentioned before is no longer a performance bottleneck, after some optimization work.
• BYN_ENABLE_ASSERTIONS=OFF helps very little with speed - I see just a 2% difference. But I do see a size difference of 20% in libbinaryen.so.

So it may make sense to disable assertions by default for build size reasons, but less for speed, I guess (I assume the reason is that our assertions are very fast checks - slow ones are behind PASS_DEBUG mode). Not sure if it's is worth changing our default here, as having assertions on does help sometimes.

cc @dschuff

[dschuff]

While it's true that having assertions on helps sometimes, at least for emscripten-releases I'm not sure it's worthwhile to have Binaryen's configuration be different from LLVM's. It's already the case that users can experience LLVM crashes and we ask them to try the -asserts version from emsdk and reproduce. So there doesn't seem to be much value to just having Binaryen with assertions. (We could of course turn assertions on in LLVM, but last time I checked that made a much more substantial difference for LLVM).

[dschuff]

On that note, https://emscripten.org/docs/tools_reference/emsdk.html seems kind of out-of-date with respect to the way most users use emsdk with emscripten-releases (e.g. it still refers to targets like "sdk-upstream-main-64bit" etc. I guess those are just for compiling from source these days)? So maybe should update that. What I was looking for is whether there is any documentation that suggests using the -asserts build, but I don't see that anywhere.

[kripken]

Yes, sdk-upstream-main-64bit are for compiling from source, I think those are still valid.

I opened https://github.com/emscripten-core/emscripten/pull/24484 to add docs for the asserts builds.