Skip to content
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

[wasm] Intrinsics for blazor #102670

Merged
merged 6 commits into from
Jun 19, 2024
Merged

[wasm] Intrinsics for blazor #102670

merged 6 commits into from
Jun 19, 2024

Conversation

kg
Copy link
Member

@kg kg commented May 24, 2024

During blazor startup these 3 methods account for a bit over 100 compiled methods if I analyzed the trace correctly. Since interp codegen dominates blazor startup (50-75% depending on how you measure it) my hope is that pruning methods out will reduce overall startup time. These three seemed like easy targets.

Add is commented out because I can't get it to work. Will be relying on help for that one unless a flash of insight helps me figure out how to fix it.

Should I add a CEQ0_I8 for IsNullRef? I'm not sure it's worth adding a new opcode just for that.

Copy link
Contributor

Tagging subscribers to this area: @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

@kg
Copy link
Member Author

kg commented May 28, 2024

Add intrinsic fixed, thanks @lambdageek for spotting the bug

@BrzVlad
Copy link
Member

BrzVlad commented May 28, 2024

Does this have any measurable impact ?

@kg
Copy link
Member Author

kg commented May 28, 2024

Does this have any measurable impact ?

We generate ~100 fewer methods during blazor startup. My timing data is very noisy, so I can't say 'this makes startup 1ms faster' with any confidence.

@kg
Copy link
Member Author

kg commented Jun 6, 2024

The interp PGO tests were relying on us tiering one of these methods, oops :-)

@kg kg force-pushed the interp-blazor-intrinsics-1 branch from d80dc29 to d970374 Compare June 6, 2024 22:30
@kg kg requested review from maraf and ilonatommy as code owners June 6, 2024 22:30
@kg
Copy link
Member Author

kg commented Jun 7, 2024

The osx interp failures look real, but I can't make sense of them. Does anyone have any ideas for why it would only be failing there?

i.e. https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-102670-merge-a8c9cc274f16491385/Directed_3/1/console.34f65d92.log?helixlogtype=result

@kg kg force-pushed the interp-blazor-intrinsics-1 branch from c37ad60 to 71cf842 Compare June 10, 2024 22:10
@kg
Copy link
Member Author

kg commented Jun 11, 2024

I can't reproduce the interp failures in either debug or release configurations. I'm doing:

clear; MONO_ENV_OPTIONS=--interpreter ~/Projects/dotnet-runtime-wasm/dotnet.sh build /bl /t:Test /p:RuntimeFlavor=mono /p:RuntimeVariant=monointerpreter /p:Configuration=Debug /p:EnableAggressiveTrimming=true /p:RunAOTCompilation=false /p:MonoEnvOptions="--interpreter" ~/Projects/dotnet-runtime-wasm/src/libraries/Common/tests/Common.Tests.csproj

but that failing suite along with other suites like S.R.T passes.

BrzVlad added 2 commits June 18, 2024 13:17
Don't allocate unnecessary temp var. Don't generate mul if type size is 1.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants