Skip to content

Remove remaining ILLinkTrim.xml from libraries #35199

Closed

Description

@marek-safar

They still keep unconditionally a lot of BCL code. On simple Xamarin app removing all remaining ILLinkTrim.xml files (except SPC) saves about 60% of the BCL size.

  • ./Microsoft.CSharp/src/ILLinkTrim.xml
  • ./System.Diagnostics.StackTrace/src/ILLinkTrim.xml
  • ./System.Security.Claims/src/ILLinkTrim.xml
  • ./System.ComponentModel.TypeConverter/src/ILLinkTrim.xml
  • ./System.Security.Principal.Windows/src/ILLinkTrim.xml
  • ./System.Private.DataContractSerialization/src/ILLinkTrim.xml
  • ./System.Linq/src/ILLinkTrim.xml
  • ./System.Runtime.InteropServices/src/ILLinkTrim.xml
  • ./System.Private.Xml.Linq/src/ILLinkTrim.xml
  • ./System.Private.Xml/src/ILLinkTrim.xml

It would be also nice to block adding them accidentally back again by not allowing ILLinkTrimXml property for them.

/cc @eerhardt @vitek-karas

Activity

  1. added this to the 5.0 milestone on Apr 20, 2020
  2. vitek-karas commented on Apr 20, 2020

    @vitek-karas
    Member

    To explain where the problem is:
    To add some explanation as why this is important. The ILLinkTrim.xml preserves the listed types/methods no matter what. When we run an application through the linker, these types/methods will be kept regardless of what the app actually uses. But in lot of cases these are there only because they are used from another part of the framework. If the app doesn't need the caller, it doesn't need the specific type/method, but linker can't tell and will always keep that. For example any app will contain System.Xml.XmlReader.CreateSqlReader (and everything it depends on) just because we may need it from System.Data.Common. So apps which doesn't even have System.Data.Common.dll (has been trimmed by the linker), still retain the CreateSqlReader method.

    Went through all of these (except CoreLib) and they all fall into just a couple of categories

    Called via reflection from another assembly

    This is solvable by adding PreserveDependency attribute on the callsite. But it requires #31712 to be done. Without it the code would be trimmed when linker runs on just the one assembly in the framework at a time.
    Note that linker will for the most part actually figure out the reflection call itself without PreserveDependency attributes, but currently there are issues which prevent this from working correctly (dotnet/linker#943, dotnet/linker#1079). Until those are resolved we require the PreserveDependency attribute to be used.

    Debugger support

    Some of the types are kept because they act as debugger visualizers or other supportive code to improve debugging experience. Ideally there would be a way to disable such support for apps which are not meant to be debugged with interactive debugger (for example apps which are delivered to stores).

    Linker issue for this: dotnet/linker#1093
    Related StackFrame issue: #34910

    COM support

    These are actually just a variant of "Called via reflection from another assembly", but they are specifically only for COM usage. Lot of scenarios in .NET don't have COM support and so ideally such code would be removed.

    Public but not really APIs

    Some APIs are internal, but are used by tools outside of the framework. Good example is sgen and APIs in System.Private.Xml. For these there's no "caller" in the framework, but we still need to retain them in the shipped framework assemblies even though there's no direct reference to them from anywhere. In that sense these behave exactly like public APIs, but we don't want anybody to call them directly.

  3. stephentoub commented on Apr 20, 2020

    @stephentoub
    Member

    There are also existing issues for some of these, e.g.
    #1413
    #30912

  4. MichalStrehovsky commented on Apr 20, 2020

    @MichalStrehovsky
    Member

    Ideally there would be a way to disable such support for apps which are not meant to be debugged with interactive debugger (for example apps which are delivered to stores).

    Note that debuggers might still use the attributes when debugging crash dumps developers can download from app stores - e.g. this is how that works in .NET Native for UWP apps. But it's true that that this is a possible size optimization on runtimes where crash dump debugging is non-existent anyway.

  5. danmoseley commented on Apr 20, 2020

    @danmoseley
    Contributor

    Categorizing explicitly --

    • Reflection from another assembly
        <!-- Used by TypeDescriptionProvider attributes in System.Private.Xml.Linq: -->
        <type fullname="MS.Internal.Xml.Linq.ComponentModel.*" />
        <!-- Called through reflection by System.ComponentModel.DefaultValueAttribute in CoreLib: -->
        <type fullname="System.ComponentModel.TypeDescriptor">
          <method name="ConvertFromInvariantString" />
        </type>
    
        <type fullname="System.Xml.XmlReader">
          <!-- called via reflection from System.Data.Common.dll -->
          <method name="CreateSqlReader" />
        </type>
    
        <type fullname="System.Security.Principal.GenericPrincipal">
          <!-- called through reflection by AppDomain -->
          <method name="GetDefaultInstance" />
    
      <assembly fullname="System.Security.Principal.Windows">
          <!-- called through reflection by AppDomain -->
          <method name="GetDefaultInstance" />
    • Reflection from tests
        <type fullname="System.Runtime.Serialization.DataContractSerializer">
          <!-- called through reflection by tests -->
          <method name="set_Option" />
        </type>
    • Debugging support
        <!-- Required by the Expression Evaluator -->
        <type fullname="Microsoft.CSharp.RuntimeBinder.DynamicMetaObjectProviderDebugView"/>
        <!-- Required by the Expression Evaluator -->
        <type fullname="Microsoft.CSharp.RuntimeBinder.DynamicMetaObjectProviderDebugView/DynamicDebugViewEmptyException"/>
    
        <!-- used by System.Private.CoreLib StackTrace code to load portable pdbs for the runtime diagnostic stack trace to get source/line info -->
        <type fullname="System.Diagnostics.StackTraceSymbols" required="true" />
    
        <!-- required by debugger, see comment in System/Linq/DebugView.cs -->
        <type fullname="System.Linq.SystemCore_EnumerableDebugView" />
        <type fullname="System.Linq.SystemCore_EnumerableDebugView`1" />
        <type fullname="System.Linq.SystemCore_EnumerableDebugViewEmptyException" />
    • Invoked by COM
        <!-- Required for COM event dispatch -->
        <type fullname="System.Runtime.InteropServices.ComEventsSink"/>
    
        <!-- The following types support the CLR's IClassFactory2 marshaling from CoreLib: -->
        <type fullname="System.ComponentModel.LicenseManager/LicenseInteropHelper" />
        <type fullname="System.ComponentModel.LicenseManager/CLRLicenseContext" />
        <type fullname="System.ComponentModel.LicenseManager/LicInfoHelperLicenseContext" />
    
        <!-- IMarshal is internal and never directly called, but needed by COM -->
        <type fullname="System.Runtime.InteropServices.IMarshal" />
    • Invoked by our tools/infra
        <type fullname="System.Xml.Serialization.XmlSerializer">
          <!-- called via reflection from SGen tool -->
          <method name="set_Mode" />
          <method name="GenerateSerializer" />
        </type>
    
        <type fullname="System.Xml.Linq.XElement">
          <!-- needed by serialization -->
          <method name=".ctor" />
        </type>
  6. eerhardt commented on Apr 20, 2020

    @eerhardt
    Member

    This is solvable by adding PreserveDependency attribute on the callsite. But it requires #31712 to be done. Without it the code would be trimmed when linker runs on just the one assembly in the framework at a time.

    I think this could be done without requiring #31712. One option could be:

    • When linking for Xamarin, add --ignore-descriptors true to the linker, which will cause the linker to ignore the ILLinkTrim.xml files.
    • We also go and add the correct PreserveDependency attributes where necessary to the libraries.
      • this means we would temporarily have duplicate info in ILLinkTrim.xml and in the PreserveDependency attributes. Until we remove ILLinkTrim.xml files completely.

    Now when the linker runs, it only sees the PreserveDependency attributes, and doesn't leave unnecessary code in the final output.

    Another option entirely would be to add different "categories" or "levels" to the entries in the ILLinkTrim.xml file. And when we run the linker against individual libraries we tell it to respect "all levels" in the ILLinkTrim.xml. And when we run the linker against the final app, we tell it to only respect the "high priority" entries (which would probably be quite small).

  7. marek-safar commented on Apr 21, 2020

    @marek-safar
    ContributorAuthor

    When linking for Xamarin, add --ignore-descriptors true to the linker, which will cause the linker to ignore the ILLinkTrim.xml files.

    This is no-go as that would ignore descriptor in all libraries including 3rd party libraries.

    And when we run the linker against the final app, we tell it to only respect the "high priority" entries (which would probably be quite small).

    How would we do that? That's exactly for linker is suppose to do automatically when code is required.

    The linker descriptors are intended for code which has to be always persisted, usage above is sub-optimal for BCL and should be fixed not hacked around.

  8. 5 remaining items

  9. self-assigned this
    on May 26, 2020
  10. eerhardt commented on Jul 10, 2020

    @eerhardt
    Member

    I believe all the 5.0 work is now done for this issue. Looking at the remaining ILLink.Descriptors.xml/ILLinkTrim.xml files in the repo, here is what is left:

    1. In CoreCLR's CoreLib and in Microsoft.CSharp, we still have COM support unconditionally rooted. This will be addressed with Add the ability to remove COM support using ILLinker #36659.
    2. Outside of COM support, CoreCLR's CoreLib has a few methods rooted that are used by external products through private reflection. And one case of EventSource's private reflection to NativeRuntimeEventSource, which are used by external code (C/C++).
    3. In the shared CoreLib ILLink.Descriptors file there are 2 super small entries: GetICUVersion (an extern method that is only used by tests) and the .ctor of ThreadPoolBoundHandle to work around a linker issue. The amount of work necessary to remove these probably doesn't justify the minimal gains we will get by removing these small methods.
    4. In Mono CoreLib, almost all of the entries are because of dependencies in the native mono runtime. I do see one entry for Used by binary formatter tests for ThreadStartException's .ctor. This same entry is in CoreCLR, and the exception is marked as [Serializable], so I think that entry needs to stay.
    5. System.Runtime.InteropServices.JavaScript is rooting the Runtime class, but this is intentional since these methods are called by external code (Javascript).

    Thus, for the libraries (outside of the CoreLibs) the only descriptor left that is unconditional is the COM usage in Microsoft.CSharp, which is tracked by a separate issue.

    I believe we can close this issue now.

  11. marek-safar commented on Jul 10, 2020

    @marek-safar
    ContributorAuthor

    I agree except for one entry which is GetICUVersion. Marking it all the time will cause problems for us in InvariantMode when the native part won't be available and we need to statically link it.

  12. eerhardt commented on Jul 10, 2020

    @eerhardt
    Member

    Opened #39102 specifically for this last entry, so someone doesn't need to read through this whole issue to see what work is left.

  13. ghost locked as resolved and limited conversation to collaborators on Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

area-Metalinkable-frameworkIssues associated with delivering a linker friendly framework

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions