Trimming Clean up

[eerhardt]

Description of Change

In order trim unnecessary IL from a Maui application, mark the following assemblies as "IsTrimmable=true", which will allow the trimmer to remove unused code in those assemblies:

• Microsoft.Maui
• Microsoft.Maui.Controls
• Microsoft.Maui.Controls.Xaml

With this change, the size of the sum of all the managed .dll files in a dotnet new maui Android arm64 app is:

main: 10.0 MB
This PR: 8.77MB

Clean up some trimming in Maui:

1. Use IsTrimmable MSBuild property on projects that are trimmable.
   • This will make them trimmable on all platforms
   • This enables the trim analyzer
2. Make BindableProperty trim-compatible by preserving public properties and methods on the DeclaringType
3. Fix trim warning when creating a TypeConverter and calling ConvertFromInvariantString.

Issues Fixed

Contributes to #1962.

This is following option (1) of #4997. I'm going to leave that issue open for now, in case we want to take a different option in the future in order to trim even more IL.

[jonathanpeppers]

The main thing I'm wondering is...

1. How do we know this doesn't break anything?
2. How do we prevent new changes from introducing trimming problems?

[eerhardt]

How do we know this doesn't break anything?

(short answer): We don't.
(long answer):
I've been analyzing the trim warnings in a Maui application, and categorizing ones that are real issues and need to be fixed. The BindableProperty one that I'm fixing along with this change is one that stood out as a "must fix" in order to enable these libraries for trimming. I've also been logging issues for harder ones that need more analysis. Note those other ones could also prevent any control vendors or other "framework"-like libraries from being trimmed as well. I believe that is a scenario we should support as well.

I've also been testing Release-built applications with this change. I haven't found an issue manually yet, but will fix any I find. If you have applications I should try, let me know.

In the end, there is a good possibility that this could break things. But getting it out there in people's hands to try it is the best way of finding these issues. We can fix any issues that come up - even if the fix is simply to unconditionally preserving certain code.

How do we prevent new changes from introducing trimming problems?

This one is easier to explain. We can enable trimming analysis during our build and baseline the existing warnings. Then when someone adds new warnings, they get errors. See #5041 for tracking this work.

[jonathanpeppers]

Here is a project that breaks using LoadFromXaml() APIs: foo.zip

We can decide how important that is? Maybe remove this API?

03-04 08:48:39.028  5328  5328 E AndroidRuntime: android.runtime.JavaProxyThrowable: Microsoft.Maui.Controls.Xaml.XamlParseException: Position 9:15. MarkupExtension not found for OnPlatform
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.ExpandMarkupsVisitor.MarkupExpansionParser.Parse(String , String& , IServiceProvider )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.ExpandMarkupsVisitor.ParseExpression(String& , IXmlNamespaceResolver , IXmlLineInfo , INode , INode )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.ExpandMarkupsVisitor.Visit(MarkupNode , INode )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.MarkupNode.Accept(IXamlNodeVisitor , INode )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.ElementNode.Accept(IXamlNodeVisitor , INode )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.ElementNode.Accept(IXamlNodeVisitor , INode )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.RootNode.Accept(IXamlNodeVisitor , INode )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.XamlLoader.Visit(RootNode , HydrationContext , Boolean )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.XamlLoader.Load(Object , String , Assembly , Boolean )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.XamlLoader.Load(Object , String , Boolean )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.XamlLoader.Load(Object , String )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at Microsoft.Maui.Controls.Xaml.Extensions.LoadFromXaml[MainPage](MainPage , String )
03-04 08:48:39.028  5328  5328 E AndroidRuntime:    at foo.MainPage..ctor()

I'll try a few other ideas that might break shortly.

[jonathanpeppers]

This example works fine:

public double MyStrokeThickness => 0.5;

public MainPage()
{
	InitializeComponent();

	var border = new Border
	{
		BindingContext = this,
	};
	var binding = new Binding(nameof(MyStrokeThickness));
	border.SetBinding(Border.StrokeThicknessProperty, binding);
}

It seems like all the properties are preserved on Border.

In Debug mode, I can hover and see the binding work -- I don't see any binding errors in Release mode.

[eerhardt]

It seems like all the properties are preserved on Border.

Yeah, basically anything with a BindableProperty on it will have all its PublicProperties and PublicMethods preserved on it.

[eerhardt]

Let's hold off on merging this PR until we can decide what to do with LoadFromXaml. I didn't realize this was an API that developers can call themselves. I thought all the XAML was converted to IL by the XamlC task during a Release build.

The issue is that Xaml is a form of code that the trimmer can't see. It can reference Types, and since the trimmer can't see which Types it uses, the trimmer doesn't know it needs to keep those Types.

I can think of the following options to resolve this:

1. Don't mark Microsoft.Maui.Controls as "Trimmable".

   • We can still take some of the other changes here to resolve trimmer warnings, and possibly trim Microsoft.Maui.dll. This would be at least a step in the right direction.

2. Take this change as is. And if someone is using LoadFromXaml in their app, instruct them to add <TrimmerRootAssembly Include="Microsoft.Maui.Controls" /> to their .csproj. This will instruct the trimmer to preserve the entire assembly. That way people who don't use LoadFromXaml can save the size off their apps. And people who do use it can make it work.

3. We could add a [DynamicDependency] on the LoadFromXaml API. This way, if this method is used in the app, the Types can be preserved. However, there are the following drawbacks with this approach:

   1. We would really want to say that all types in the Microsoft.Maui.Controls assembly be preserved. That way we wouldn't have to list each Type that could be used in Xaml. However, the trimmer doesn't have this feature (yet). We would have to ask for it to be implemented to say [DynamicDependency(DynammicallyAccessedMemberTypes.All, "*", "Microsoft.Maui.Controls")].
   2. This would only address the issue for our built-in controls. It wouldn't solve the problem for 3rd party extenders who wanted to allow their control assemblies to be trimmed.

4. We could add a Feature Switch to the LoadFromXaml behavior. Let's call it Microsoft.Maui.LoadFromXaml.IsSupported. When LoadFromXaml.IsSupported=false, calling LoadFromXaml will throw an exception instructing the developer to set LoadFromXaml.IsSupported=true. By default in a trimmed, Release built version of an app, LoadFromXaml.IsSupported will be false. For all other times, it will be true. The benefit that this approach has is when LoadFromXaml.IsSupported=true we can use an ILLink.Descriptor.xml file that will tell the trimmer "when LoadFromXaml.IsSupported=true, then preserve this whole assembly".

   1. 3rd party extenders could also follow this approach. That way their control assemblies can be marked as "Trimmable", but when LoadFromXaml is enabled, their whole assembly can be preserved as well.

cc @vitek-karas as an FYI.

[StephaneDelcroix]

Controls.Core, Controls.Xaml, Maui.Core and user-defined code won't be fully-trimmable until we enforce XamlC everywhere, compile all the bindings, and most of all, disallow LoadFromXaml (and the capability of having White Label apps). I'm ready to discuss this, but maybe as part of net7 ??

[jonathanpeppers]

I also think in addition to the LoadFromXaml() example #5028 (comment), I think you could decorate any page with [XamlCompilation(XamlCompilationOptions.Skip)] to get the same behavior.

So a thought I had about this:

1. We make a new $(MauiPublishTrimmed) property that you have to opt into in .NET 6. We document it as a way to get your app smaller.
2. When enabled, a feature flag makes any LoadFromXaml() calls fail, and just tells you to turn off $(MauiPublishTrimmed).

In some future .NET 7 release $(MauiPublishTrimmed) can become the default? (or disappear)

[eerhardt]

Maui.Core

For my education, What types in Maui.Core are created by Xaml? I was under the assumption that only stuff in Maui.Controls was created directly by Xaml.

[StephaneDelcroix]

