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

TypeManager.RegisterType() doesn't use type mappings #1831

Closed
jonpryor opened this issue Jun 15, 2018 · 0 comments · Fixed by #1890
Closed

TypeManager.RegisterType() doesn't use type mappings #1831

jonpryor opened this issue Jun 15, 2018 · 0 comments · Fixed by #1890
Assignees
Labels
Area: App Runtime Issues in `libmonodroid.so`. Area: Performance Issues with performance. vs-sync For internal use only; creates a VSTS "mirror" issue.
Milestone

Comments

@jonpryor
Copy link
Member

jonpryor commented Jun 15, 2018

When recently profiling Xamarin.Android application startup, we observed that a callstack "rooted" in TypeManager.RegisterType() was using LINQ.

However, the real problem is that TypeManager.RegisterType() doesn't use our type mapping files!

https://github.com/xamarin/xamarin-android/blob/bb8183c8c16667b18ba9197e78294b17a0af532c/src/Mono.Android/Java.Interop/TypeManager.cs#L310-L326

It should at minimum be using JNIEnv.GetJniName(Type). Then, we need to make sure that monodroid_typemap_managed_to_java() is actually returning values, so that we don't hit the (slow, LINQ-using) JavaNativeTypeManager.ToJniName() fallback path.

VS bug #635223

@jonpryor jonpryor added Area: App Runtime Issues in `libmonodroid.so`. Area: Performance Issues with performance. labels Jun 15, 2018
@jonpryor jonpryor added this to To do in Xamarin.Android Runtime Optimizations via automation Jun 15, 2018
@jonpryor jonpryor added this to the d15-9 milestone Jun 15, 2018
@JonDouglas JonDouglas added the vs-sync For internal use only; creates a VSTS "mirror" issue. label Jun 18, 2018
@xamarin-release-manager xamarin-release-manager modified the milestones: d15-9, d15-8 Jun 20, 2018
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this issue Jun 27, 2018
This fixes dotnet#1831

Make sure we use the code path, which uses the `typename.*` maps. It
is much faster than `JavaNativeTypeManager.ToJniName`.

Tested with x86 emulator again. It saves 17ms in XA template and 26ms
in XamlSamples app. So it scales well.

It also decreases JIT usage. XA template: *Compiled methods* 1954 to
1880.
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this issue Jun 27, 2018
This fixes dotnet#1831

Make sure we use the code path, which uses the `typename.*` maps. It
is much faster than `JavaNativeTypeManager.ToJniName`.

Tested with x86 emulator again. It saves 17ms (from 18ms to 1ms) in XA
template and 26ms (from 28ms to 2ms) in XamlSamples app. So it scales
well.

It also decreases JIT usage. XA template: *Compiled methods* 1954 to
1880.
Xamarin.Android Runtime Optimizations automation moved this from To do to Done Jun 28, 2018
jonpryor pushed a commit that referenced this issue Jun 28, 2018
Fixes: #1831

Make sure we use the code path, which uses the `typename.*` maps.
It is much faster than `JavaNativeTypeManager.ToJniName()`.

Tested with x86 emulator again. It saves 17ms (from 18ms to 1ms) in
XA template and 26ms (from 28ms to 2ms) in XamlSamples app.

This change also decreases JIT usage: in the XA template app,
*Compiled methods* is reduced from 1954 to 1880 methods.
radekdoulik added a commit that referenced this issue Jun 29, 2018
Fixes: #1831

Make sure we use the code path, which uses the `typename.*` maps.
It is much faster than `JavaNativeTypeManager.ToJniName()`.

Tested with x86 emulator again. It saves 17ms (from 18ms to 1ms) in
XA template and 26ms (from 28ms to 2ms) in XamlSamples app.

This change also decreases JIT usage: in the XA template app,
*Compiled methods* is reduced from 1954 to 1880 methods.
@ghost ghost locked as resolved and limited conversation to collaborators Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App Runtime Issues in `libmonodroid.so`. Area: Performance Issues with performance. vs-sync For internal use only; creates a VSTS "mirror" issue.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants