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

Add Fragment.OnCreateAnimation(int32, bool, int32) to the list that saves us from a System.Reflection.Emit call #7212

Closed
PureWeen opened this issue Jul 27, 2022 · 5 comments · Fixed by #7267
Assignees
Labels
Area: App Runtime Issues in `libmonodroid.so`.

Comments

@PureWeen
Copy link
Member

PureWeen commented Jul 27, 2022

Android application type

.NET 7.0 MAUI
Android for .NET (net7.0-android)

Affected platform version

.NET 7

Description

I'm currently generating the AOT Profiles for non shell apps in .NET MAUI

There's one place where it's currently falling back to System.Reflection.Emit

https://github.com/dotnet/maui/blob/0062faeba1dd391b1468798cd29299403b9b67f0/src/Core/src/Platform/Android/Navigation/NavigationViewFragment.cs#L88

2022-07-25 11:25:06.254 14932-14932/? D/monodroid-assembly: Falling back to System.Reflection.Emit for delegate type '_JniMarshal_PPIZI_L': IntPtr n_OnCreateAnimation_IZI(IntPtr, IntPtr, Int32, Boolean, Int32)

We only need these changes for NET7

Steps to Reproduce

If you run the following project ShellVsNonShell.zip

with the following logging enabled then you should see it popup in logcat

adb shell setprop debug.mono.log default,assembly,mono_log_level=debug,mono_log_mask=all

You might also see one around OnPageScrolled that's ok to ignore. We have a fix in the MAUI repository to remove this code path.

Did you find any workaround?

No response

Relevant log output

2022-07-25 11:25:06.254 14932-14932/? D/monodroid-assembly: Falling back to System.Reflection.Emit for delegate type '_JniMarshal_PPIZI_L': IntPtr n_OnCreateAnimation_IZI(IntPtr, IntPtr, Int32, Boolean, Int32)
@PureWeen PureWeen added Area: App Runtime Issues in `libmonodroid.so`. needs-triage Issues that need to be assigned. labels Jul 27, 2022
@jonathanpeppers jonathanpeppers removed the needs-triage Issues that need to be assigned. label Jul 27, 2022
@jonathanpeppers jonathanpeppers added this to the .NET 7 milestone Jul 27, 2022
@jonpryor
Copy link
Member

@PureWeen: We can't add all possible method signatures (there are hundreds in Mono.Android.dll alone), so the question is: where do we draw the line?

The current "line" is "used by our default templates", specifically all android and maui templates.

It sounds like this signature isn't used by the current default templates. As such, I am inclined to skip this signature.

In .NET 8, this should be moot, as we should be using LLVM Marshal Methods (e1af958), which avoids the need for System.Reflection.Emit wrappers at runtime.

@PureWeen
Copy link
Member Author

@jonpryor This signature is used on MAUI for users that don't use Shell for their default experience so it'll be a fairly common scenario for users migrating over from XF to MAUI

This is the only addition that would be a nice to have for NET7

@jonathanpeppers
Copy link
Member

Is there a non-Shell template? Do we have an idea how many opt into this?

@PureWeen
Copy link
Member Author

PureWeen commented Aug 15, 2022

@jonathanpeppers as far as I know we don't have a way to extract data that indicates the type of elements/controls that users use.

The updates I made last time for our AOT profile represent the two possible page structures that users go with when building a MAUI/XF app dotnet/maui#8941. This particular method will get invoked for any scenario where a user uses a NavigatePage. I don't completely know the impact of adding this so I'd say

  • If I promise to not ask for any more additions for NET7 and it's easy enough to just add this one then can we? :-)
  • If adding this is going to cause a windfall of issues that results in a lot of extra work in order for us to stay below the APK sizes we want to stay below then we can just ignore since it's irrelevant in NET8.

My hope for NET8 is that both shell/non shell follow all the same platform paths as well.

@jonathanpeppers
Copy link
Member

So this is in the AOT profile here:

https://github.com/dotnet/maui/blob/64a5a588a8c4c1413f43bb41ba488214abb96d25/.nuspec/maui.aotprofile.txt#L7107-L7113

Introduced in: dotnet/maui@15df4ed

We should probably just fix this one, and maybe it's the last one for .NET 7? Unless a new one pops up in the templates or .NET Podcast app.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Aug 16, 2022
Context: dotnet/maui@15df4ed
Fixes: dotnet#7212

