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

[mono] Raise inlining length limit in the interpreter from 20 to 30 #83490

Merged
merged 1 commit into from
Mar 16, 2023

Conversation

kg
Copy link
Member

@kg kg commented Mar 16, 2023

In the past raising the inlining length limit provided some big easy wins on wasm, and it also allows the jiterpreter to work better. Some critical stuff is in this 'over 20 bytes but under 30 bytes' range. For example, V8.Crypto from the dotnet/performance suite is significantly bottlenecked on ListX.get_Item, which just calls List.get_Item.

Previously when I tried this (as part of another PR) it caused one specific obscure test to fail on CI that didn't fail locally, so we'll see if it fails again and I can figure it out...

… enough to allow List<T>.get_Item to be inlined, among other things
@ghost
Copy link

ghost commented Mar 16, 2023

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

Issue Details

In the past raising the inlining length limit provided some big easy wins on wasm, and it also allows the jiterpreter to work better. Some critical stuff like List.get_Item is in this 'over 20 bytes but under 30 bytes' range. For example, V8.Crypto from the dotnet/performance suite is significantly bottlenecked on ListX.get_Item, which just calls List.get_Item.

Previously when I tried this (as part of another PR) it caused one specific obscure test to fail on CI that didn't fail locally, so we'll see if it fails again and I can figure it out...

Author: kg
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@kg
Copy link
Member Author

kg commented Mar 16, 2023

Looks like it still breaks. It used to cause a specific assertion (I'll have to see if I can find it) but now it causes random segfaults. :(

@kg
Copy link
Member Author

kg commented Mar 16, 2023

Looks like it still breaks. It used to cause a specific assertion (I'll have to see if I can find it) but now it causes random segfaults. :(

Looking closer, one segfault is a known issue, and the other is in a lane that shouldn't engage the interpreter, so they might be random CI flakes. Will re-run.

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.

2 participants