Skip to content

[Xamarin.Android.Build.Tasks] disable Switch.System.Reflection.ForceInterpretedInvoke - #7972

Merged
jonpryor merged 1 commit into
dotnet:mainfrom
jonathanpeppers:Switch.System.Reflection.ForceInterpretedInvoke
Apr 21, 2023
Merged

[Xamarin.Android.Build.Tasks] disable Switch.System.Reflection.ForceInterpretedInvoke#7972
jonpryor merged 1 commit into
dotnet:mainfrom
jonathanpeppers:Switch.System.Reflection.ForceInterpretedInvoke

Conversation

@jonathanpeppers

@jonathanpeppersjonathanpeppers commented Apr 19, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Fixes: dotnet/runtime#83893

In .NET 8, System.Reflection.ConstructorInfo/MethodInfo.Invoke() will call System.Reflection.Emit when called more than once. This impacts startup in mobile applications, so it may not be a desired feature.

Unfortunately, this appears to happen quite easily in Android apps, some examples:

The types of situations this happens:

  • You call a Java method from C# that returns a Java.Lang.Object subclass.

  • You override a Java method in C#, that has a Java.Lang.Object parameter.

To solve this problem, we can set:

<ItemGroup>
<RuntimeHostConfigurationOption Include="Switch.System.Reflection.ForceInterpretedInvoke" Value="$(_SystemReflectionForceInterpretedInvoke)" Trim="true" />
</ItemGroup>

And we can set $(_SystemReflectionForceInterpretedInvoke) to test out the setting in various apps.

I added a test to verify the "private" switch is actually set.

I also updated the .aotprofile to verify that all System.Reflection.Emit code paths disappear from dotnet new android applications.

jonathanpeppers
jonathanpeppers commented Apr 19, 2023
View reviewed changes
Comment threadsrc/profiled-aot/dotnet.aotprofile.txt
Comment on lines -2328 to -2345
void System.Reflection.Emit.DynamicMethod:.cctor ()
void System.Reflection.Emit.DynamicMethod:.ctor (string,System.Type,System.Type[],System.Reflection.Module,bool)
void System.Reflection.Emit.DynamicMethod:create_dynamic_method (System.Reflection.Emit.DynamicMethod,string,System.Reflection.MethodAttributes,System.Reflection.CallingConventions)
void System.Reflection.Emit.DynamicMethod:CreateDynMethod ()
void System.Reflection.Emit.DynamicMethod:Init (string,System.Reflection.MethodAttributes,System.Reflection.CallingConventions,System.Type,System.Type[],System.Type,System.Reflection.Module,bool,bool)
void System.Reflection.Emit.ILGenerator:.ctor (System.Reflection.Module,System.Reflection.Emit.ITokenGenerator,int)
void System.Reflection.Emit.ILGenerator:Emit (System.Reflection.Emit.OpCode,int)
void System.Reflection.Emit.ILGenerator:Emit (System.Reflection.Emit.OpCode,System.Reflection.ConstructorInfo)
void System.Reflection.Emit.ILGenerator:Emit (System.Reflection.Emit.OpCode,System.Reflection.FieldInfo)
void System.Reflection.Emit.ILGenerator:Emit (System.Reflection.Emit.OpCode,System.Reflection.MethodInfo)
void System.Reflection.Emit.ILGenerator:Emit (System.Reflection.Emit.OpCode,System.Type)
void System.Reflection.Emit.ILGenerator:Emit (System.Reflection.Emit.OpCode)
void System.Reflection.Emit.ILGenerator:emit_int (int)
void System.Reflection.Emit.ILGenerator:label_fixup (System.Reflection.MethodBase)
void System.Reflection.Emit.ILGenerator:ll_emit (System.Reflection.Emit.OpCode)
void System.Reflection.Emit.ILGenerator:make_room (int)
void System.Reflection.Emit.OpCodes:.cctor ()
void System.Reflection.Emit.RuntimeModuleBuilder:.cctor ()

@jonathanpeppersjonathanpeppersApr 19, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

@pjcollins

pjcollins commented Apr 19, 2023

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

azure-pipelinesBot commented Apr 19, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jonathanpeppers
[Xamarin.Android.Build.Tasks] disable Switch.System.Reflection.ForceI…
0b53dca
…nterpretedInvoke
Fixes: dotnet/runtime#83893
In .NET 8, `System.Reflection.ConstructorInfo/MethodInfo.Invoke()` will
call `System.Reflection.Emit` when called more than once. This impacts
startup in mobile applications, so it may not be a desired feature.
Unfortunately, this appears to happen quite easily in Android apps,
some examples:
* https://gist.github.com/ivanpovazan/2563ea9d2fea320e6425cfcc58da3ee5
* https://gist.github.com/ivanpovazan/d2546d4abad17900d4366cc29e1689b2
The types of situations this happens:
* You call a Java method from C# that returns a `Java.Lang.Object`
subclass.
* You override a Java method in C#, that has a `Java.Lang.Object`
parameter.
To solve this problem, we can set:
<ItemGroup>
<RuntimeHostConfigurationOption Include="Switch.System.Reflection.ForceInterpretedInvoke" Value="$(_SystemReflectionForceInterpretedInvoke)" Trim="true" />
</ItemGroup>
And we can set `$(_SystemReflectionForceInterpretedInvoke)` to test
out the setting in various apps.
I added a test to verify the "private" switch is actually set.
I also updated the `.aotprofile` to verify that all
`System.Reflection.Emit` code paths disappear from `dotnet new
android` applications.
@jonathanpeppers
jonathanpeppersforce-pushed the Switch.System.Reflection.ForceInterpretedInvoke branch from 5ccd09d to 0b53dcaCompareApril 19, 2023 19:47
@jonathanpeppers

jonathanpeppers commented Apr 20, 2023

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

azure-pipelinesBot commented Apr 20, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor

jonpryor commented Apr 21, 2023

Copy link
Copy Markdown
Contributor

Draft commit message:

Fixes: https://github.com/dotnet/runtime/issues/83893
Context: https://github.com/dotnet/runtime/pull/72717
In .NET 8, `System.Reflection.{ConstructorInfo,MethodInfo}.Invoke()`
will use `System.Reflection.Emit` when called more than once.
This impacts startup in mobile applications, so it may not be a
desirable feature.
Unfortunately, this appears to happen quite easily in Android apps;
some examples (using a custom dotnet/runtime build for extra output):
*https://gist.github.com/ivanpovazan/2563ea9d2fea320e6425cfcc58da3ee5*https://gist.github.com/ivanpovazan/d2546d4abad17900d4366cc29e1689b2
The primary situation in which this happens is that all Java-originated
`Java.Lang.Object` subclass constructor invocations always hit
`ConstructorInfo.Invoke()`; see `TypeManager.Activate()`.
To solve this problem, we can set:
<ItemGroup> <RuntimeHostConfigurationOption Include="Switch.System.Reflection.ForceInterpretedInvoke" Value="$(_SystemReflectionForceInterpretedInvoke)" Trim="true" /></ItemGroup>
And we can set `$(_SystemReflectionForceInterpretedInvoke)` to test
out the setting in various apps.
I added a test to verify the "private" switch is actually set.
I also updated the `.aotprofile` to verify that all
`System.Reflection.Emit` code paths disappear from
`dotnet new android` applications.

@jonpryor
jonpryor merged commit 5687506 into dotnet:mainApr 21, 2023
@jonathanpeppers
jonathanpeppers deleted the Switch.System.Reflection.ForceInterpretedInvoke branch April 21, 2023 18:41
grendello added a commit to grendello/xamarin-android that referenced this pull request Apr 24, 2023
@grendello
Merge branch 'main' into blazor-hang
4b595a8
* main:
[Xamarin.Android.Build.Tasks] enable ForceInterpretedInvoke switch (dotnet#7972)
[Mono.Android] Bind API-UpsideDownCake Beta 1 (dotnet#7980)
Bump to xamarin/xamarin-android-tools/main@8bc07503 (dotnet#7863)
[automation] Add 'xaSourcePath' to yaml so they can be used from monodroid. (dotnet#7978)
Bump to dotnet/installer@16c10f8115 8.0.100-preview.4.23218.1 (dotnet#7969)
[docs] Add UnitTest.md (dotnet#7877)
[ci] Suppress fork PR build warnings (dotnet#7973)
[Xamarin.Android.Build.Tasks] Bump ZipFlushFilesLimit (dotnet#7957)
Bump to dotnet/installer@3ca7ad1c79 8.0.100-preview.4.23211.1 (dotnet#7946)
[CI] Allow passing xamarin-android checkout dir to nested templates. (dotnet#7961)
[Xamarin.Android.Build.Tasks] Fix `-int.ToString()` for locales (dotnet#7941)
[ci] Automatically retry failed apk-instrumentation tests. (dotnet#7963)
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Apr 28, 2023
@jonathanpeppers
[android] AOT profiles for .NET 8 Preview 4
30e3dc1
Updated the profile, following the instructions in:
src/ProfiledAot/README.md
Testing a `dotnet new maui` project on a Pixel 5:
Before:
Average(ms): 568
Std Err(ms): 2.21610268514595
Std Dev(ms): 7.00793201387621
After:
Average(ms): 548.5
Std Err(ms): 2.65518360947035
Std Dev(ms): 8.39642781187333
Most notably calls to System.Reflection.Emit are now gone, after we
landed:
dotnet/android#7972
@jonathanpeppersjonathanpeppers mentioned this pull request Apr 28, 2023
Merged
mattleibow pushed a commit to dotnet/maui that referenced this pull request Apr 29, 2023
@jonathanpeppers
[android] AOT profiles for .NET 8 Preview 4 (#14838)
f24e8da
Updated the profile, following the instructions in:
src/ProfiledAot/README.md
Testing a `dotnet new maui` project on a Pixel 5:
Before:
Average(ms): 568
Std Err(ms): 2.21610268514595
Std Dev(ms): 7.00793201387621
After:
Average(ms): 548.5
Std Err(ms): 2.65518360947035
Std Dev(ms): 8.39642781187333
Most notably calls to System.Reflection.Emit are now gone, after we
landed:
dotnet/android#7972
@jonathanpeppersjonathanpeppers mentioned this pull request Aug 29, 2023
Merged
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 22, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dellis1972dellis1972Awaiting requested review from dellis1972
@jonpryorjonpryorAwaiting requested review from jonpryor

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[android] .NET 8 performance regression in System.Reflection.MethodInfo/ConstructorInfo.Invoke()

3 participants

@jonathanpeppers@pjcollins@jonpryor