[Trimming] Add a feature flag to disable XAML loading at runtime

[simonrozsival]

Description of Change

This PR adds a feature flag that allows developers to disable XAML loading at runtime. This is useful when XAML files are compiled using XamlC and the LoadFromXaml and XamlLoader code is never used anyway. The file size savings I'm seeing with NativeAOT on iOS with the feature disabled is 5.1% (378 kB).

Moreover, the XAML loading code is not trimming-friendly. The XAML document can reference arbitrary types and the static analysis cannot find them. For this reason, the feature should be disabled by default for NativeAOT apps.

Discussion points:

• Naming:
  • MSBuild property: XamlLoadingIsEnabled -> MauiXamlRuntimeParsingSupport
  • Property in code: FeatureFlags.IsXamlLoadingEnabled -> Microsoft.Maui.RuntimeFeature.IsXamlRuntimeParsingSupported
• Finalize warning messages (suggestions for phrasing or grammar changes are welcome, English is not my first language)
• How to handle ResourceDictionary.SetAndLoadSource? The current implementation assumes that XamlC will compile all resource dictionaries and add the correct [XamlResourceId] attributes so the codepath that throws exception is never hit. On the other hand, it breaks the principle that whenever there is a call to a method that can throw because the feature is disabled should produce a build time warning. After examining the code in SetAndLoadSource and the code in XamlC that generates the call I wonder if we could avoid calling the method altogether and instead instantiate the generated resource dictionary directly. Is there a specific reason why we're not doing it, or is that something that wasn't worth implementing yet?
• Does it make sense to annotate IResourcesLoader since it is an internal interface? I am a little confused why we have the interface in the first place and why we use it to access ResourceLoader through DI in the first place. Could this setup be simplified?

/cc @jonathanpeppers @StephaneDelcroix @mdh1418 @ivanpovazan @vitek-karas

Issues Fixed

Contributes to #18658 and #19397
Once this change is merged it should decrease the number of warnings tracked by the test that should be introduced by #19194

NativeAOT on iOS

	Before (net9.0 branch)	After	Delta
Trimming warnings	54	27	27
AOT warnings	4	4	0
.ipa size	5,643,195 B	5,228,900 B	414,295 B (7.3%)

[jonathanpeppers]

Oh thinking about it more, should XamlCompilationOptions.Skip have [RequiresUnreferencedCode] on it? So you get trimmer warnings if you use that enum value.

[vitek-karas]

