maui
maui copied to clipboard
[Trimming] Add a feature flag to disable XAML loading at runtime
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:
- [x] Naming:
- MSBuild property:
XamlLoadingIsEnabled
->MauiXamlRuntimeParsingSupport
- Property in code:
FeatureFlags.IsXamlLoadingEnabled
->Microsoft.Maui.RuntimeFeature.IsXamlRuntimeParsingSupported
- MSBuild property:
- [x] Finalize warning messages (suggestions for phrasing or grammar changes are welcome, English is not my first language)
- [x] ~~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 inSetAndLoadSource
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?~~ - [x] ~~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 accessResourceLoader
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 | After | Delta | |
---|---|---|---|
Trimming warnings | 64 | 36 | 28 |
AOT warnings | 5 | 5 | 0 |
.ipa size | 7,425,853 B | 7,047,340 B | 378,513 B (5.1%) |
Oh thinking about it more, should XamlCompilationOptions.Skip
have [RequiresUnreferencedCode]
on it? So you get trimmer warnings if you use that enum value.
@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.
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:
https://github.com/dotnet/maui/blob/7acd5c2990aa0241bf896c743d81d7f716b05868/src/Controls/src/Build.Tasks/XamlCTask.cs#L87-L88
https://github.com/dotnet/maui/blob/7acd5c2990aa0241bf896c743d81d7f716b05868/src/Controls/src/Build.Tasks/XamlCTask.cs#L102-L103
https://github.com/dotnet/maui/blob/7acd5c2990aa0241bf896c743d81d7f716b05868/src/Controls/src/Build.Tasks/XamlCTask.cs#L132-L133
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?
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.
@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: https://github.com/dotnet/maui/commit/8821d48ce279fab529965dfada09cd6c0e819b6d
- 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?
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.
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.
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?
Looks like it was a manual bump.
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.
I've always had to bump manually (also any new package versions have to be manually mirorred to the dotnet-public first - https://github.com/dotnet/arcade/blob/main/Documentation/MirroringPackages.md)
I revisited the condition in ResourceDictionary
which was supposed to ensure that the developers have the same experience in Debug and Release configurations and I simplified the code:
- we now compile all XAML files by default (https://github.com/dotnet/maui/pull/19390)
- the codepath which can throw will only run of the developer opts out of XAML compilation (both in Debug and Release/NativeAOT)
- if developer opts out of XAML compilation for some file, they will get a build warning from the source generator or from XamlC (https://github.com/dotnet/maui/pull/20127)
On top of that, I updated the source generator to generate [UnconditionalSuppressMessage]
for all generated InitializeComponent()
methods - they always contain calls to LoadFromXaml
which will always produce IL2026
. XamlC later removes the attribute so that we don't suppress trimming warnings from ILC when it compiles the generated IL.
Please have another look, @StephaneDelcroix.
/rebase
/rebase
If this is green, I think we are OK to merge:
I am testing how green CI is, in general, at:
- https://github.com/dotnet/maui/pull/20387
- https://github.com/dotnet/maui/pull/20388
Agreed.
FWIW, the last tip of net9
branch https://github.com/dotnet/maui/commit/9c03adcef4fe852f14738825a9cba8df2312b8a7 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