For my education, What types in Maui.Core are created by Xaml? I was under the assumption that only stuff in Maui.Controls was created directly by Xaml.

<CornerRadius /> would be valid. same goes for maui.graphics

[eerhardt]

I've updated this PR to just push the changes that can be made safely today. I am not enabling more assemblies to be IsTrimmable=true. But I'm fixing a few trim-warnings and enabling the trim analyzer for projects that are marked IsTrimmable=true.

Please take another look.

[jonathanpeppers]

The error on CI is odd:

CSC : error AD0001: Analyzer 'ILLink.RoslynAnalyzer.COMAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'.

I don't really see what would cause this?

It looks like we could probably document or blog about how to opt into linking everything:

https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming/trimming-options#trimmed-assemblies

<Target Name="ConfigureTrimming"
        BeforeTargets="PrepareForILLink">
  <ItemGroup>
    <ManagedAssemblyToLink Condition="'%(Filename)' == 'Microsoft.Maui' or '%(Filename)' == 'Microsoft.Maui.Controls' or '%(Filename)' == 'Microsoft.Maui.Controls.Xaml'">
      <IsTrimmable>true</IsTrimmable>
    </ManagedAssemblyToLink>
  </ItemGroup>
</Target>

[vitek-karas]

The CI failure is probably dotnet/linker#2622 - that issue is fixed in 6.0.3xx SDKs. I could not find the offending pattern in the source code though - so maybe it's something different. Hard to tell though.
The reason it shows up is that the COMAnalyzer is turned on due to IsTrimmable=true on the Essentials project.

[jonathanpeppers]

@eerhardt ok, I see so the $(Trimmable) MSBuild property adds more analyzers, and one hits that issue.

Maybe we can go back to [assembly: AssemblyMetadata("IsTrimmable", "True")] until we get a build of the .NET SDK where that is fixed?

[eerhardt]

I was able to repro this build error locally by using msbuild instead of dotnet build (which is why I didn't see it in the first place).

I could not find the offending pattern in the source code though - so maybe it's something different. Hard to tell though.

According to the binlog, the error is here:

maui/src/Essentials/src/DeviceDisplay/DeviceDisplay.uwp.cs

Lines 101 to 102 in 97d4553

	DEVMODE vDevMode = new DEVMODE();
	EnumDisplaySettings(mi.Value.DeviceNameToLPTStr(), -1, ref vDevMode);

maui/src/Essentials/src/DeviceDisplay/DeviceDisplay.uwp.cs

Lines 240 to 245 in 97d4553

	[DllImport("User32.dll")]
	[return: MarshalAs(UnmanagedType.Bool)]
	static extern Boolean EnumDisplaySettings(
	byte[] lpszDeviceName,
	[param: MarshalAs(UnmanagedType.U4)] int iModeNum,
	[In, Out] ref DEVMODE lpDevMode);

So it is in the same method as dotnet/linker#2622. But it isn't exactly a pointer. Both CI and locally are using SDK 6.0.300-preview.22154.4. The SHA of the ILLink Analyzer is https://github.com/dotnet/linker/commits/e9cfb5413a6a7a7b5bfc3b9a73671be2b18642cf, which appears to have @tlakollo's fix. So this appears to be a new bug to me. I've opened dotnet/linker#2686 for this.

Maybe we can go back to [assembly: AssemblyMetadata("IsTrimmable", "True")] until we get a build of the .NET SDK where that is fixed?

I'm just going to disable this analyzer for now instead. That way we can still get the other trim analyzers and the other benefits of the IsTrimmable MSBuild property.

[vitek-karas]

@jtschuster will likely look into the fix in the analyzer (he also fixed the previous NullRef there).

[jonathanpeppers]

We should be able to merge this now:

1. This mainly just adds new illink attributes now.
2. No new assemblies are trimmable
3. We use the $(IsTrimmable) MSBuild property which opts into analyzers that the C# assembly attribute didn't