Add support for the new WASM Exception Handling feature by vargaz · Pull Request #66435 · dotnet/runtime

vargaz · 2022-03-10T05:04:19Z

No description provided.

…onse.

WASM EH uses the newly added llvm catchpad instructions instead of the old landingpad instructions. Add a --wasm-exceptions option to control whenever the old JS style or the new WASM style code is generated.

vargaz · 2022-03-10T05:07:01Z

Benchmarks:
BEFORE:


.md
| measurement | time |
|-:|-:|
| Exceptions, NoExceptionHandling | 0.0761us |
| Exceptions, TryCatch | 0.1133us |
| Exceptions, TryCatchThrow | 0.0162ms |
| Exceptions, TryCatchFilter | 0.1101us |
| Exceptions, TryCatchFilterInline | 0.0824us |
| Exceptions, TryCatchFilterThrow | 0.0270ms |
| Exceptions, TryCatchFilterThrowApplies | 0.0225ms |
| Json, non-ASCII text serialize | 1.2152ms |
| Json, non-ASCII text deserialize | 2.2020ms |
| Json, small serialize | 0.1147ms |
| Json, small deserialize | 0.1594ms |
| Json, large serialize | 22.4554ms |
| Json, large deserialize | 46.6294ms |

AFTER:

.md
| measurement | time |
|-:|-:|
| Exceptions, NoExceptionHandling | 0.0222us |
| Exceptions, TryCatch | 0.0383us |
| Exceptions, TryCatchThrow | 0.0760ms |
| Exceptions, TryCatchFilter | 0.0389us |
| Exceptions, TryCatchFilterInline | 0.0379us |
| Exceptions, TryCatchFilterThrow | 0.0865ms |
| Exceptions, TryCatchFilterThrowApplies | 0.0819ms |
| Json, non-ASCII text serialize | 1.3284ms |
| Json, non-ASCII text deserialize | 2.5833ms |
| Json, small serialize | 0.0931ms |
| Json, small deserialize | 0.1310ms |
| Json, large serialize | 21.0037ms |
| Json, large deserialize | 37.7421ms |

A micro benchmark which iterated on an IEnumerable got about 60% faster.

kg · 2022-03-10T06:02:19Z

💃🎉🎊🥳

It is controlled by the $(WasmExceptionHandling) msbuild property. Its only used in AOT mode, the interpreter mode wouldn't benefit from it. The only file in the runtime which contains a throw or a catch is llvm-runtime.cpp, so create 2 small static libraries from this file: * libmono-wasm-eh-js - compiled to use JS style EH * libmono-wasm-eh-wasm - compiled to use WASM style EH This avoids the need to compile 2 versions of the whole runtime. Requires emscripten 3.x.

radekdoulik

LGTM

radekdoulik · 2022-03-10T08:51:06Z


 if(CMAKE_BUILD_TYPE STREQUAL "Release")
- add_custom_command(TARGET dotnet POST_BUILD COMMAND ${EMSDK_PATH}/upstream/bin/wasm-opt --strip-dwarf ${NATIVE_BIN_DIR}/dotnet.wasm -o ${NATIVE_BIN_DIR}/dotnet.wasm)
+ add_custom_command(TARGET dotnet POST_BUILD COMMAND ${EMSDK_PATH}/upstream/bin/wasm-opt --enable-exception-handling --strip-dwarf ${NATIVE_BIN_DIR}/dotnet.wasm -o ${NATIVE_BIN_DIR}/dotnet.wasm)


Does --enable-exception-handling do something also for wasm-eh-js case?

Its needed for wasm-opt to not fail, it looks harmless otherwise.

radekdoulik · 2022-03-10T09:06:34Z

The measurements look nice! One thing I wonder about is why the throw cases are now slower. Do you know what might be behind this? Maybe this path is not just optimized yet in v8?

radekdoulik · 2022-03-10T12:34:18Z

/azp run runtime-wasm

azure-pipelines · 2022-03-10T12:34:33Z

Azure Pipelines successfully started running 1 pipeline(s).

SamMonoRT · 2022-03-10T14:30:09Z

Contributes to #63399

Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>

vargaz · 2022-03-10T15:02:20Z

/azp run runtime-wasm

azure-pipelines · 2022-03-10T15:02:35Z

Azure Pipelines successfully started running 1 pipeline(s).

radical · 2022-03-10T19:28:20Z

 <_EmccCommonFlags Include="-s EXPORT_ES6=1" Condition="'$(WasmEnableES6)' == 'true'" />
 <_EmccCommonFlags Include="-g" Condition="'$(WasmNativeStrip)' == 'false'" />
 <_EmccCommonFlags Include="-v" Condition="'$(EmccVerbose)' != 'false'" />
