Strip the ILLinkTrim.xml file from System.Private.Xml

[eerhardt]

The two methods being preserved in System.Private.Xml (CreateSqlReader and GenerateSerializer) are large, and are not necessary when linking a mobile app. These methods only need to be preserved when linking the shared framework. When a normal app is linked, the linker is able to figure out if the two methods are necessary.

Contributes to #35199

Fixes #30912

[ghost]

Tagging subscribers to this area: @buyaa-n, @krwq
Notify danmosemsft if you want to be subscribed.

[stephentoub]

When a normal app is linked, the linker is able to figure out if the two methods are necessary.

Is #30912 addressed then as well? The linker recognizes the pattern used in

runtime/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SqlXml.cs

Line 134 in e1ffadd

s_createSqlReaderMethodInfo = typeof(System.Xml.XmlReader).GetMethod("CreateSqlReader", BindingFlags.Static | BindingFlags.NonPublic);

? Should we add PreserveDependency anyway, to make it explicit?

[eerhardt]

Is #30912 addressed then as well? The linker recognizes the pattern used in

runtime/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SqlXml.cs

Line 134 in e1ffadd

s_createSqlReaderMethodInfo = typeof(System.Xml.XmlReader).GetMethod("CreateSqlReader", BindingFlags.Static | BindingFlags.NonPublic);

?

Yes, I tested it on a standalone application:

        static void Main(string[] args)
        {
            SqlXml x = new SqlXml();
            XmlReader r = x.CreateReader();
            Console.WriteLine(r);
        }

And told the linker --ignore-descriptors true (which ignores the ILLinkTrim.xml file during the app build), and it still preserved the internal XmlReader.CreateSqlReader method. I then removed the x.CreateReader() call, and the internal XmlReader.CreateSqlReader method was removed.

Should we add PreserveDependency anyway, to make it explicit?

I think that is a good idea. I like being explicit so we don't have any surprises in case the linker changes behavior or has a bug.

A problem to be solved here is how we test this so we can catch regressions. We don't really have the capability of linking test applications today. Maybe once Xamarin apps are fully plumbed into our builds, we could make a Xamarin app that does similar to the above, and we run it to ensure it behaves correctly?

[eerhardt]

Just a note for the size difference this change makes on the WorldCities Xamarin app:

Here's the linked app without the change:

And here's the linked app with this change:

NOTE: the linker-dependencies.xml.gz file is not part of the app. It contains the dependencies between all the code, and it helps analyze why a method/type is kept in the trimmed app.

[marek-safar]

Perhaps instead of hard to notice flag in csproj we could call the XML file differently for this mode and use that to control the logic. We also need to be careful not to ever build this library twice.

[eerhardt]

Perhaps instead of hard to notice flag in csproj we could call the XML file differently for this mode and use that to control the logic.

This is a good idea. I've implemented it. I went back and forth on some different names:

• ILLinkTrim_NonEmbedded.xml
• ILLinkTrim_Stripped.xml
• ILLinkTrim_SharedFrameworkOnly.xml

but settled on ILLinkTrim_LibraryBuild.xml. Let me know what you think. I can definitely change the file name if a better name is chosen.

[marek-safar]

Let me know what you think. I can definitely change the file name if a better name is chosen.

This looks good! About the name, I'd personally go with ILLink.Descriptors.Build.xml but I'm fine with what you like.