Add design doc for trimming opt-in/opt-out

[sbomer]

This includes some background on what we added for .NET5, and a proposal for more options in .NET6 :

• An assembly-level attribute to opt into trimming
• Simplified trimming opt-in from the SDK
• Simplified "trim all assemblies" flag

This is a summary of the discussion in #1269 and https://github.com/dotnet/sdk/issues/14642, and the .NET 5 discussion in dotnet/sdk#12035.

[sbomer]

@eerhardt PTAL (I can't seem to request your review)

[marek-safar]

Nice write up!

[eerhardt]

Looks good, thanks for writing this up.

I only have one comment of real substance - whether we need to implement @(TrimmableAssembly) in .NET 6 or if we can push it to future.

[rolfbjarne]

For Xamarin.iOS it's fairly common thing to make the linker as aggressive as possible (to get the app size down), and then figure out how to unbreak everything that broke - by disabling the trimming for specific assemblies (or even give the linker xml definitions of what to preserve).

Scenario:

Customer has a class library project, and multiple app head projects (say one for Xamarin.iOS and Xamarin.TVOS) that reference the class library project. The customer tries to trim all assemblies, but realizes that the linker removes too much from his class library project, and wants to tell the linker to not trim his class library project. From this proposal, it seems that the customer has to modify the project file for each app head project (and any future projects that may end up referencing his class library project) to add the IsTrimmable metadata. Is that correct?

[marek-safar]

I think this scenario should be sufficiently covered with TrimmableAssembly, so the developer would end up with something like this in the csproj file (assuming none of the assemblies have IsTrimmable set).

<ItemGroup>
  <TrimmableAssembly Include="assembly1" />
  <TrimmableAssembly Include="assembly2" />
  <TrimmableAssembly Include="assembly3" />
<!--  <TrimmableAssembly Include="assembly4" /> cannot be linked -->
  <TrimmableAssembly Include="assembly5" />
</ItemGroup>

[rolfbjarne]

@marek-safar afaict that won't work if the developer sets <TrimAllAssemblies>true</TrimAllAssemblies> first.

[marek-safar]

TrimAllAssemblies is not planned for .NET6, it's under Possible future work section https://github.com/mono/linker/pull/1647/files#diff-9abbbcd880aff3bdefcbefe9d49f810ae6eb372bf40816da53f3e470a8c483bfR116

[eerhardt]

In the future if we had TrimAllAssemblies, we could add metadata to the TrimmableAssembly item to set Trimmable=false which would opt that assembly out of trimming.

[rolfbjarne]

@marek-safar there will be a way to enable linking for all assemblies (currently implemented with the MtouchLink MSBuild variable), so the point still stands: if linking is enabled for all assemblies, then your solution won't work.

[MichalStrehovsky]

For Xamarin.iOS it's fairly common thing to make the linker as aggressive as possible

Do we have actual telemetry for this is this more of a gut feel?

In .NET Native, we played with settings like this a lot and imagined how useful they are (offering various opt-in/opt-out granularities at assembly levels). We wrote blog posts and tools on how to tune the size.

Turned out we lived in a bubble where all of this was clear, easy to use, and useful, but the real world disagreed. Inspecting actual real world apps in the Windows Store we found out only around 3% of all apps in the Store ever touched any of the defaults.

Out of those 3%, pretty much everyone just asked to root more things because we were breaking their app. Only 0.03% of apps opted out of the defaults and asked for more aggressive trimming, even though our testing indicated 70% of apps could do that without seeing any trimming related bugs.

This was when Windows Phone was still a thing and size mattered more.

[sbomer]

From this proposal, it seems that the customer has to modify the project file for each app head project (and any future projects that may end up referencing his class library project) to add the IsTrimmable metadata. Is that correct?

@rolfbjarne Just to confirm - that's correct. If aggressive linking is already common, I can see the usefulness of adding AssemblyMetadataAttribute("IsTrimmable", "False") in .NET 5, but I have some concerns:

In the scenario you outlined, is the desire to disable trimming only locally for the projects that reference it, or also for other developers? The attribute has the downside that it could be too permanent. If the attributed library ships as a nuget package, another developer who only uses a small portion of it would end up with a larger app. I added some notes along these lines to the doc.

we could add metadata to the TrimmableAssembly item to set Trimmable=false

@eerhardt I added this suggestion as an alternative under NonTrimmableAssembly - but I don't think this would address @rolfbjarne's concern since it still needs to be set in each app's project file.

Using embedded descriptor to force root the entire assembly.

@vitek-karas with lazy loading, this will only keep the assembly if some part of it is used in a way that's understood by the linker (IsTrimmable=false metadata/attribute would keep it regardless). This may or may not be the right behavior, depending on @rolfbjarne's scenario.

[rolfbjarne]

@MichalStrehovsky

For Xamarin.iOS it's fairly common thing to make the linker as aggressive as possible

Do we have actual telemetry for this is this more of a gut feel?

I'll try to get some telemetry for this.

@sbomer

In the scenario you outlined, is the desire to disable trimming only locally for the projects that reference it, or also for other developers?

Also for other developers.

The attribute has the downside that it could be too permanent. If the attributed library ships as a nuget package, another developer who only uses a small portion of it would end up with a larger app.

From this proposal it sounded like it would be possible to override the attribute with an MSBuild property, if the consumer of the library wanted to do so.

Using embedded descriptor to force root the entire assembly.

@vitek-karas with lazy loading, this will only keep the assembly if some part of it is used in a way that's understood by the linker (IsTrimmable=false metadata/attribute would keep it regardless). This may or may not be the right behavior, depending on @rolfbjarne's scenario.

Yes, this sounds like something that could work from a technical standpoint.

But from a developer perspective it just looks unpolished if I try to use AssemblyMetadataAttribute("IsTrimmable", "False") and then get a warning that either tells me to read some documentation that tells me to add a resource file with a specific name and content to accomplish more or less the same thing as what AssemblyMetadataAttribute("IsTrimmable", "False") would do.

[sbomer]

The attribute has the downside that it could be too permanent.[...]

From this proposal it sounded like it would be possible to override the attribute with an MSBuild property

True, the MSBuild overrides make any combination possible - I'm just worried about the defaults you get from the attributes, especially as more people start to use aggressive trimming.

I added a description of your scenario to the doc under the future section @rolfbjarne. I'll go ahead and merge this since I think there's agreement about the opt-in, and if there's more data about the opt-out we can revisit.