Will I get this warning in an app which doesn't call any of the affected public APIs, but if I just enable the feature switch?
If so, then this should mention the feature switch and how to disable it (as that's the way to "fix" this).

Or maybe a better way to put it - I create an app, and enable this feature. Then I AOT the app. Is there any case where the behavior of the app would change after AOT in this case? Especially is there a way to hit one of the new exceptions? If the answer is yes, then we should make it to produce a warning. We do this for example for startup hooks - if you enable startup hooks, you will get trim warning. It may be that at runtime you don't run startup hooks and so you never hit the problem - but we "play it safe" and produce a warning regardless.
I would expect similar behavior here...

[vitek-karas]

It's also a good idea to add the Url property to the RUC so that we have a way to direct users to a longer and much more detailed description of the problem and how to solve it. In this case one such solution would be to discuss XamlC for example.

[simonrozsival]

Will I get this warning in an app which doesn't call any of the affected public APIs, but if I just enable the feature switch?

No, this warning will only show up in apps that call .LoadFromXaml(...) explicitly.

It's also a good idea to add the Url property to the RUC so that we have a way to direct users to a longer and much more detailed description of the problem and how to solve it.

I think we'll need to revisit this later. We currently don't have any documentation or guidelines that we could point to.

[vitek-karas]

This looks like the place which should produce a warning if the feature is enabled. Note that RUC warnings (IL2026) have a special behavior and they're not part of the "singlewarn" behavior - so the user should see the actual message, and not just "MAUI has trimming problems". This behavior was specifically designed for cases like this.

[simonrozsival]

@jonathanpeppers the RUC attribute cannot be applied to enum values (it's just for classes, methods, and constructors). Could we produce these warnings whenever XamlC runs into one of these attributes? One similar scenario: We should probably emit warning when MauiEnableXamlLoading is disabled and MauiXamlCValidateOnly is enabled.

[jonathanpeppers]

Ok, yeah it kind of makes sense you can't put ILLink attributes on an enum. It's only because we have a custom MSBuild task that rewrites IL -- that makes this even possible.

So, I think we could manually warn in a couple places:

maui/src/Controls/src/Build.Tasks/XamlCTask.cs

Lines 87 to 88 in 7acd5c2

	if ((options & XamlCompilationOptions.Skip) == XamlCompilationOptions.Skip)
	skipassembly = true;

maui/src/Controls/src/Build.Tasks/XamlCTask.cs

Lines 102 to 103 in 7acd5c2

	if ((options & XamlCompilationOptions.Skip) == XamlCompilationOptions.Skip)
	skipmodule = true;

maui/src/Controls/src/Build.Tasks/XamlCTask.cs

Lines 132 to 133 in 7acd5c2

	if ((options & XamlCompilationOptions.Skip) == XamlCompilationOptions.Skip)
	skiptype = true;

I think we could pass in the MSBuild property to this task to know if the new feature flag is enabled. It might should even be an error?

[vitek-karas]

Ok, yeah it kind of makes sense you can't put ILLink attributes on an enum.

This has been discussed several times in the past, but it's difficult. In IL enums look like ints (ldc.0 and so on). The only way to recognize enums is by looking at the type of a parameter they're passed into. So doable, but not super robust... so for now we avoided investing into it.

[jonathanpeppers]

LGTM, I reran some lanes and things look green now.

[mattleibow]

We have a lot of xaml-related things in here, so maybe this should be defined here too?

maui/src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets

Line 136 in c1dfd70

<!-- XamlC -->

[simonrozsival]

I'm not sure if that is the right place to put the feature flag defaults. This feature flag is related to XAML but I think there will be more feature switches that aren't related to it.

But I can definitely imagine moving this to a separate .targets file with a target that sets up the feature switches before the RunILLink target runs.

[jonathanpeppers]

@simonrozsival do you know offhand if any of the trimmer warnings so far are from Microsoft.Maui.dll / Core.csproj, or are they all from Microsoft.Maui.Controls.dll & Microsoft.Maui.Controls.Xaml.dll?

It might make sense to put the trimmer switches in "Controls", as it is possible to create an app that uses parts of MAUI and not Microsoft.Maui.Controls.dll.

[simonrozsival]

The warnings we're getting are mostly from Microsoft.Maui.Controls.dll, but for example the DI related ones are in Microsoft.Maui.dll (for example the MauiFactory related code). Yesterday we talked about putting the existing reflection-based image source service resolution code behind a feature flag. Wouldn't we need a new separate setup with a separate ILLink.Subsitutions.xml to introduce that feature flag if we moved this feature flag from Core into Controls?

[jonathanpeppers]

I think another reason to move from src/Workload/Microsoft.Maui.Sdk/Sdk/Microsoft.Maui.Sdk.Before.targets, is this file is in the workload, and wouldn't be able to be updated independently with $(MauiVersion) or the PackageReference in the project template.

@mattleibow is there a .targets file that would also include a way for feature flags to exist in Core/Microsoft.Maui.dll?

[simonrozsival]

I see. I can move the code to Microsoft.Maui.Controls.targets. Even with the code in that targets file, we can still define IL substitutions for the RuntimeFeature class which will stay in Core.

[jonathanpeppers]

I think that's the way to go for now, I was looking in:

"C:\Program Files\dotnet\library-packs\Microsoft.Maui.Controls.Core.8.0.3.nupkg"

And there are no .targets files in there currently.

[ivanpovazan]

@simonrozsival as you noticed locally, template tests are also failing on CI with:

  Failed PublishNativeAOTCheckWarnings("maui","net9.0-ios","ios-arm64") [1 m 7 s]
  Error Message:
   System.ArgumentNullException : Value cannot be null. (Parameter 'format')
  Stack Trace:
     at System.ArgumentNullException.Throw(String paramName)
   at System.String.FormatHelper(IFormatProvider provider, String format, ReadOnlySpan`1 args)

this is due to a new version of MSBuild.StructuredLogger API, and as discussed offline seems to be fixed by bumping the version of the package.

This was already done on main with: 8821d48

• As a short-term solution I suppose you can just cherry-pick that commit
• As a long-term solution (to avoid versioning problems) @jonathanpeppers should we wire integration tests references with darc, so we are always up-to-date?

[jonathanpeppers]

should we wire integration tests references with darc, so we are always up-to-date?

MSBuild.StructuredLogger is third party (although Kirill works for MSFT). I would guess that main got updated because @dependabot updated it?

The general problem here happens if MSBuild changes the .binlog format. This will probably happen a couple times during .NET 9 previews, it happened during 6, 7, 8.

[simonrozsival]

There are still some failing tests, but they don't seem unique to this PR, and I can see the same failures in other CI runs.

[mattleibow]

/rebase

[jonathanpeppers]

If this is green, I think we are OK to merge:

I am testing how green CI is, in general, at:

• dotnet format src/Core/src/Core.csproj #20387
• [net9.0] dotnet format src/Core/src/Core.csproj #20388

[ivanpovazan]

Agreed.

FWIW, the last tip of net9 branch 9c03adc got merged with macOS and iOS Controls (v16.4) failures, due to missing workloads:

Executing: /Users/builder/azdo/_work/3/s/bin/dotnet/dotnet build "../../src/Controls/samples/Controls.Sample.UITests/Controls.Sample.UITests.csproj" --framework net9.0-ios --configuration Release /p:BuildIpa=true /bl:/Users/builder/azdo/_work/3/a/logs/Controls.Sample.UITests-Release-ios.binlog /tl
MSBuild version 17.10.0-preview-24101-01+07fd5d51f for .NET
  Determining projects to restore...
/Users/builder/azdo/_work/3/s/bin/dotnet/sdk/9.0.100-preview.1.24101.2/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.ImportWorkloads.targets(38,5): error NETSDK1178: The project depends on the following workload packs that do not exist in any of the workloads available in this installation: Microsoft.NETCore.App.Runtime.AOT.Cross.net8.ios-arm64 Microsoft.NETCore.App.Runtime.AOT.Cross.net8.iossimulator-arm64 Microsoft.NETCore.App.Runtime.AOT.Cross.net8.iossimulator-x64 [/Users/builder/azdo/_work/3/s/src/Graphics/src/Graphics/Graphics.csproj::TargetFramework=net8.0-ios]

in https://dev.azure.com/xamarin/public/_build/results?buildId=107057&view=logs&j=bdef7eb6-2ec3-5ca2-56c7-25b2b8e714f2&t=f852c752-4869-5efa-8563-adc59bcd5d54&l=1011