From 13e62212b9b6de6ff0b198207fcbe5531bf6b442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 20 Mar 2024 09:49:22 +0100 Subject: [PATCH 1/3] Trim unused interfaces So far, the compiler could only trim unused interface __methods__; the interface __types__ themselves were left alone. There is not a huge cost associated with having these extra `MethodTable`s, but it sill impacts size/working set/startup. Initial estimate showed this could actually be up to 2% for BasicMinimalApi so I decided to investigate a fix. This is an attempt to start trimming them. I chose a relatively conservative approach: * Stop unconditionally rooting the interface `MethodTable` in the implementing class `MethodTable` InterfaceList. Instead check whether someone else marked it first. * Track whether an interface type definition is used in any way. This avoids problem with variance, etc. If an interface definition is used, all instantiations that we were trying to optimize away get the `MethodTable` and won't be optimized. We should be able to do better than this, maybe, at some point. * Classes that implement generic interfaces with default methods need special treatment because we index into interface list at runtime and we might not know the correct index yet. So just forget the optimization in that case. Fixes #66716. --- .../src/System/Runtime/TypeCast.cs | 15 +++-- .../DynamicallyAccessedMembersBinder.cs | 6 ++ .../Compiler/Dataflow/ReflectionMarker.cs | 2 +- .../DependencyAnalysis/CanonicalEETypeNode.cs | 6 +- .../Compiler/DependencyAnalysis/EETypeNode.cs | 48 ++++++++++++-- .../InterfaceDispatchMapNode.cs | 15 +++-- .../DependencyAnalysis/InterfaceUseNode.cs | 40 ++++++++++++ .../NativeLayoutVertexNode.cs | 18 ++++++ .../NecessaryCanonicalEETypeNode.cs | 6 +- .../DependencyAnalysis/NodeFactory.cs | 12 ++++ .../DependencyAnalysis/SealedVTableNode.cs | 64 +++++++++++-------- .../ILCompiler.Compiler.csproj | 1 + 12 files changed, 188 insertions(+), 45 deletions(-) create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceUseNode.cs diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 3dcf2bae02f246..8286cec024953f 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -146,12 +146,17 @@ internal enum AssignmentVariation interfaceMap++; interfaceCount--; } while (interfaceCount > 0); + } - extra: - if (mt->IsIDynamicInterfaceCastable) - { - goto slowPath; - } + extra: + // NOTE: this check is outside the `if (interfaceCount != 0)` check because + // we could have devirtualized and inlined all uses of IDynamicInterfaceCastable + // (and optimized the interface MethodTable away) and still have a type that + // is legitimately marked IDynamicInterfaceCastable (without having the MethodTable + // of IDynamicInterfaceCastable in the interface list). + if (mt->IsIDynamicInterfaceCastable) + { + goto slowPath; } obj = null; diff --git a/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs b/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs index 6780cb5619289a..a590d46650c899 100644 --- a/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs @@ -121,6 +121,12 @@ public static IEnumerable GetDynamicallyAccessedMembers(this T foreach (var e in typeDefinition.GetEventsOnTypeHierarchy(filter: null, bindingFlags: BindingFlags.Public | declaredOnlyFlags)) yield return e; } + + if (memberTypes.HasFlag(DynamicallyAccessedMemberTypes.Interfaces)) + { + foreach (DefType iface in typeDefinition.GetAllInterfaceImplementations(declaredOnly: true)) + yield return iface; + } } public static IEnumerable GetConstructorsOnType(this TypeDesc type, Func filter, BindingFlags? bindingFlags = null) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs index e33fb398605950..279418b2fac3c9 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMarker.cs @@ -79,7 +79,7 @@ internal void MarkTypeSystemEntity(in MessageOrigin origin, TypeSystemEntity ent MarkEvent(origin, @event, reason, accessKind); break; // case InterfaceImplementation - // Nothing to do currently as Native AOT will preserve all interfaces on a preserved type + // This is handled in the MetadataType case above } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CanonicalEETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CanonicalEETypeNode.cs index 38381c4a57887f..c86e85a3be546a 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CanonicalEETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CanonicalEETypeNode.cs @@ -96,8 +96,12 @@ protected override void OutputGCDesc(ref ObjectDataBuilder builder) protected override void OutputInterfaceMap(NodeFactory factory, ref ObjectDataBuilder objData) { - for (int i = 0; i < _type.RuntimeInterfaces.Length; i++) + foreach (DefType intface in _type.RuntimeInterfaces) { + // If the interface was optimized away, skip it + if (!factory.InterfaceUse(intface.GetTypeDefinition()).Marked) + continue; + // Interface omitted for canonical instantiations (constructed at runtime for dynamic types from the native layout info) objData.EmitZeroPointer(); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs index 8149c2c7886ddf..a6e213d34120bb 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -299,6 +299,12 @@ public sealed override bool HasConditionalStaticDependencies return true; } + // If the type implements at least one interface, calls against that interface could result in this type's + // implementation being used. + // It might also be necessary for casting purposes. + if (_type.RuntimeInterfaces.Length > 0) + return true; + if (!EmitVirtualSlots) return false; @@ -328,11 +334,6 @@ public sealed override bool HasConditionalStaticDependencies currentType = currentType.BaseType; } - // If the type implements at least one interface, calls against that interface could result in this type's - // implementation being used. - if (_type.RuntimeInterfaces.Length > 0) - return true; - return _hasConditionalDependenciesFromMetadataManager; } } @@ -367,6 +368,18 @@ public sealed override IEnumerable GetConditionalSt "Information about static bases for type with template")); } + if (!_type.IsGenericDefinition && !_type.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + foreach (DefType iface in _type.RuntimeInterfaces) + { + var ifaceDefinition = (DefType)iface.GetTypeDefinition(); + result.Add(new CombinedDependencyListEntry( + GetInterfaceTypeNode(factory, iface), + factory.InterfaceUse(ifaceDefinition), + "Interface definition was visible")); + } + } + if (!EmitVirtualSlots) return result; @@ -526,6 +539,14 @@ public sealed override IEnumerable GetConditionalSt { // Canonical instance default methods need to go through a thunk that adds the right generic context defaultIntfMethod = factory.TypeSystemContext.GetDefaultInterfaceMethodImplementationThunk(defaultIntfMethod, defType.ConvertToCanonForm(CanonicalFormKind.Specific), providingInterfaceDefinitionType); + + // The above thunk will index into interface list to find the right context. Make sure to keep all interfaces prior to this one + for (int i = 0; i < interfaceIndex; i++) + { + result.Add(new CombinedDependencyListEntry( + factory.InterfaceUse(defTypeRuntimeInterfaces[i].GetTypeDefinition()), + factory.VirtualMethodUse(interfaceMethod), "Interface with shared default methods folows this")); + } } result.Add(new CombinedDependencyListEntry(factory.MethodEntrypoint(defaultIntfMethod), factory.VirtualMethodUse(interfaceMethod), "Interface method")); @@ -580,6 +601,9 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact // emitting it. dependencies.Add(new DependencyListEntry(_optionalFieldsNode, "Optional fields")); + if (_type.IsInterface) + dependencies.Add(factory.InterfaceUse(_type.GetTypeDefinition()), "Interface is used"); + if (EmitVirtualSlots) { if (!_type.IsArrayTypeWithoutGenericInterfaces()) @@ -690,7 +714,11 @@ protected override ObjectData GetDehydratableData(NodeFactory factory, bool relo // Emit interface map SlotCounter interfaceSlotCounter = SlotCounter.BeginCounting(ref /* readonly */ objData); - OutputInterfaceMap(factory, ref objData); + + if (!relocsOnly) + { + OutputInterfaceMap(factory, ref objData); + } // Update slot count int numberOfInterfaceSlots = interfaceSlotCounter.CountSlots(ref /* readonly */ objData); @@ -1090,7 +1118,13 @@ protected virtual void OutputInterfaceMap(NodeFactory factory, ref ObjectDataBui { foreach (var itf in _type.RuntimeInterfaces) { - objData.EmitPointerReloc(GetInterfaceTypeNode(factory, itf)); + IEETypeNode interfaceTypeNode = GetInterfaceTypeNode(factory, itf); + + // Only emit interfaces that were not optimized away. + if (interfaceTypeNode.Marked) + { + objData.EmitPointerReloc(interfaceTypeNode); + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchMapNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchMapNode.cs index 94cde954dc01fc..c902c896cebae9 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchMapNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceDispatchMapNode.cs @@ -164,6 +164,8 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory) bool needsEntriesForInstanceInterfaceMethodImpls = !isInterface || ((MetadataType)declType).IsDynamicInterfaceCastableImplementation(); + int entryIndex = 0; + // Resolve all the interfaces, but only emit non-static and non-default implementations for (int interfaceIndex = 0; interfaceIndex < declTypeRuntimeInterfaces.Length; interfaceIndex++) { @@ -171,6 +173,9 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory) var definitionInterfaceType = declTypeDefinitionRuntimeInterfaces[interfaceIndex]; Debug.Assert(interfaceType.IsInterface); + if (!factory.InterfaceUse(interfaceType.GetTypeDefinition()).Marked) + continue; + IReadOnlyList virtualSlots = factory.VTable(interfaceType).Slots; for (int interfaceMethodSlot = 0; interfaceMethodSlot < virtualSlots.Count; interfaceMethodSlot++) @@ -210,11 +215,11 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory) int genericContext = targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific).RequiresInstArg() ? StaticVirtualMethodContextSource.ContextFromThisClass : StaticVirtualMethodContextSource.None; - staticImplementations.Add((interfaceIndex, emittedInterfaceSlot, emittedImplSlot, genericContext)); + staticImplementations.Add((entryIndex, emittedInterfaceSlot, emittedImplSlot, genericContext)); } else { - builder.EmitShort((short)checked((ushort)interfaceIndex)); + builder.EmitShort((short)checked((ushort)entryIndex)); builder.EmitShort((short)checked((ushort)emittedInterfaceSlot)); builder.EmitShort((short)checked((ushort)emittedImplSlot)); entryCount++; @@ -271,7 +276,7 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory) } } staticDefaultImplementations.Add(( - interfaceIndex, + entryIndex, emittedInterfaceSlot, implSlot.Value, genericContext)); @@ -279,13 +284,15 @@ private void EmitDispatchMap(ref ObjectDataBuilder builder, NodeFactory factory) else { defaultImplementations.Add(( - interfaceIndex, + entryIndex, emittedInterfaceSlot, implSlot.Value)); } } } } + + entryIndex++; } // Now emit the default implementations diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceUseNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceUseNode.cs new file mode 100644 index 00000000000000..9ef45bb711844c --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/InterfaceUseNode.cs @@ -0,0 +1,40 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +using ILCompiler.DependencyAnalysisFramework; + +using Internal.TypeSystem; + +using Debug = System.Diagnostics.Debug; + +namespace ILCompiler.DependencyAnalysis +{ + /// + /// Tracks uses of interface in IL sense: at the level of type definitions. + /// Trim warning suppressions within the framework prevent us from optimizing + /// at a smaller granularity (e.g. canonical forms or concrete instantiations). + /// + internal sealed class InterfaceUseNode : DependencyNodeCore + { + public TypeDesc Type { get; } + + public InterfaceUseNode(TypeDesc type) + { + Debug.Assert(type.IsTypeDefinition); + Debug.Assert(type.IsInterface); + Type = type; + } + + protected override string GetName(NodeFactory factory) => $"Interface use: {Type}"; + + public override IEnumerable GetStaticDependencies(NodeFactory factory) => null; + public override bool InterestingForDynamicDependencyAnalysis => false; + public override bool HasDynamicDependencies => false; + public override bool HasConditionalStaticDependencies => false; + public override bool StaticDependenciesAreComputed => true; + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) => null; + public override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory context) => null; + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs index 1248906a4fba37..037a1aff341626 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs @@ -909,6 +909,15 @@ public override IEnumerable GetStaticDependencies(NodeFacto } } + foreach (GenericParameterDesc genericParam in _method.GetTypicalMethodDefinition().Instantiation) + { + foreach (TypeDesc typeConstraint in genericParam.TypeConstraints) + { + if (typeConstraint.IsInterface) + yield return new DependencyListEntry(context.InterfaceUse(typeConstraint.GetTypeDefinition()), "Used as constraint"); + } + } + yield return new DependencyListEntry(context.GenericDictionaryLayout(_method), "Dictionary layout"); } @@ -1026,6 +1035,15 @@ public override IEnumerable GetStaticDependencies(NodeFacto yield return new DependencyListEntry(context.MethodEntrypoint(_type.GetStaticConstructor().GetCanonMethodTarget(CanonicalFormKind.Specific)), "cctor for template"); } + foreach (GenericParameterDesc genericParam in _type.GetTypeDefinition().Instantiation) + { + foreach (TypeDesc typeConstraint in genericParam.TypeConstraints) + { + if (typeConstraint.IsInterface) + yield return new DependencyListEntry(context.InterfaceUse(typeConstraint.GetTypeDefinition()), "Used as constraint"); + } + } + if (!_isUniversalCanon) { DefType closestCanonDefType = (DefType)_type.GetClosestDefType().ConvertToCanonForm(CanonicalFormKind.Specific); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NecessaryCanonicalEETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NecessaryCanonicalEETypeNode.cs index b3a020e7d67ede..7fda2963953282 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NecessaryCanonicalEETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NecessaryCanonicalEETypeNode.cs @@ -23,8 +23,12 @@ public NecessaryCanonicalEETypeNode(NodeFactory factory, TypeDesc type) : base(f protected override void OutputInterfaceMap(NodeFactory factory, ref ObjectDataBuilder objData) { - for (int i = 0; i < _type.RuntimeInterfaces.Length; i++) + foreach (DefType intface in _type.RuntimeInterfaces) { + // If the interface was optimized away, skip it + if (!factory.InterfaceUse(intface.GetTypeDefinition()).Marked) + continue; + // Interface omitted for canonical instantiations (constructed at runtime for dynamic types from the native layout info) objData.EmitZeroPointer(); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs index dd839dc612de7c..3d213666a3d87f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -366,6 +366,11 @@ private void CreateNodeCaches() return new VariantInterfaceMethodUseNode(method); }); + _interfaceUses = new NodeCache((TypeDesc type) => + { + return new InterfaceUseNode(type); + }); + _readyToRunHelpers = new NodeCache(CreateReadyToRunHelperNode); _genericReadyToRunHelpersFromDict = new NodeCache(CreateGenericLookupFromDictionaryNode); @@ -1175,6 +1180,13 @@ public DependencyNodeCore VariantInterfaceMethodUse(MethodDesc decl return _variantMethods.GetOrAdd(decl); } + private NodeCache _interfaceUses; + + public DependencyNodeCore InterfaceUse(TypeDesc type) + { + return _interfaceUses.GetOrAdd(type); + } + private NodeCache _readyToRunHelpers; public ISymbolNode ReadyToRunHelper(ReadyToRunHelperId id, object target) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs index 85c9e40eec058f..ff7745b59f329d 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs @@ -14,6 +14,7 @@ public class SealedVTableNode : ObjectNode, ISymbolDefinitionNode { private readonly TypeDesc _type; private List _sealedVTableEntries; + private DependencyList _nonRelocationDependencies; public SealedVTableNode(TypeDesc type) { @@ -126,7 +127,10 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) MethodDesc implMethod = declType.FindVirtualFunctionTargetMethodOnObjectType(virtualSlots[i]); if (implMethod.CanMethodBeInSealedVTable(factory)) - _sealedVTableEntries.Add(SealedVTableEntry.FromVirtualMethod(implMethod)); + { + IMethodNode node = factory.MethodEntrypoint(implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !implMethod.Signature.IsStatic && declType.IsValueType); + _sealedVTableEntries.Add(SealedVTableEntry.FromVirtualMethod(implMethod, node)); + } } TypeDesc declTypeDefinition = declType.GetTypeDefinition(); @@ -173,7 +177,10 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) targetMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(implMethod.GetTypicalMethodDefinition(), (InstantiatedType)implType); if (targetMethod.CanMethodBeInSealedVTable(factory) || implMethod.Signature.IsStatic) - _sealedVTableEntries.Add(SealedVTableEntry.FromVirtualMethod(targetMethod)); + { + IMethodNode node = factory.MethodEntrypoint(targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !targetMethod.Signature.IsStatic && declType.IsValueType); + _sealedVTableEntries.Add(SealedVTableEntry.FromVirtualMethod(targetMethod, node)); + } } } else @@ -185,7 +192,24 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) { DefType providingInterfaceDefinitionType = (DefType)implMethod.OwningType; implMethod = implMethod.InstantiateSignature(declType.Instantiation, Instantiation.Empty); - _sealedVTableEntries.Add(SealedVTableEntry.FromDefaultInterfaceMethod(implMethod, providingInterfaceDefinitionType)); + + MethodDesc canonImplMethod = implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); + if (canonImplMethod.IsCanonicalMethod(CanonicalFormKind.Any) && !canonImplMethod.Signature.IsStatic) + { + // Canonical instance default interface methods need to go through a thunk that acquires the generic context from `this`. + // Static methods have their generic context passed explicitly. + canonImplMethod = factory.TypeSystemContext.GetDefaultInterfaceMethodImplementationThunk(canonImplMethod, declType.ConvertToCanonForm(CanonicalFormKind.Specific), providingInterfaceDefinitionType); + + // The above thunk will index into interface list to find the right context. Make sure to keep all interfaces prior to this one + for (int i = 0; i < interfaceIndex; i++) + { + _nonRelocationDependencies ??= new DependencyList(); + _nonRelocationDependencies.Add(factory.InterfaceUse(declTypeRuntimeInterfaces[i].GetTypeDefinition()), "Interface with shared default methods folows this"); + } + } + IMethodNode node = factory.MethodEntrypoint(canonImplMethod, unboxingStub: implMethod.OwningType.IsValueType && !implMethod.Signature.IsStatic); + + _sealedVTableEntries.Add(SealedVTableEntry.FromDefaultInterfaceMethod(implMethod, providingInterfaceDefinitionType, node)); } } } @@ -196,7 +220,9 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory) { - var result = new DependencyList(); + BuildSealedVTableSlots(factory, relocsOnly: true); + + var result = new DependencyList(_nonRelocationDependencies ?? []); // When building the sealed vtable, we consult the vtable layout of these types TypeDesc declType = _type.GetClosestDefType(); @@ -216,11 +242,9 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly) if (BuildSealedVTableSlots(factory, relocsOnly)) { - DefType defType = _type.GetClosestDefType(); - for (int i = 0; i < _sealedVTableEntries.Count; i++) { - IMethodNode relocTarget = _sealedVTableEntries[i].GetTarget(factory, defType); + IMethodNode relocTarget = _sealedVTableEntries[i].Target; if (factory.Target.SupportsRelativePointers) objData.EmitReloc(relocTarget, RelocType.IMAGE_REL_BASED_RELPTR32); @@ -242,31 +266,19 @@ private readonly struct SealedVTableEntry { private readonly MethodDesc _method; private readonly DefType _interfaceDefinition; + public readonly IMethodNode Target; - private SealedVTableEntry(MethodDesc method, DefType interfaceDefinition) + private SealedVTableEntry(MethodDesc method, DefType interfaceDefinition, IMethodNode target) { Debug.Assert(interfaceDefinition == null || method.GetTypicalMethodDefinition().OwningType == interfaceDefinition.GetTypeDefinition()); - (_method, _interfaceDefinition) = (method, interfaceDefinition); + (_method, _interfaceDefinition, Target) = (method, interfaceDefinition, target); } - public static SealedVTableEntry FromVirtualMethod(MethodDesc method) - => new SealedVTableEntry(method, null); + public static SealedVTableEntry FromVirtualMethod(MethodDesc method, IMethodNode target) + => new SealedVTableEntry(method, null, target); - public static SealedVTableEntry FromDefaultInterfaceMethod(MethodDesc method, DefType interfaceOnDefinition) - => new SealedVTableEntry(method, interfaceOnDefinition); - - public IMethodNode GetTarget(NodeFactory factory, TypeDesc implementingClass) - { - bool isStaticVirtualMethod = _method.Signature.IsStatic; - MethodDesc implMethod = _method.GetCanonMethodTarget(CanonicalFormKind.Specific); - if (_interfaceDefinition != null && !isStaticVirtualMethod && implMethod.IsCanonicalMethod(CanonicalFormKind.Any)) - { - // Canonical instance default interface methods need to go through a thunk that acquires the generic context from `this`. - // Static methods have their generic context passed explicitly. - implMethod = factory.TypeSystemContext.GetDefaultInterfaceMethodImplementationThunk(implMethod, implementingClass.ConvertToCanonForm(CanonicalFormKind.Specific), _interfaceDefinition); - } - return factory.MethodEntrypoint(implMethod, unboxingStub: !isStaticVirtualMethod && _method.OwningType.IsValueType); - } + public static SealedVTableEntry FromDefaultInterfaceMethod(MethodDesc method, DefType interfaceOnDefinition, IMethodNode target) + => new SealedVTableEntry(method, interfaceOnDefinition, target); public bool Matches(MethodDesc method) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index 3fa2e02e88d0f2..190544b31373df 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -422,6 +422,7 @@ + From 008f01ec8042cfa23abe4dff91bc07cde848b0c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 21 Mar 2024 07:52:52 +0100 Subject: [PATCH 2/3] Also remove from reflection metadata --- .../Compiler/AnalysisBasedMetadataManager.cs | 5 +++++ .../Compiler/DependencyAnalysis/TypeMetadataNode.cs | 4 ---- .../ILCompiler.Compiler/Compiler/MetadataManager.cs | 12 ++++++++++++ .../Compiler/UsageBasedMetadataManager.cs | 5 +++++ .../ILCompiler/Metadata/IMetadataPolicy.cs | 5 +++++ .../ILCompiler/Metadata/Transform.Type.cs | 3 +++ .../nativeaot/SmokeTests/Reflection/Reflection.cs | 7 +++++-- 7 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs index 8c24c7addbb080..7b9f19c8e8be16 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedMetadataManager.cs @@ -261,6 +261,11 @@ public bool GeneratesMetadata(EcmaModule module, ExportedTypeHandle exportedType return GeneratesMetadata(targetType); } + public bool GeneratesInterfaceImpl(MetadataType typeDef, MetadataType interfaceImpl) + { + return _parent.IsInterfaceUsed(interfaceImpl.GetTypeDefinition()); + } + public bool IsBlocked(MetadataType typeDef) { return _blockingPolicy.IsBlocked(typeDef); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs index b76ec72538a22e..87a2c829c0269b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs @@ -49,10 +49,6 @@ public override IEnumerable GetStaticDependencies(NodeFacto if (baseType != null) GetMetadataDependencies(ref dependencies, factory, baseType, "Base type of a reflectable type"); - // TODO-SIZE: if we start trimming interface lists, we can probably trim here - foreach (DefType interfaceType in _type.ExplicitlyImplementedInterfaces) - GetMetadataDependencies(ref dependencies, factory, interfaceType, "Interface of a reflectable type"); - var mdManager = (UsageBasedMetadataManager)factory.MetadataManager; if (_type.IsEnum) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs index 2f92ccdb9048e7..19952d1209b659 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs @@ -77,6 +77,7 @@ private readonly SortedSet _typeGVMEntries private readonly SortedSet _typeTemplates = new SortedSet(TypeSystemComparer.Instance); private readonly SortedSet _typesWithGenericStaticBaseInfo = new SortedSet(TypeSystemComparer.Instance); private readonly SortedSet _genericMethodHashtableEntries = new SortedSet(TypeSystemComparer.Instance); + private readonly HashSet _usedInterfaces = new HashSet(); private List<(DehydratableObjectNode Node, ObjectNode.ObjectData Data)> _dehydratableData = new List<(DehydratableObjectNode Node, ObjectNode.ObjectData data)>(); @@ -328,6 +329,11 @@ protected virtual void Graph_NewMarkedNode(DependencyNodeCore obj) { _genericMethodHashtableEntries.Add(genericMethodsHashtableEntryNode.Method); } + + if (obj is InterfaceUseNode interfaceUse) + { + _usedInterfaces.Add(interfaceUse.Type); + } } protected virtual bool AllMethodsCanBeReflectable => false; @@ -985,6 +991,12 @@ public IEnumerable GetGenericMethodHashtableEntries() return _genericMethodHashtableEntries; } + public bool IsInterfaceUsed(TypeDesc type) + { + Debug.Assert(type.IsTypeDefinition); + return _usedInterfaces.Contains(type); + } + internal IEnumerable GetCompiledMethodBodies() { return _methodBodiesGenerated; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs index 2b823f42465108..f5033e521a6670 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs @@ -1041,6 +1041,11 @@ public bool GeneratesMetadata(EcmaModule module, ExportedTypeHandle exportedType return GeneratesMetadata(targetType) || !_factory.CompilationModuleGroup.ContainsType(targetType); } + public bool GeneratesInterfaceImpl(MetadataType typeDef, MetadataType interfaceImpl) + { + return _factory.MetadataManager.IsInterfaceUsed(interfaceImpl.GetTypeDefinition()); + } + public bool IsBlocked(MetadataType typeDef) { return _blockingPolicy.IsBlocked(typeDef); diff --git a/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/IMetadataPolicy.cs b/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/IMetadataPolicy.cs index 3aa61454e57f9b..408a3be6ded51f 100644 --- a/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/IMetadataPolicy.cs +++ b/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/IMetadataPolicy.cs @@ -48,6 +48,11 @@ public interface IMetadataPolicy /// bool GeneratesMetadata(Cts.Ecma.EcmaModule module, Ecma.ExportedTypeHandle exportedType); + /// + /// Returns true if InterfaceImpl should be generated for this type. + /// + bool GeneratesInterfaceImpl(Cts.MetadataType typeDef, Cts.MetadataType interfaceImpl); + /// /// Returns true if a type should be blocked from generating any metadata. /// Blocked interfaces are skipped from interface lists, and custom attributes referring to diff --git a/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/Transform.Type.cs b/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/Transform.Type.cs index fc3ea4922c8172..4ab76e79005da8 100644 --- a/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/Transform.Type.cs +++ b/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler/Metadata/Transform.Type.cs @@ -287,6 +287,9 @@ private void InitializeTypeDef(Cts.MetadataType entity, TypeDefinition record) { if (IsBlocked(interfaceType)) continue; + if (!_policy.GeneratesInterfaceImpl(entity, (Cts.MetadataType)interfaceType)) + continue; + record.Interfaces.Add(HandleType(interfaceType)); } } diff --git a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs index 27f332cbdd14c6..acad3d3934194a 100644 --- a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs +++ b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs @@ -1042,7 +1042,9 @@ class TestTypesInMethodSignatures { interface IUnreferenced { } - class UnreferencedBaseType : IUnreferenced { } + interface IReferenced { } + + class UnreferencedBaseType : IUnreferenced, IReferenced { } class UnreferencedMidType : UnreferencedBaseType { } class ReferencedDerivedType : UnreferencedMidType { } @@ -1061,8 +1063,9 @@ public static void Run() Assert.Equal(count, 3); - // This one could in theory fail if we start trimming interface lists + // We expect to see only IReferenced but not IUnreferenced Assert.Equal(1, mi.GetParameters()[0].ParameterType.GetInterfaces().Length); + Assert.Equal(typeof(IReferenced), mi.GetParameters()[0].ParameterType.GetInterfaces()[0]); } } From 7a4a308fa7351ff447cffc1f465af4db98d0e411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 21 Mar 2024 12:07:28 +0100 Subject: [PATCH 3/3] Fix tests --- .../Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs | 2 +- ...WithPreserveFieldsHasBackingFieldsOfPropertiesRemoved.cs | 6 +----- ...UnusedInterfaceTypeOnTypeWithPreserveNothingIsRemoved.cs | 1 - 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs b/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs index a590d46650c899..3b317fbd01cd34 100644 --- a/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs +++ b/src/coreclr/tools/Common/Compiler/Dataflow/DynamicallyAccessedMembersBinder.cs @@ -124,7 +124,7 @@ public static IEnumerable GetDynamicallyAccessedMembers(this T if (memberTypes.HasFlag(DynamicallyAccessedMemberTypes.Interfaces)) { - foreach (DefType iface in typeDefinition.GetAllInterfaceImplementations(declaredOnly: true)) + foreach (DefType iface in typeDefinition.GetAllInterfaceImplementations(declaredOnly)) yield return iface; } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/TypeWithPreserveFieldsHasBackingFieldsOfPropertiesRemoved.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/TypeWithPreserveFieldsHasBackingFieldsOfPropertiesRemoved.cs index 4384c8d2a4333e..f516a7945f9abf 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/TypeWithPreserveFieldsHasBackingFieldsOfPropertiesRemoved.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/TypeWithPreserveFieldsHasBackingFieldsOfPropertiesRemoved.cs @@ -40,32 +40,28 @@ class Unused : IFoo, IFoo, IFoo, IFoo2, IFoo3>.Bar { get; set; } } - [Kept (By = Tool.NativeAot)] interface IDog { string Name { get; set; } } - [Kept] + [Kept (By = Tool.Trimmer)] interface IFoo { int Bar { get; set; } } - [Kept (By = Tool.NativeAot)] interface IFoo2 { int Bar2 { get; set; } } - [Kept (By = Tool.NativeAot)] interface IFoo3 { int Bar3 { get; set; } } - [Kept (By = Tool.NativeAot)] class Cat { } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UnusedInterfaceTypeOnTypeWithPreserveNothingIsRemoved.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UnusedInterfaceTypeOnTypeWithPreserveNothingIsRemoved.cs index 31659ef977254b..ad436e324c4d4d 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UnusedInterfaceTypeOnTypeWithPreserveNothingIsRemoved.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UnusedInterfaceTypeOnTypeWithPreserveNothingIsRemoved.cs @@ -10,7 +10,6 @@ public static void Main () { } - [Kept (By = Tool.NativeAot)] interface IFoo { }