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] Optimize jiterpreter type cast opcodes & add zero page optimizations #86928

Merged
merged 14 commits into from
Jun 1, 2023

Conversation

kg
Copy link
Member

@kg kg commented May 30, 2023

This PR inlines a sizable portion of MINT_CASTCLASS/MINT_ISINST logic into jiterpreter traces to create fast paths for exact matches/null pointers that avoids running all the type check machinery we normally would run. Based on my instrumentation these fast paths will be used ~30% of the time, and measurements show a 1-3% improvement for the json section of browser-bench (this is pretty hard to measure, I may try to construct a decent synthetic benchmark for this).

This PR also applies zero page optimizations if available, by fusing the null check into the obj->vtable load. The helper code is all tweaked so that in the event a check fails due to a null ptr (null inputs are fairly uncommon in the instrumentation) it is converted into a success since MINT_CASTCLASS and MINT_ISINST both succeed for null instead of throwing.

It may be worth inlining the interface check entirely, but I haven't done the work to test that yet. The existence of special interfaces for arrays makes that harder to do.

This PR also fully inlines the implementation of MINT_UNBOX.

@kg kg added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) arch-wasm WebAssembly architecture area-Codegen-Jiterpreter-mono labels May 30, 2023
@ghost ghost assigned kg May 30, 2023
@ghost
Copy link

ghost commented May 30, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Opening as draft because it relies on #86403.

This PR inlines a sizable portion of CASTCLASS/ISINST logic into jiterpreter traces to create fast paths for exact matches/null pointers that avoids running all the type check machinery we normally would run. Based on my instrumentation these fast paths will be used ~30% of the time, and measurements show a 1-3% improvement for the json section of browser-bench (this is pretty hard to measure, I may try to construct a decent synthetic benchmark for this).

This PR also applies zero page optimizations if available, by fusing the null check into the obj->vtable load. The helper code is all tweaked so that in the event a check fails due to a null ptr (null inputs are fairly uncommon in the instrumentation) it is converted into a success since CASTCLASS and ISINST both succeed for null.

It may be worth inlining the interface check entirely, but I haven't done the work to test that yet. The existence of special interfaces for arrays makes that harder to do.

Author: kg
Assignees: -
Labels:

NO-MERGE, arch-wasm, area-Codegen-Jiterpreter-mono

Milestone: -

@vargaz
Copy link
Contributor

vargaz commented May 31, 2023

isinst doesn't succeed on nulls.

@kg
Copy link
Member Author

kg commented May 31, 2023

isinst doesn't succeed on nulls.

Thanks, I'll revise it

@kg kg force-pushed the wasm-jiterp-zero-page-2 branch from 609ed84 to 31d5a91 Compare June 1, 2023 08:00
@kg
Copy link
Member Author

kg commented Jun 1, 2023

I was puzzled that none of the scenarios I expected to get faster were getting faster with these changes. It turned out that the scenarios I was testing were actually relying on MINT_UNBOX. So I wrote some toy BDN microbenchmarks to use and also optimized the unbox operation. Some comparison timings for main vs this PR:

scenario main this PR relative %
(int)obj 1.285ms 1.159ms 90%
if (obj is int value) 1.62ms 1.218ms 75%

@kg kg removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 1, 2023
@kg kg marked this pull request as ready for review June 1, 2023 08:08
@kg kg merged commit 5953aab into dotnet:main Jun 1, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants