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

Consider enabling the interpreter by default for release builds #13019

Open
rolfbjarne opened this issue Jan 31, 2023 · 37 comments
Open

Consider enabling the interpreter by default for release builds #13019

rolfbjarne opened this issue Jan 31, 2023 · 37 comments
Assignees
Labels
area-publishing Issues with the app packaging/publishing process (ipk/apk/msix/trimming) p/1 Work that is critical for the release, but we could probably ship without platform/iOS 🍎 s/triaged Issue has been reviewed t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented Jan 31, 2023

Description

Currently the interpreter is enabled for Debug builds:

<UseInterpreter Condition="'$(UseInterpreter)' == '' and '$(Configuration)' == 'Debug'">True</UseInterpreter>
<MtouchLink Condition="'$(MtouchLink)' == '' and '$(Configuration)' == 'Debug' and '$(UseInterpreter)' == 'true'">None</MtouchLink>

At least for iOS and Mac Catalyst, we might want to make this the default for Release builds as well, because I'm seeing a lot of customers running into problems where their app crashes once they switch to Release builds, because they used some feature in their app that requires the interpreter.

Some customers don't even test their apps in Release mode, they go straight to trying to publish and they're obviously rather upset when their apps are rejected because they crash at launch.

While the interpreter might slow things down, the performance will be fine for most apps, and if someone really needs the additional performance they can disable the interpreter.

The main downside might be that crash reports will be rather useless, because all managed stack frames will just show up as the interpreter doing "stuff".

Steps to Reproduce

Triage bugs for a while.

Alternatively read about customer's experiences online (Twitter, blog posts, etc.).

Example blog post: https://www.andreasnesheim.no/my-experience-with-migrating-my-app-from-xamarin-forms-to-net-maui/

Link to public reproduction project repository

N/A

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

iOS, macOS

Affected platform versions

All so far

Did you find any workaround?

  1. Better documentation.
  2. Better error messages.

Relevant log output

No response

References

@rolfbjarne rolfbjarne added the t/bug Something isn't working label Jan 31, 2023
@symbiogenesis
Copy link
Contributor

Is there a list of known deficiencies in the current release mode tooling? I have experienced similar issues.

@Eilon Eilon added area-publishing Issues with the app packaging/publishing process (ipk/apk/msix/trimming) legacy-area-perf Startup / Runtime performance labels Jan 31, 2023
@noque-lind
Copy link

I would like to know which features require the Interpreter. Because we have run into the same thing. UseIntepreter works for release mode, otherwise it crashes at launch. I'd like to try the app without the interpreter because our client is not really happy with the performance compared to how the app ran in Xamarin Forms.

@Ghevi
Copy link

Ghevi commented Feb 1, 2023

@noque-lind @symbiogenesis
Can't provide a list but i know that Entity Framework Core requires the interpreter on iOS
9176, 12537

@mattleibow
Copy link
Member

@jonathanpeppers this feels like a nice way to make customers happy and still have all the power to get that performance?

@rolfbjarne do we know what part of entity framework needs the interpreter? Technically we can make all platforms use a slower code path because a popular library needs it. However, maybe entity framework can provide an alternate code path for AOT-only environments?

@jonathanpeppers
Copy link
Member

@mattleibow just don't do this for Android, because it disables the JIT. iOS/MacCatalyst can't JIT, so the only downside is whatever the increase to app size. Plus there might be some things about weird stack traces Rolf mentions above.

@rolfbjarne
Copy link
Member Author

@mattleibow

@rolfbjarne do we know what part of entity framework needs the interpreter? Technically we can make all platforms use a slower code path because a popular library needs it. However, maybe entity framework can provide an alternate code path for AOT-only environments?

I haven't investigated why entity framework needs the interpreter. I also believe there are other libraries that needs it, so it's not only entity framework.

@symbiogenesis
Copy link
Contributor

symbiogenesis commented Feb 8, 2023

Linking this bug for context. It is what I have marked on the line that I set the interpreter on for iOS. A couple of you have already commented there, but others might not be aware.

I am using Entity Framework, and that is likely a main reason why I set it.

dotnet/runtime#74015

@PureWeen PureWeen added this to the .NET 8 Planning milestone Feb 8, 2023
@symbiogenesis
Copy link
Contributor

symbiogenesis commented Feb 16, 2023

Here is an example of something that seems to work for me on Debug mode, but fails on Release mode

In this branch there is a converter that is marked as internal. On Windows and/or in Debug mode it works, but on Android release mode it crashes with:

System.MethodAccessException: Method Maui.DataGrid.SortDataTypeConverter..ctor()' is inaccessible from method YourProject.Views.UsersPage.InitializeComponent()' at YourProject.Views.UsersPage.InitializeComponent...

To reproduce the error, just run the sample project in Android release mode. It should crash on launch.

Changing SortDataTypeConverter to public fixes it, but I don't see why Release mode should be special, so it seems like a bug.

To retrieve the exception in Release mode I have subscribed to AppDomain.CurrentDomain.UnhandledException and TaskScheduler.UnobservedTaskException from the MainActivity in the Android platform code, and logged to the console from there.

https://github.com/symbiogenesis/Maui.DataGrid/tree/android-release-mode

@rdavisau
Copy link
Contributor

Is it possible in new dotnet to do the equivalent of the Xamarin.iOS --interpreter=-all (aot everything, but allow the interpreter to handle codegen)?

@rolfbjarne
Copy link
Member Author

Is it possible in new dotnet to do the equivalent of the Xamarin.iOS --interpreter=-all (aot everything, but allow the interpreter to handle codegen)?

This should work the same:

<PropertyGroup>
    <MtouchInterpreter>-all</MtouchInterpreter>
</PropertyGroup>

@Streamtech-dev
Copy link

Streamtech-dev commented Apr 13, 2023

I was experiencing crashes when writing to sqlite in release mode on ios. Using the interpreter stopped that happening. It also made some other minor things work again like colour changes to controls. I only experienced these problems with ios, which worked fine in debug mode. Other platforms all worked OK in both debug and release. I used the example from above and changed Debug to Release to make it work.

<PropertyGroup>
	<UseInterpreter Condition="'$(UseInterpreter)' == '' and '$(Configuration)' == 'Release'">True</UseInterpreter>
	<MtouchLink Condition="'$(MtouchLink)' == '' and '$(Configuration)' == 'Release' and '$(UseInterpreter)' == 'true'">None</MtouchLink>
</PropertyGroup>

@mattleibow
Copy link
Member

Using the interpreter by default may cause perf issues for everyone - even those who don't need it.

The opposite is that some portion of the users may have issues in release and waste time trying to fix it.

Both are bad, but which is the most significant number? Is there a way to have partial interpretation so that the libraries that have issues can use the interpreter but the rest of the app is AOT?

Also, have we been able to determine what causes the issues? Maybe there is some method use in EF for example that needs to have an AOT friendly implementation on some platforms?

TL;DR: I am suggesting that if there is an issue with a few libraries for a few people then maybe we can fix those instead of negatively impacting performance for all users.

@cdavidyoung
Copy link

"The opposite is that some portion of the users may have issues in release and waste time trying to fix it."

That is my case. My iOS app CacheAll worked perfectly in debug and so I published the release version on the app store. This also worked fine until you went to save and then it would silently crash with no crash report.

Fortunately, a little Googling revealed the true trick, which I put in the iOS release configuration of the .csproj file. I verified that this corrected the problem by first running in VS Mac the release version without this fix and noting that the app crashed when saving. Then I added this fix, did a clean all and deleted bin and obj, and ran again in VS Mac. Now the app does not crash.

Subsequently I uploaded to the app store and downloaded the app via TestFlight, which also showed that the crash was fixed.

BTW, so far I have not noticed a performance issue. @rolfbjarne Does UseInterpreter only affect the code that was crashing or does it affect the execution of the app overall?

@Rory-Reid
Copy link

I've just hit this myself after developing on mac for months but having no mac users, until today, precisely because it works in debug but not release.

While I understand the concerns for performance, it's also not a great out of the box experience for MAUI, and given that it's not an obvious tradeoff to consider (I don't remember seeing it in any documentation or tutorials I hit, anecdotally) it feels like it could be quite devastating to run into for people planning to use MAUI for mac and iOS if they depend on a library that needs it (for me it was Microsoft.Data.Sqlite I believe).

Tough call, but I would like this to be more obvious/easier to deal with than it is today. The Microsoft MAUI csproj templates have a bespoke commented-out-by-default section for tizen, in a similar vein, it might be nice to see something like this added there if not enabled by default:

<!-- Uncomment to enable the interpreter. This is needed in release if you or your dependencies use reflection -->
<!-- <PropertyGroup Condition="$(TargetFramework.Contains('-maccatalyst')) Or $(TargetFramework.Contains('-ios'))">-->
<!--     <UseInterpreter>true</UseInterpreter>-->
<!-- </PropertyGroup>-->

@mattleibow
Copy link
Member

Yeah,you changed me :) I like this idea... We can have it enabled actually and then maybe have some publishing doc that focuses on optimization where we can talk linking, reflection and interpreters...

Should this be on from the workloads and you opt out... This will affect existing users. Or should this be a templates thing and only be something for new users.

Today we enable for release builds implicitly so maybe we just always set it for ios/maccatalyst and just release for Android. That is semi inconsistent but might be the best. The the project can override however they like but at least the default always works.

@mattleibow
Copy link
Member

@davidbritch @rolfbjarne @jonathanpeppers do we have a doc on all the options that are good for optimizing maui apps? Obviously there are millions of buttons, but more the ones that most people use.

Should the interpreter be there along with the pros and cons?

Rory-Reid added a commit to Rory-Reid/Basemix that referenced this issue Apr 30, 2023
Turns out this wasn't working because macs don't have, disallow or
prevent some stuff. I've not nailed down exactly what, it seems to be
code generation and reflection oriented.

Unfortunately, that's very much needed for the Sqlite adapter, probably
other libs too.

The way the debug build works is it enables "The Interpreter" which I
have actually struggled to find much information on from a quick google,
but I guess it fills in where necessary. Downside being it's slower, but
slower is better than "the app doesn't work at all".

Here, I conditionally enable the interpreter for release builds now.

I've joined a thread about this on the MAUI Repo here:
dotnet/maui#13019 (comment)
@RobTF
Copy link

RobTF commented Jun 21, 2023

To the comment;

Some customers don't even test their apps in Release mode, they go straight to trying to publish and they're obviously rather upset when their apps are rejected because they crash at launch.

This sort of points the finger at customers, rightly so in many cases I'm sure - however there are a few nuances here;

  • Rightly or wrongly, we as .NET developers have been trained to think that building in release mode is intrinsically safe and simply provides a performance increase. It is so rare I can't even think of a case where other frameworks such as WinForms, WPF, ASP.NET etc. ever have this issue of release mode being significantly less stable, especially in weird subtle ways. The significance and danger of this should be documented more clearly - more fairly seasoned Xamarin developers may understand this as it is the same sort of issue which applies to linking, but newcomers likely won't.
  • "Testing our apps" is easy to say, but it might be a job that takes months for any non trivial app. We have enough of that when debugging our own code. I can imagine that even with the best of intentions edge cases may slip through.
  • When the problem does occur - the error traces are garbage. What leads developers to turning the interpreter on is little more than shotgun debugging e.g. "I'll try switching on the crash-less flag". This is a tooling issue.
  • This is not a third party library problem - most of the errors solved by this flag that I've seen stem from bindings.

Quick question: Does the interpreter exist in Xamarin? If not, with it switched on does this negate any of the performance enhancements of MAUI vs. Xamarin? I don't ever recall it being mentioned prior to MAUI.

@jonathanpeppers
Copy link
Member

Rightly or wrongly, we as .NET developers have been trained to think that building in release mode is intrinsically safe

The trimmer does output the message:

Optimizing assemblies for size may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink

This is one source of when Release builds go wrong, but maybe this could be a warning instead of a message?

The interpreter is more of a setting for iOS, as it enables more System.Reflection.Emit-related APIs to work in a JIT-not-allowed environment. There really isn't a reason to turn the interpreter on for Android, as it is only used for hot reload scenarios.

@rolfbjarne
Copy link
Member Author

To the comment;

Some customers don't even test their apps in Release mode, they go straight to trying to publish and they're obviously rather upset when their apps are rejected because they crash at launch.

This sort of points the finger at customers, rightly so in many cases I'm sure

I fully agree with all your comments, and I didn't mean to point any fingers at anyone but ourselves, and I'm sorry if it came off that way.

Quick question: Does the interpreter exist in Xamarin? If not, with it switched on does this negate any of the performance enhancements of MAUI vs. Xamarin? I don't ever recall it being mentioned prior to MAUI.

Yes, the interpreter has been around for a few years now, but it was an experimental feature in Xamarin (you had to opt in).

This meant few people tried it, and even fewer ran into differences between Debug and Release.

One reason this has become such an annoying problem is because the interpreter is required for Hot Reload (which is new in MAUI), so it was turned on by default for Debug, and thus a lot of people would happily (and rightfully so) code away, using features that would require (by accident or by design) the interpreter.

@rdavisau
Copy link
Contributor

To my understanding, in most (all?) situations, none of the assemblies available at compile time should require the interpreter to execute. "Problematic" libraries emit new code into new dynamic assemblies that only exist at runtime. These would typically be JIT'd, but here need to be interpreted, and (probably/practically) can't be named in your csproj ahead of time.

So I think the solution in dotnet ios, is the same as in Xamarin.iOS - this:

<PropertyGroup>
    <MtouchInterpreter>-all</MtouchInterpreter>
</PropertyGroup>

and it might be a reasonable project default.

@baaaaif
Copy link

baaaaif commented Sep 15, 2023

I became aware of this fix in the EF Core 8 RC1 blog post dotnet/efcore#29725
and now I'm wondering whether this might be related to this topic?
Does this make UseInterpreter obsolete, at least for using EF Core Sqlite for iOS?

(https://devblogs.microsoft.com/dotnet/announcing-ef8-rc1/)

@MichaelLHerman
Copy link

Just for the record, it's not just EF Core. I have also experienced problems that only appear in Release mode when using Autofac with its autogenerated Factory registrations

image

https://autofac.readthedocs.io/en/latest/resolve/relationships.html#dynamic-instantiation-func-b
autofac/Autofac#1398 (comment)

I'll be switching over to Microsoft DI

@dirivero
Copy link

dirivero commented Mar 13, 2024

Any news on this one?
Wouldn't the setting below be a sensible default for iOS as @rdavisau and @rolfbjarne pointed out?

<PropertyGroup Condition="$(TargetFramework.Contains('-ios'))">
    <MtouchInterpreter>-all</MtouchInterpreter>
</PropertyGroup>

Another common scenario that fails is Linq Expressions with more that 2 parameters:
dotnet/runtime#94063

@DDHSchmidt
Copy link

Any news on this one? Wouldn't the setting below be a sensible default for iOS as @rdavisau and @rolfbjarne pointed out?

<PropertyGroup Condition="$(TargetFramework.Contains('-ios'))">
    <MtouchInterpreter>-all</MtouchInterpreter>
</PropertyGroup>

Another common scenario that fails is Linq Expressions with more that 2 parameters: dotnet/runtime#94063

Actually, after some more troubles with that parameter I'd advise against that as a default.
Even with -all we still experienced crashes in our iOS-Release builds, the latest one in the Maui Community Toolkit :/

Finding the root culprit/assembly is a very manual & cumbersome process, even after @rolfbjarne gave me some tips on Discord to speed things up (Kudos for that 👍)
We still put up with <MtouchInterpreter>-all</MtouchInterpreter> because our migrated Maui app is still slow as ass, compared to the old Xamarin version and we need to squeeze out every ounce of performance we can get 😕

NativeAOT is an even bigger pita. The crashes are even more numerous and the stacktraces/hints next to nonexistant.
At the current pace I expect NativeAOT not to be usable for anything but console apps before .Net 11, but I digress...

I would recommend to stick with <UseInterpreter>true</UseInterpreter> as a default for Debug & Release builds unless you consider all MAUI users to be either compiler-experts or masochists.

@mjo151
Copy link

mjo151 commented Mar 18, 2024

I would like to point out one big benefit of AOT that doesn't seem to be discussed: code protection. IL code is stripped when using AOT, and this helps protect the app from reverse engineering. When enabling the interpreter, the IL code is not stripped from the compiled assemblies. I don't think many users are aware of this difference. Some users probably don't care, but others might. I have been down the road of using obfuscation for protection, but it is honestly really painful. In my opinion, this is one of the biggest benefits of using AOT.

In a related point, it appears that IL Stripping and MtouchInterpreter is all or nothing. For example, if you want to use the interpreter for a single assembly and AOT everything else (e.g. -all,System.Collections.Immutable), it appears that IL is NOT stripped from the AOTed assemblies. Maybe that is by design, but I did not expect that to be the case.

@dirivero
Copy link

@mjo151 do you know if IL is not stripped even if you set -all alone?

@RobTF
Copy link

RobTF commented Mar 18, 2024

Whatever the benefits of a AOT, if the downside of it being enabled is that my app doesn't run they don't matter too much.

We view AOT/IL stripping etc. as very, very dodgy once you get out of "hello world" territory and smells more of the result of an academic exercise done within the CLR team than something that should be relied upon in production. Automated tooling that results in potentially changing the meaning of a program resulting in requiring deep manual testing to ensure it hasn't broken anything has massive inherent risk. Its USP seems to be "making stuff faster" but in reality, my customers will be happier with an app which takes another 0.5 seconds to launch but is stable than something that just fails more quickly.

We've had to completely disable AOT in Android production due to weird errors preventing our users booting the app at all. The underlying issue is still open.

@rolfbjarne
Copy link
Member Author

In a related point, it appears that IL Stripping and MtouchInterpreter is all or nothing. For example, if you want to use the interpreter for a single assembly and AOT everything else (e.g. -all,System.Collections.Immutable), it appears that IL is NOT stripped from the AOTed assemblies. Maybe that is by design, but I did not expect that to be the case.

That's by design. If that single interpreted assembly references another assembly that's AOT-compiled, the AOT-compiled assembly might not have AOT-compiled everything the interpreted assembly needs. In these cases the interpreter needs to interpret code from the AOT-compiled assembly, and as such the actual IL to interpret must be present.

@mjo151
Copy link

mjo151 commented Mar 18, 2024

@mjo151 do you know if IL is not stripped even if you set -all alone?

I just tried only the -all option and it looks like the IL code was not stripped.

In a related point, it appears that IL Stripping and MtouchInterpreter is all or nothing. For example, if you want to use the interpreter for a single assembly and AOT everything else (e.g. -all,System.Collections.Immutable), it appears that IL is NOT stripped from the AOTed assemblies. Maybe that is by design, but I did not expect that to be the case.

That's by design. If that single interpreted assembly references another assembly that's AOT-compiled, the AOT-compiled assembly might not have AOT-compiled everything the interpreted assembly needs. In these cases the interpreter needs to interpret code from the AOT-compiled assembly, and as such the actual IL to interpret must be present.

OK, I figured there was a good reason.

@mjo151
Copy link

mjo151 commented Mar 18, 2024

Whatever the benefits of a AOT, if the downside of it being enabled is that my app doesn't run they don't matter too much.

We view AOT/IL stripping etc. as very, very dodgy once you get out of "hello world" territory and smells more of the result of an academic exercise done within the CLR team than something that should be relied upon in production. Automated tooling that results in potentially changing the meaning of a program resulting in requiring deep manual testing to ensure it hasn't broken anything has massive inherent risk. Its USP seems to be "making stuff faster" but in reality, my customers will be happier with an app which takes another 0.5 seconds to launch but is stable than something that just fails more quickly.

We've had to completely disable AOT in Android production due to weird errors preventing our users booting the app at all. The underlying issue is still open.

Xamarin.iOS apps have always been AOTed and have worked perfectly. I've had a Xamarin.iOS in production since 2013, and never once had to use the interpreter.

@janne-hmp
Copy link

I proposed a new Development configuration for interpreter and other stuff speeding up e.g. XAML development here #21678 .

@Eilon Eilon added t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) and removed legacy-area-perf Startup / Runtime performance labels May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-publishing Issues with the app packaging/publishing process (ipk/apk/msix/trimming) p/1 Work that is critical for the release, but we could probably ship without platform/iOS 🍎 s/triaged Issue has been reviewed t/bug Something isn't working t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Projects
None yet
Development

No branches or pull requests