In the app we're recording AOT profiles for .NET MAUI, we added a
`FlyoutPage` and navigated to it. This was good because it added
common scenarios to the profile:

* Navigation from one page to another
* A non-Shell API, `FlyoutPage`
* `FlyoutPage` also implicitly uses `NavigationPage`?

This caused the app to start using System.Reflection.Emit and logging:

    08-16 09:46:24.687 31517 31517 D monodroid-assembly: Falling back to System.Reflection.Emit for delegate type '_JniMarshal_PPIZI_L': IntPtr
n_OnCreateAnimation_IZI(IntPtr, IntPtr, Int32, Boolean, Int32)

I added a case for `_JniMarshal_PPIZI_L`, this log message is now gone
when I build and run this app.

I think this should be the last one of these we add, unless one
appears from either:

* `dotnet new maui` or `dotnet new maui-blazor`
* [.NET Podcast app][0]

[0]: https://github.com/microsoft/dotnet-podcasts
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Aug 17, 2022
Context: dotnet/maui@15df4ed
Fixes: dotnet#7212

In the app we're recording AOT profiles for .NET MAUI, we added a
`FlyoutPage` and navigated to it. This was good because it added
common scenarios to the profile:

* Navigation from one page to another
* A non-Shell API, `FlyoutPage`
* `FlyoutPage` also implicitly uses `NavigationPage`?

This caused the app to start using System.Reflection.Emit and logging:

    08-16 09:46:24.687 31517 31517 D monodroid-assembly: Falling back to System.Reflection.Emit for delegate type '_JniMarshal_PPIZI_L': IntPtr
n_OnCreateAnimation_IZI(IntPtr, IntPtr, Int32, Boolean, Int32)

I added a case for `_JniMarshal_PPIZI_L`, this log message is now gone
when I build and run this app.

I think this should be the last one of these we add, unless one
appears from either:

* `dotnet new maui` or `dotnet new maui-blazor`
* [.NET Podcast app][0]

[0]: https://github.com/microsoft/dotnet-podcasts
jonpryor pushed a commit that referenced this issue Aug 22, 2022
Fixes: #7212

Context: 32cff43
Context: dotnet/maui@15df4ed

In the app we're recording AOT profiles for .NET MAUI, we added a
`FlyoutPage` and navigated to  it. This was good because it added
common scenarios to the profile:

  * Navigation from one page to another
  * A non-Shell API, `FlyoutPage`
  * `FlyoutPage` also implicitly uses `NavigationPage`?

This caused the app to start using System.Reflection.Emit and logging:

	D monodroid-assembly: Falling back to System.Reflection.Emit for delegate type '_JniMarshal_PPIZI_L': IntPtr n_OnCreateAnimation_IZI(IntPtr, IntPtr, Int32, Boolean, Int32)

I added a case for `_JniMarshal_PPIZI_L`, and this log message is now
gone when I build and run this app.

I think this should be the last one of these we add, unless one
appears from either:

  * `dotnet new maui` or `dotnet new maui-blazor`
  * [.NET Podcast app][0]

[0]: https://github.com/microsoft/dotnet-podcasts
jonathanpeppers added a commit that referenced this issue Aug 22, 2022
Fixes: #7212

Context: 32cff43
Context: dotnet/maui@15df4ed

In the app we're recording AOT profiles for .NET MAUI, we added a
`FlyoutPage` and navigated to  it. This was good because it added
common scenarios to the profile:

  * Navigation from one page to another
  * A non-Shell API, `FlyoutPage`
  * `FlyoutPage` also implicitly uses `NavigationPage`?

This caused the app to start using System.Reflection.Emit and logging:

	D monodroid-assembly: Falling back to System.Reflection.Emit for delegate type '_JniMarshal_PPIZI_L': IntPtr n_OnCreateAnimation_IZI(IntPtr, IntPtr, Int32, Boolean, Int32)

I added a case for `_JniMarshal_PPIZI_L`, and this log message is now
gone when I build and run this app.

I think this should be the last one of these we add, unless one
appears from either:

  * `dotnet new maui` or `dotnet new maui-blazor`
  * [.NET Podcast app][0]

[0]: https://github.com/microsoft/dotnet-podcasts
@ghost ghost locked as resolved and limited conversation to collaborators Sep 22, 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`.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants