-
Notifications
You must be signed in to change notification settings - Fork 16
Dotnet Uses Legacy Wasm Exceptions. #188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for webkit-jetstream-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Add a pass to the build script that replaces them with the non-legacy execptions. This doesn't seem to impact performance. The dotnet runtime still seems to generate one module/function that has a legacy exception but that function is never called. It's also somewhat degenerate since it's just `(try catch_all)` and has no other body. This doesn't seem to make a difference for the score in all CLIs and we don't want legacy exception usage in JetStream to be the thing that keeps the legacy wasm exceptions "feature" alive. Note: I didn't run the build script. I just ran the wasm-opt commands on the build products. Mostly because when I recompile the code doesn't run or is 5x slower than the previous binaries.
fe8bc49
to
3de5730
Compare
echo "Copying symbol maps..." | tee -a "$BUILD_LOG" | ||
cp ./src/dotnet/obj/Release/net9.0/wasm/for-publish/dotnet.native.js.symbols ./build-interp/wwwroot/_framework/ | ||
|
||
for wasmFile in $(find "./build-interp" -type f -name "*.wasm"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment explaining this step.
Generally SGTM to avoid depending on legacy EH, but:
|
What's the name of the function ? Or how do I find it ? |
Agreed, would be good to know this. I would also highly encourage switching to
I think it might have been a stale build artifact from a prior build. If I remove the additions to
It looks like they're roughly the same size. For
|
It doesn't appear to have a name anywhere. The entire module is just
which makes me think it's some kind of dynamically generated module in the interp variant of the benchmark. |
I think we are using legacy feature detection but should be using new one |
Oh interesting, I think the benchmark still executes legacy exceptions, at least enough to trigger a tier up to our optimizing tiers for aot. I added some logging but I don't know which module they're coming from.
|
Looking deeper into the issue. Net9 and Net10 both use Emscripten version 3.1.56, which doesn't support final EH spec. Our JetStream benchmark is compiled with Net9. I think that we would need to upgrade Emscripten to at least 4.0.0 in order to use I created dotnet/runtime#119961 for that. |
Add a pass to the build script that replaces them with the non-legacy execptions. This doesn't seem to impact performance. The dotnet runtime still seems to generate one module/function that has a legacy exception but that function is never called. It's also somewhat degenerate since it's just
(try catch_all)
and has no other body.This doesn't seem to make a difference for the score in all CLIs and we don't want legacy exception usage in JetStream to be the thing that keeps the legacy wasm exceptions "feature" alive.
Note: I didn't run the build script. I just ran the wasm-opt commands on the build products. Mostly because when I recompile the code doesn't run or is 5x slower than the previous binaries.