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

Potential perf improvements for Mono AOT #76318

Closed
adamsitnik opened this issue Sep 28, 2022 · 7 comments
Closed

Potential perf improvements for Mono AOT #76318

adamsitnik opened this issue Sep 28, 2022 · 7 comments
Labels
Milestone

Comments

@adamsitnik
Copy link
Member

#73768 has brought a lot of improvements for CLR, but regressed Mono. It was "reverted" in #75917 to fix Mono.

When looking at the WASM and Mono data in the Reporting System I've noticed that Mono AOT (regular one, not WASM) was actually benefiting from #73768

Example:

Span.IndexOf for Mono AOT x64

image

I am not 100% sure that it applies to all architectures, as Mono AOT arm64 has not received any new data inputs in the last few weeks (cc @naricc @DrewScoggins).

It does not apply to Mono Interpeter:

image

@vargaz @jkotas should we modify the #if defs introduced in #75917 to not be applied to Mono AOT? I think that doing this for main branch should be relatively safe (as long as we benchmark the change before merging).

@adamsitnik adamsitnik added this to the 8.0.0 milestone Sep 28, 2022
@adamsitnik
Copy link
Member Author

We could also apply it to WASM AOT config (#75801)

@jkotas
Copy link
Member

jkotas commented Sep 28, 2022

We do not build separate libraries for Mono AOT vs. non-AOT config. We have one set of managed libraries per architecture, for all codegen strategies. You would not be able to do this using #ifdef. It would need to be some sort of new intrinsic and the check done dynamically.

Also, there are mixed partially AOTed configs where the AOT vs. non-AOT is difference is per method. It makes the switch even harder.

@naricc
Copy link
Member

naricc commented Sep 28, 2022

I don't think there is a way to distinguish aot/non-aot in an #ifdef, is there?

@lewing

@jkotas
Copy link
Member

jkotas commented Sep 28, 2022

I think this can be closed.

We need to fix the Mono limitations that lead to the bad perf regressions, and delete the forked implementation that was added as a workaround for .NET 7. It is ok to take small perf regressions in some scenarios.

@jeffhandley
Copy link
Member

Great; thanks for the clarification that we cannot distinguish aot/non-aot in that manner. @adamsitnik and I wanted to confirm we weren't overlooking a different way to dial this for 7.0.

@lewing
Copy link
Member

lewing commented Sep 28, 2022

#76033 will likely fix the (most of) the wasm AOT case were we to revert but we don't yet have a solution for the interpreter

@adamsitnik
Copy link
Member Author

@lewing do we have an issue for tracking this? #76033 has closed #75801 and I can't find interpreter issue

@ghost ghost locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants