Skip to content

AndroidUseInterpreter should work in net6 #5633

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
joj opened this issue Feb 17, 2021 · 6 comments · Fixed by #5635
Closed

AndroidUseInterpreter should work in net6 #5633

joj opened this issue Feb 17, 2021 · 6 comments · Fixed by #5635
Assignees
Labels
enhancement Proposed change to current functionality.

Comments

@joj
Copy link
Contributor

joj commented Feb 17, 2021

In .net4 AndroidUseInterpreter used to switch the runtime directory with one that includes an "-interpreter" suffix. That would automatically make the runtime use the interpreter. For net6, the runtime includes both versions in the package,
and the way to switch interpreter on is to add this environment variable:
debug.mono.runtime_args=--interp=-inline

The parameter is at the moment unchanged for net6, meaning that it adds a prefix that doesn't work anymore, and it doesn't do the environment, which is needed. The ask is to switch to that and then make sure interpreter works (that may require some help from mono runtime).

Thanks!
J

@jonathanpeppers jonathanpeppers self-assigned this Feb 17, 2021
@jonathanpeppers jonathanpeppers added this to the One .NET milestone Feb 17, 2021
@jonathanpeppers
Copy link
Member

This would be adding some new logic to:

https://github.com/xamarin/xamarin-android/blob/214dd76d9441c9974775f0b29feec0d5d8d1097f/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L1503-L1518

Basically emit some extra @(AndroidEnvironment) lines for .NET 6 when $(AndroidUseInterpreter) is true.

An additional work item would be to standardize on an MSBuild property for both iOS/Android, as $(MTouchExtraArgs) is used for enabling the interpreter at the moment.

@lambdageek lambdageek self-assigned this Feb 17, 2021
@lambdageek
Copy link
Member

lambdageek commented Feb 17, 2021

I tried AndroidUseInterpreter and it "works" in that the runtime tries to start up with interp support, but it breaks because it passes MONO_AOT_MODE_LAST which used to work with old mono, but not net6 mono.

I'm working on a PR to update to use MONO_AOT_MODE_INTERP_ONLY (on top of #5632)

Edit

The relevant runtime changes were to fix mono/mono#18893 - which was done in mono/mono#20159 - that PR went into dotnet/runtime master but not into mono/mono 2020-02. So the update to use MONO_AOT_MODE_INTERP_ONLY will be net6-only.

@lambdageek

This comment has been minimized.

lambdageek added a commit to lambdageek/xamarin-android that referenced this issue Feb 18, 2021
This depends on a fix to mono/mono#18893 (make interp only mode
distinct from MONO_AOT_MODE_LAST) which was done in
mono/mono#20159 but only for .NET 6, not previous Mono releases.

Fixes dotnet#5633
lambdageek added a commit to lambdageek/xamarin-android that referenced this issue Feb 19, 2021
This depends on a fix to mono/mono#18893 (make interp only mode
distinct from MONO_AOT_MODE_LAST) which was done in
mono/mono#20159 but only for .NET 6, not previous Mono releases.

Fixes dotnet#5633
lambdageek added a commit to lambdageek/xamarin-android that referenced this issue Feb 26, 2021
This depends on a fix to mono/mono#18893 (make interp only mode
distinct from MONO_AOT_MODE_LAST) which was done in
mono/mono#20159 but only for .NET 6, not previous Mono releases.

Fixes dotnet#5633
lambdageek added a commit to lambdageek/xamarin-android that referenced this issue Mar 2, 2021
This depends on a fix to mono/mono#18893 (make interp only mode
distinct from MONO_AOT_MODE_LAST) which was done in
mono/mono#20159 but only for .NET 6, not previous Mono releases.

Fixes dotnet#5633
lambdageek added a commit to lambdageek/xamarin-android that referenced this issue Mar 3, 2021
This depends on a fix to mono/mono#18893 (make interp only mode
distinct from MONO_AOT_MODE_LAST) which was done in
mono/mono#20159 but only for .NET 6, not previous Mono releases.

Fixes dotnet#5633
jonpryor pushed a commit that referenced this issue Mar 3, 2021
Fixes: #5633

Context: mono/mono#18893
Context: mono/mono#20159

The `$(AndroidUseInterpreter)` property (f5a9494), née the
`$(_AndroidUseInterpreter)` property (42822e0), only worked with the
Legacy runtime (mono/mono), not the .NET 6 MonoVM runtime.

Update `src/monodroid` so that `$(AndroidUseInterpreter)` is now
meaningful when running on the .NET 6 MonoVM runtime.
When `$(AndroidUseInterpreter)`=True, set the AOT mode to
`MonoAotMode::MONO_AOT_MODE_INTERP_ONLY`.  This causes MonoVM to use
it's builtin interpreter instead of the normal JIT behavior.
@jonathanpeppers
Copy link
Member

@lambdageek do we still need to do something on the MSBuild side when $(AndroidUseInterpreter) is true?

Do we need debug.mono.runtime_args=--interp=-inline anymore?

@joj
Copy link
Contributor Author

joj commented Mar 4, 2021

Can we make AndroidUseInterpreter opt out (meaning, default to true when debugging)? What do you think? That will enable these new functionalities by default, and allow users to turn it off if it's causing problems.

@lambdageek
Copy link
Member

There's two orthogonal issues:

  1. Is AndroidUseInterpreter usable with net6 now? Yes.
  2. Should something more be done to enable hot reload and other experiences in Debug configurations? Also yes, but I don't think it should be controlled by AndroidUseInterpreter or by changing defaults. We talked about having some property that indicates that hot reload is needed. @StephaneDelcroix

(On the runtime side I will be implementing support for the new DOTNET_MODIFIABLE_ASSEMBLIES environment variable setting dotnet/runtime#47274 which will obviate the need to explicitly turn off inlining with environment variables. But obviously the new env var will need to be set)

@ghost ghost locked as resolved and limited conversation to collaborators Jun 3, 2022
@jpobst jpobst added the enhancement Proposed change to current functionality. label Dec 1, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
enhancement Proposed change to current functionality.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants