[nativeaot] fix Exists() checks

[jonathanpeppers]

Context: dotnet/android#9846

Trying to build our Mono.Android-Tests suite for NativeAOT, it fails with:

EXEC Failed to load assembly 'System.IO'

Which is caused by passing:

--root:System.IO

I couldn't find anything in the Android workload that would cause this.

It appears the Microsoft.NETCore.Native.targets have an issue caused by this specific project:

• When adding to @(_IlcRootedAssemblies) it does an Exists() check

• So Exists('System.IO.Compression') is called. This is true because there is a folder in the working directory named System.IO.Compression!

https://github.com/dotnet/android/tree/main/tests/Mono.Android-Tests/Mono.Android-Tests/System.IO.Compression

• %(FileName) is then System.IO as .Compression is stripped as the %(Extension) metadata.

I believe the fix here is to use System.IO.File.Exists() instead of MSBuild's Exists() as it returns true for files and folders.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[am11]

Does it apply to other usages of Exists in build integration as well?

% git grep Exists src/coreclr/nativeaot/BuildIntegration 
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets:    <Warning Condition="Exists($(UserRuntimeConfig))" Text="The published project has a runtimeconfig.template.json that is not supported by PublishAot. Move the configuration to the project file using RuntimeHostConfigurationOption." />
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets:    <Delete Files="$(_symbolTargetPath)" Condition="'$(_symbolIsFile)' == 'true' and Exists('$(_symbolTargetPath)')" />
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets:      Condition="'$(_symbolIsFile)' == 'true' and Exists('$(_symbolSourcePath)')" />
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets:    <RemoveDir Directories="$(_symbolTargetPath)" Condition="'$(_symbolIsFile)' == 'false' and Exists('$(_symbolTargetPath)')" />
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets:      Condition="'$(_symbolIsFile)' == 'false' and Exists('$(_symbolSourcePath)')" />
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets:      <UseSystemZlib Condition="'$(UseSystemZlib)' == '' and !Exists('$(IlcFrameworkNativePath)libz.a')">true</UseSystemZlib>
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets:      <UseSystemBrotli Condition="'$(UseSystemBrotli)' == '' and !Exists('$(IlcFrameworkNativePath)libbrotlicommon.a')">true</UseSystemBrotli>
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets:    <Error Condition="!Exists('$(SysRoot)') and '$(_IsiOSLikePlatform)' == 'true'"
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets:    <ItemGroup Condition="'$(StaticOpenSslLinking)' != 'true' and Exists('$(IlcSdkPath)nonportable.txt')">
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets:    <ItemGroup Condition="!Exists('$(IlcSdkPath)debugucrt.txt')">

[jonathanpeppers]

I think it applies to the section below these comments:

<!-- Now process the raw ItemGroup into the form that ILC accepts: assembly names only -->
<!-- Use the logic that ILLinker uses: if the file exists, this is a file name. Otherwise it's an assembly name -->

[MichalStrehovsky]

Thanks! I still really wish we didn't have to do this, but this is how it was defined for ILLink, for better or worse.

[MichalStrehovsky]

Rooting assemblies has really bad impact on size/compile time/etc with native AOT, much worse than for ILLinker (we can optimize things quite well if they're not reflection visible; making them reflection visible disables optimizations and generates a lot of extra data and code). Having to root system assemblies is not very ideal.

[jonathanpeppers]

Rooting assemblies has really bad impact on size/compile time/etc with native AOT, much worse than for ILLinker (we can optimize things quite well if they're not reflection visible; making them reflection visible disables optimizations and generates a lot of extra data and code). Having to root system assemblies is not very ideal.

This is definitely just a step to get it working, but is the same as what we have on iOS right now:

• Run ILLink w/ our custom trimmer steps
• Pass ILLink's output to ILC, root everything

If you have ideas to optimize this better, the two approaches are at:

• https://github.com/dotnet/macios/blob/91e1d8e6f50b6da01f14500108683bc4ee6e5d34/dotnet/targets/Xamarin.Shared.Sdk.targets#L1335-L1341
• https://github.com/dotnet/android/blob/c1e0d1084301a6a64fa6eb96b6fcc698e2178262/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets#L88-L96

Android is still experimental in .NET 10; iOS they support NativeAOT in .NET 9+.

Ideally one day, we'd remove all the custom trimmer steps and just run ILC by itself.

[MichalStrehovsky]

This is definitely just a step to get it working, but is the same as what we have on iOS right now:

Yep, iOS runs ILLinker too, but iOS doesn't root the resulting assemblies for native AOT. It may look like the rooting shouldn't matter because we already trimmed, but native AOT does the trimming on top of native artifacts and that allows trimming a lot more.

To give an idea of what that means:

using System.Reflection;

class Program
{
    public static void Main()
    {
        // Call something
        GrabString();

        // Generates trimming warnings, but we want to observe how trimming happened
        foreach (var m in Type.GetType(string.Concat("Pro", "gram")).GetMethods(BindingFlags.Public | BindingFlags.Static | BindingFlags.DeclaredOnly))
            Console.WriteLine(m);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static string GrabString() => "Hello";

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static string UnusedMethod() => "Hola";
}

This will print without trimming:

Void Main()
System.String GrabString()
System.String UnusedMethod()

With PublishTrimmed it will print:

Void Main()
System.String GrabString()

That matches expectations.

But with native AOT it will print:

Void Main()

This is because native AOT optimized away the reflection metadata for GrabString. The method body is still there, but one cannot get to it with reflection (if you wonder why Main is there, it's because we special case it so that Assembly.EntryPoint works). Native AOT does many optimizations like this and they add up.

You can run dotnet publish -p:PublishAot=true with an undocumented -p:IlcTrimMetadata=false. If you do that, native AOT will simulate IL-level trimming. The output of the above program is going to be:

Void Main()
System.String GrabString()

Same as ILLink - if a method was called, we generate a method body and the body can be reflection accessed. If it wasn't called, it's completely removed.

IlcTrimMetadata=false will let you approximate the cost of "run ILLink + root everything that survived IL trimming" - it's pretty much the same thing. As an example, dotnet new webapiaot by default produces a 9,195,008 byte executable. With IlcTrimMetadata=false, it balloons to 16,128,000 bytes. The reason are all the missed optimization (e.g. if a simple property getter could get fully inlined everywhere, the method would disappear - but now we're forcing an unused standalone method body and all the reflection metadata about it such as name, parameter names, and parameter types)

The impact of reflection-rooting things that should not be roots is so massive that any collected perf numbers are meaningless (such size difference is going to affect startup, working set, everything, not just size).

[jonathanpeppers]

Refreshing my memory, the line such as:

<TrimmerRootAssembly Include="@(_AndroidILLinkAssemblies->'%(Filename)')" Exclude="System.Private.CoreLib" TrimMode="All" />

Was mostly me trying to get Android to work at all for the first time.

Let me see if we can remove it, for sure.

The impact of reflection-rooting things that should not be roots is so massive that any collected perf numbers are meaningless (such size difference is going to affect startup, working set, everything, not just size).

For what it's worth, the startup time we've seen is already better than Mono+Profiled AOT. So, if we're not even doing it correctly yet, things might improve even further.

[jonathanpeppers]

I found some apps can launch with only rooting a single assembly:

<TrimmerRootAssembly Include="Mono.Android" TrimMode="All" />

But then a dotnet new maui template wouldn't launch; I imagine it needs a couple more.

The problem is we need to "root" every Java.Lang.Object subclass. You can define these in AndroidManifest.xml and other Android XML files, so the trimmer won't necessarily know about them. We have a custom trimmer step in ILLink that makes it work.

It will take longer to solve this correctly, but I can imagine some combination of:

• We fix trimmer warnings in Android, so [DynamicallyAccessedMembers] is properly passed around. This will probably require some rethinking, as we have string values passed in from Java.
• An MSBuild task looks at AndroidManifest.xml, and generates an XML file to pass to ILC preserve things.

[agocke]

The problem is we need to "root" every Java.Lang.Object subclass.

We should talk about the precise dependencies here. I believe the new type map proposal is aimed at avoiding this blanket preservation.

[MichalStrehovsky]

It might be possible to do something short-term that would allow dropping the TrimmerRootAssembly - iOS had similar problems and also rooted things for ILC after ILLinker executed, but they were able to drop it. Worst case a custom step could generate DynamicDependencyAttributes that native AOT will respect. (@ivanpovazan might have a better memory than me on what was done for iOS).

I agree with Andy that medium-term we'd want the new type map proposal to solve this.