nextclade icon indicating copy to clipboard operation
nextclade copied to clipboard

Turn exception handling off in nextclade_wasm/nextalign

Open johnlees opened this issue 3 years ago • 2 comments

Love the use of WASM for nextalign!

I was wondering whether you had tried/considered turning exception handling off when compiling your code (-s DISABLE_EXCEPTION_CATCHING=1 -fno-exceptions)? Anecdotally, I've found this has made WASM run about 10x faster.

A pretty quick and dirty way to remove them is using a preprocessor define to replace except blocks with abort().

johnlees avatar Jun 11 '21 15:06 johnlees

Hi @johnlees,

Yes, we already have exception catching disabled in all but the root module (the one that talks to JS). But we need exceptions and we need to catch some of the exceptions, which are not fatal errors, in the root module. Otherwise this https://github.com/nextstrain/nextclade/issues/434 happens.

Maybe we can sometimes remove that requirement too. There is still a lot of room for improvement. If you have suggestions, we'd be happy! Contributions are also super welcome!

ivan-aksamentov avatar Jun 11 '21 16:06 ivan-aksamentov

Ah that's great – mostly just wanted to check this was on your radar as I didn't find this clear when I first used emscripten. I'd looked at one of your CMake files (but must have been the wrong one) where it looked like they were switched on.

There might be an advantage to removing your final use of exceptions (I only got the speedup when they were totally removed from the wasm, but I think you might have more wasm modules?). https://twitter.com/bitmagicio/status/1222726308515459072 It's not great C++, but based on doing this in openmp blocks where you can't throw, returning -1 or similar can work without much change.

I'll see if I can have a closer look at some point, but looks like your emscripten flags are pretty similar to what I would have tried 🙂

johnlees avatar Jun 14 '21 13:06 johnlees

Closing this because Nextclade has been reimplemented in Rust, which does not have exceptions, so this no longer applies.

ivan-aksamentov avatar Jan 27 '23 20:01 ivan-aksamentov