+ <_EmccCommonFlags Include="-s DISABLE_EXCEPTION_CATCHING=0" Condition="'$(WasmExceptionHandling)' == 'false'" />


Suggested change
<_EmccCommonFlagsInclude="-s DISABLE_EXCEPTION_CATCHING=0"Condition="'$(WasmExceptionHandling)' == 'false'" />
<_EmccCommonFlagsInclude="-s DISABLE_EXCEPTION_CATCHING=0"Condition="'$(WasmExceptionHandling)' != 'true'" />

Why is this needed ? The property is initialized to false if not set.
<WasmExceptionHandling Condition="'$(WasmExceptionHandling)' == ''">false</WasmExceptionHandling>

radical · 2022-03-10T19:54:49Z

 - $(RunAOTCompilationAfterBuild) - Run AOT compilation even after Build. By default, it is run only for publish.
 Defaults to false.
 - $(WasmAotProfilePath) - Path to an AOT profile file.
+ - $(WasmExceptionHandling) - Enable support for the WASM Exception Handling feature.


This should be renamed to WasmEnableExceptionHandling.

that sounds like there wouldn't be exception handling support if it isn't true though

WasmEnableWasmEHFeature? WasmEnableNativeExceptionHandlingFeature?

vargaz · 2022-03-11T02:56:29Z

The CI failures look unrelated

vargaz · 2022-03-11T16:50:17Z

Will merge this for now, the property can be renamed later.

vargaz added 2 commits March 9, 2022 23:53

[mono] Fix parsing options added by utils/options.h when using --resp…
7d8ad92
…onse.

[mono][jit] Add support for WASM EH.
33edf29
WASM EH uses the newly added llvm catchpad instructions instead of the old landingpad instructions. Add a --wasm-exceptions option to control whenever the old JS style or the new WASM style code is generated.

vargaz requested review from SamMonoRT, lambdageek and marek-safar as code owners March 10, 2022 05:04

ghost assigned vargaz Mar 10, 2022

ghost added the area-Build-mono label Mar 10, 2022

vargaz requested review from kg, radekdoulik and radical and removed request for SamMonoRT, lambdageek and marek-safar March 10, 2022 05:04

vargaz commented Mar 10, 2022
View reviewed changes

Comment threadsrc/mono/CMakeLists.txt Outdated

vargaz force-pushed the wasm-eh branch from 515b6cc to 14ea0cfCompare March 10, 2022 05:49

kg reviewed Mar 10, 2022
View reviewed changes

Comment threadsrc/mono/mono/mini/CMakeLists.txt Outdated
kgMar 10, 2022
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💃🎉🎊🥳

kg reviewed Mar 10, 2022
View reviewed changes

Comment threadsrc/mono/mono/mini/mini-llvm.c Outdated

kg reviewed Mar 10, 2022
View reviewed changes

Comment threadsrc/mono/wasm/build/WasmApp.Native.targets Outdated

vargaz force-pushed the wasm-eh branch from 14ea0cf to 2ed91d5Compare March 10, 2022 07:52

radekdoulik approved these changes Mar 10, 2022
View reviewed changes

Add missing quote
8ee5fad

lambdageek reviewed Mar 10, 2022
View reviewed changes

Comment threadsrc/mono/mono/mini/CMakeLists.txt

lambdageek reviewed Mar 10, 2022
View reviewed changes

Comment threadsrc/mono/CMakeLists.txt Outdated

Update src/mono/mono/mini/CMakeLists.txt
ae8fe27
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>

Update src/mono/CMakeLists.txt
cde0b79
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>

radical reviewed Mar 10, 2022
View reviewed changes

vargaz merged commit 3e2d483 into dotnet:mainMar 11, 2022

vargaz deleted the wasm-eh branch March 11, 2022 16:50

ghost locked as resolved and limited conversation to collaborators Apr 10, 2022

	<_EmccCommonFlagsInclude="-s DISABLE_EXCEPTION_CATCHING=0"Condition="'$(WasmExceptionHandling)' == 'false'" />
	<_EmccCommonFlagsInclude="-s DISABLE_EXCEPTION_CATCHING=0"Condition="'$(WasmExceptionHandling)' != 'true'" />

Uh oh!

Conversation

vargaz commented Mar 10, 2022

Uh oh!

Uh oh!

vargaz commented Mar 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

radekdoulik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radekdoulik commented Mar 10, 2022

Uh oh!

radekdoulik commented Mar 10, 2022

Uh oh!

azure-pipelinesBot commented Mar 10, 2022

Uh oh!

Uh oh!

Uh oh!

SamMonoRT commented Mar 10, 2022

Uh oh!

vargaz commented Mar 10, 2022

Uh oh!

azure-pipelinesBot commented Mar 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vargaz commented Mar 11, 2022

Uh oh!

vargaz commented Mar 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants