binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Avoid unreachable exception

Open Squareys opened this issue 3 years ago • 1 comments

Hi all,

as mentioned in #4165, a small (but insignificant in the grand scheme of things) performance improvement can be gained from avoiding the exception here, since when it gets thrown, it blocks all threads for 1-2 ms. Since unreachable is not that common in most code (we might have 20-40 instances of it), that means the savings could be 40*1ms*8 = 320 ms for 8 threads here. In the benchmarks I did for the SmallSet in EffectsAnalyzer, this gets drowned out by noise.

Depending how risky it is to make this change, you might consider discarding the PR. It looks like the wasm-fuzz-types.exe is pretty happy so far, though (1376658 iterations). I wasn't sure what change needed to be made in execution-results.h 🤔

Best, Jonathan

Squareys avatar Aug 09 '22 16:08 Squareys

I think this is a good idea, but yeah, it needs some changes in at least execution-results. I think the code there does a callFunction which now will return a Flow with NONCONSTANT_FLOW - we need to turn that into a Trap, which is what we used to get before. There might be more places that need this, like wasm-ctor-eval - anything that does callFunction or callExport will need to be audited. But if we do that work, I think we could get rid of the C++ exceptions stuff, which would be a nice speedup.

I can look into this when I find time, if you don't want to.

kripken avatar Aug 09 '22 17:08 kripken