Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce the number of forced MethodTables #79594

Merged
merged 4 commits into from Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -23,6 +23,7 @@ public sealed class AnalysisBasedMetadataManager : GeneratingMetadataManager, IC
{
private readonly List<ModuleDesc> _modulesWithMetadata;
private readonly List<MetadataType> _typesWithRootedCctorContext;
private readonly List<TypeDesc> _forcedTypes;

private readonly Dictionary<TypeDesc, MetadataCategory> _reflectableTypes = new Dictionary<TypeDesc, MetadataCategory>();
private readonly Dictionary<MethodDesc, MetadataCategory> _reflectableMethods = new Dictionary<MethodDesc, MetadataCategory>();
Expand All @@ -32,7 +33,7 @@ public sealed class AnalysisBasedMetadataManager : GeneratingMetadataManager, IC
public AnalysisBasedMetadataManager(CompilerTypeSystemContext typeSystemContext)
: this(typeSystemContext, new FullyBlockedMetadataBlockingPolicy(),
new FullyBlockedManifestResourceBlockingPolicy(), null, new NoStackTraceEmissionPolicy(),
new NoDynamicInvokeThunkGenerationPolicy(), Array.Empty<ModuleDesc>(),
new NoDynamicInvokeThunkGenerationPolicy(), Array.Empty<ModuleDesc>(), Array.Empty<TypeDesc>(),
Array.Empty<ReflectableEntity<TypeDesc>>(), Array.Empty<ReflectableEntity<MethodDesc>>(),
Array.Empty<ReflectableEntity<FieldDesc>>(), Array.Empty<ReflectableCustomAttribute>(),
Array.Empty<MetadataType>(), default)
Expand All @@ -47,6 +48,7 @@ public AnalysisBasedMetadataManager(CompilerTypeSystemContext typeSystemContext)
StackTraceEmissionPolicy stackTracePolicy,
DynamicInvokeThunkGenerationPolicy invokeThunkGenerationPolicy,
IEnumerable<ModuleDesc> modulesWithMetadata,
IEnumerable<TypeDesc> forcedTypes,
IEnumerable<ReflectableEntity<TypeDesc>> reflectableTypes,
Comment on lines +51 to 52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between forcedTypes and reflectableTypes - per the change description these should be basically identical now...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reflectableTypes are types that are eligible to be reflection visible, but are not rooted. forcedTypes are rooted.

I forgot to mention that we have a concept of reflection blocked types - types that produce a special System.Type at runtime.

reflectableTypes basically helps compilation phase to decide whether something should be blocked or not.

It unfortunately doesn't match well with the below reflectableFields and reflectableMethods because types have the unfortunate property of becoming visible to reflection just because they exist (unless blocked).

IEnumerable<ReflectableEntity<MethodDesc>> reflectableMethods,
IEnumerable<ReflectableEntity<FieldDesc>> reflectableFields,
Expand All @@ -57,6 +59,7 @@ public AnalysisBasedMetadataManager(CompilerTypeSystemContext typeSystemContext)
{
_modulesWithMetadata = new List<ModuleDesc>(modulesWithMetadata);
_typesWithRootedCctorContext = new List<MetadataType>(rootedCctorContexts);
_forcedTypes = new List<TypeDesc>(forcedTypes);

foreach (var refType in reflectableTypes)
{
Expand Down Expand Up @@ -180,10 +183,9 @@ void ICompilationRootProvider.AddCompilationRoots(IRootingServiceProvider rootPr

const string reason = "Reflection";

foreach (var pair in _reflectableTypes)
foreach (var type in _forcedTypes)
{
if ((pair.Value & MetadataCategory.RuntimeMapping) != 0)
rootProvider.AddCompilationRoot(pair.Key, reason);
rootProvider.AddReflectionRoot(type, reason);
}

foreach (var pair in _reflectableMethods)
Expand Down
Expand Up @@ -341,7 +341,7 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet
{
if (systemTypeValue.RepresentedType.Type.IsEnum)
{
reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ConstructedTypeSymbol(systemTypeValue.RepresentedType.Type.MakeArrayType()), "Enum.GetValues");
reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedType(systemTypeValue.RepresentedType.Type.MakeArrayType()), "Enum.GetValues");
}
}
else
Expand Down Expand Up @@ -381,7 +381,7 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet
&& systemTypeValue.RepresentedType.Type.GetParameterlessConstructor() is MethodDesc ctorMethod
&& !reflectionMarker.Factory.MetadataManager.IsReflectionBlocked(ctorMethod))
{
reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectableMethod(ctorMethod), "Marshal API");
reflectionMarker.Dependencies.Add(reflectionMarker.Factory.ReflectedMethod(ctorMethod), "Marshal API");
}
}
}
Expand Down
Expand Up @@ -128,8 +128,8 @@ private static void AddDependenciesDueToCustomAttributes(ref DependencyList depe
// Make a new list in case we need to abort.
var caDependencies = factory.MetadataManager.GetDependenciesForCustomAttribute(factory, constructor, decodedValue, parent) ?? new DependencyList();

caDependencies.Add(factory.ReflectableMethod(constructor), "Attribute constructor");
caDependencies.Add(factory.ConstructedTypeSymbol(constructor.OwningType), "Attribute type");
caDependencies.Add(factory.ReflectedMethod(constructor), "Attribute constructor");
caDependencies.Add(factory.ReflectedType(constructor.OwningType), "Attribute type");

if (AddDependenciesFromCustomAttributeBlob(caDependencies, factory, constructor.OwningType, decodedValue))
{
Expand Down Expand Up @@ -193,7 +193,7 @@ private static bool AddDependenciesFromField(DependencyList dependencies, NodeFa
if (factory.MetadataManager.IsReflectionBlocked(field))
return false;

dependencies.Add(factory.ReflectableField(field), "Custom attribute blob");
dependencies.Add(factory.ReflectedField(field), "Custom attribute blob");

return true;
}
Expand Down Expand Up @@ -234,7 +234,7 @@ private static bool AddDependenciesFromPropertySetter(DependencyList dependencie
setterMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(setterMethod, (InstantiatedType)attributeType);
}

dependencies.Add(factory.ReflectableMethod(setterMethod), "Custom attribute blob");
dependencies.Add(factory.ReflectedMethod(setterMethod), "Custom attribute blob");
}

return true;
Expand All @@ -259,7 +259,7 @@ private static bool AddDependenciesFromCustomAttributeArgument(DependencyList de

// Reflection will need to be able to allocate this type at runtime
// (e.g. this could be an array that needs to be allocated, or an enum that needs to be boxed).
dependencies.Add(factory.ConstructedTypeSymbol(type), "Custom attribute blob");
dependencies.Add(factory.ReflectedType(type), "Custom attribute blob");

if (type.UnderlyingType.IsPrimitive || type.IsString || value == null)
return true;
Expand Down
Expand Up @@ -14,6 +14,9 @@ namespace ILCompiler.DependencyAnalysis
{
/// <summary>
/// Represents a field that has metadata generated in the current compilation.
/// This corresponds to a ECMA-335 FieldDef record. It is however not a 1:1
/// mapping because a field could be used in the AOT compiled program without generating
/// the reflection metadata for it (which would not be possible in IL terms).
/// </summary>
/// <remarks>
/// Only expected to be used during ILScanning when scanning for reflection.
Expand Down
Expand Up @@ -14,6 +14,9 @@ namespace ILCompiler.DependencyAnalysis
{
/// <summary>
/// Represents a method that has metadata generated in the current compilation.
/// This corresponds to a ECMA-335 MethodDef record. It is however not a 1:1
/// mapping because a method could be used in the AOT compiled program without generating
/// the reflection metadata for it (which would not be possible in IL terms).
/// </summary>
/// <remarks>
/// Only expected to be used during ILScanning when scanning for reflection.
Expand Down
Expand Up @@ -41,7 +41,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
&& ecmaModule.EntryPoint is MethodDesc entrypoint
&& !factory.MetadataManager.IsReflectionBlocked(entrypoint))
{
dependencies.Add(factory.ReflectableMethod(entrypoint), "Reflectable entrypoint");
dependencies.Add(factory.ReflectedMethod(entrypoint), "Reflectable entrypoint");
}

CustomAttributeBasedDependencyAlgorithm.AddDependenciesDueToCustomAttributes(ref dependencies, factory, (EcmaAssembly)_module);
Expand Down
Expand Up @@ -277,14 +277,19 @@ private void CreateNodeCaches()
return new TypeGVMEntriesNode(type);
});

_reflectableMethods = new NodeCache<MethodDesc, ReflectableMethodNode>(method =>
_reflectedMethods = new NodeCache<MethodDesc, ReflectedMethodNode>(method =>
{
return new ReflectableMethodNode(method);
return new ReflectedMethodNode(method);
});

_reflectableFields = new NodeCache<FieldDesc, ReflectableFieldNode>(field =>
_reflectedFields = new NodeCache<FieldDesc, ReflectedFieldNode>(field =>
{
return new ReflectableFieldNode(field);
return new ReflectedFieldNode(field);
});

_reflectedTypes = new NodeCache<TypeDesc, ReflectedTypeNode>(type =>
{
return new ReflectedTypeNode(type);
});

_genericStaticBaseInfos = new NodeCache<MetadataType, GenericStaticBaseInfoNode>(type =>
Expand Down Expand Up @@ -881,16 +886,22 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type)
return _gvmTableEntries.GetOrAdd(type);
}

private NodeCache<MethodDesc, ReflectableMethodNode> _reflectableMethods;
public ReflectableMethodNode ReflectableMethod(MethodDesc method)
private NodeCache<MethodDesc, ReflectedMethodNode> _reflectedMethods;
public ReflectedMethodNode ReflectedMethod(MethodDesc method)
{
return _reflectedMethods.GetOrAdd(method);
}

private NodeCache<FieldDesc, ReflectedFieldNode> _reflectedFields;
public ReflectedFieldNode ReflectedField(FieldDesc field)
{
return _reflectableMethods.GetOrAdd(method);
return _reflectedFields.GetOrAdd(field);
}

private NodeCache<FieldDesc, ReflectableFieldNode> _reflectableFields;
public ReflectableFieldNode ReflectableField(FieldDesc field)
private NodeCache<TypeDesc, ReflectedTypeNode> _reflectedTypes;
public ReflectedTypeNode ReflectedType(TypeDesc type)
{
return _reflectableFields.GetOrAdd(field);
return _reflectedTypes.GetOrAdd(type);
}

private NodeCache<MetadataType, GenericStaticBaseInfoNode> _genericStaticBaseInfos;
Expand Down
Expand Up @@ -13,12 +13,15 @@ namespace ILCompiler.DependencyAnalysis
{
/// <summary>
/// Represents a field that is gettable/settable from reflection.
/// The field can be on a non-generic type, generic type definition, or an instantiatied type.
/// To match IL semantics, we maintain that a field on a generic type will be consistently
/// reflection-accessible. Either the field is accessible on all instantiations or on none of them.
/// </summary>
public class ReflectableFieldNode : DependencyNodeCore<NodeFactory>
public class ReflectedFieldNode : DependencyNodeCore<NodeFactory>
{
private readonly FieldDesc _field;

public ReflectableFieldNode(FieldDesc field)
public ReflectedFieldNode(FieldDesc field)
{
Debug.Assert(!field.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any)
|| field.OwningType.ConvertToCanonForm(CanonicalFormKind.Specific) == field.OwningType);
Expand Down Expand Up @@ -46,7 +49,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
// Ensure we consistently apply reflectability to all fields sharing the same definition.
// Bases for different instantiations of the field have a conditional dependency on the definition node that
// brings a ReflectableField of the instantiated field if it's necessary for it to be reflectable.
dependencies.Add(factory.ReflectableField(typicalField), "Definition of the reflectable field");
dependencies.Add(factory.ReflectedField(typicalField), "Definition of the reflectable field");
}

// Runtime reflection stack needs to see the type handle of the owning type
Expand Down
Expand Up @@ -13,12 +13,16 @@ namespace ILCompiler.DependencyAnalysis
{
/// <summary>
/// Represents a method that is visible to reflection.
/// The method can be on a non-generic type, generic type definition, or an instantiatied type.
/// To match IL semantics, we maintain that a method on a generic type will be consistently
/// reflection-accessible. Either the method is accessible on all instantiations or on none of them.
/// Similar invariants hold for generic methods.
/// </summary>
public class ReflectableMethodNode : DependencyNodeCore<NodeFactory>
public class ReflectedMethodNode : DependencyNodeCore<NodeFactory>
{
private readonly MethodDesc _method;

public ReflectableMethodNode(MethodDesc method)
public ReflectedMethodNode(MethodDesc method)
{
Debug.Assert(!method.IsCanonicalMethod(CanonicalFormKind.Any) ||
method.GetCanonMethodTarget(CanonicalFormKind.Specific) == method);
Expand Down Expand Up @@ -46,13 +50,13 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
MethodDesc typicalMethod = _method.GetTypicalMethodDefinition();
if (typicalMethod != _method)
{
dependencies.Add(factory.ReflectableMethod(typicalMethod), "Definition of the reflectable method");
dependencies.Add(factory.ReflectedMethod(typicalMethod), "Definition of the reflectable method");
}

MethodDesc canonMethod = _method.GetCanonMethodTarget(CanonicalFormKind.Specific);
if (canonMethod != _method)
{
dependencies.Add(factory.ReflectableMethod(canonMethod), "Canonical version of the reflectable method");
dependencies.Add(factory.ReflectedMethod(canonMethod), "Canonical version of the reflectable method");
}

// Make sure we generate the method body and other artifacts.
Expand Down
@@ -0,0 +1,54 @@
// 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
{
/// <summary>
/// Represents a type that is forced to be visible from reflection.
/// The system needs to implicitly assume that any allocated type could be visible from
/// reflection due to <see cref="object.GetType" />, so presence of this node is not
/// a necessary condition for a type to be reflection visible. However, the presence of this
/// node indicates that a new reflectable type was forced into existence by e.g. dataflow
/// analysis, and is not just a byproduct of allocating an instance of this type.
/// </summary>
public class ReflectedTypeNode : DependencyNodeCore<NodeFactory>
{
private readonly TypeDesc _type;

public ReflectedTypeNode(TypeDesc type)
{
Debug.Assert(!type.IsCanonicalSubtype(CanonicalFormKind.Any)
|| type.ConvertToCanonForm(CanonicalFormKind.Specific) == type);
_type = type;
}

public TypeDesc Type => _type;

public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory)
{
return new DependencyListEntry[]
{
new DependencyListEntry(factory.MaximallyConstructableType(_type), "Reflection target"),
};
}
protected override string GetName(NodeFactory factory)
{
return "Reflectable type: " + _type.ToString();
}

public override bool InterestingForDynamicDependencyAnalysis => false;
public override bool HasDynamicDependencies => false;
public override bool HasConditionalStaticDependencies => false;
public override bool StaticDependenciesAreComputed => true;
public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => null;
public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) => null;
}
}
Expand Up @@ -14,6 +14,9 @@ namespace ILCompiler.DependencyAnalysis
{
/// <summary>
/// Represents a type that has metadata generated in the current compilation.
/// This node corresponds to an ECMA-335 TypeDef record. It is however not a 1:1
/// mapping because IL could be compiled into machine code without generating a record
/// in the reflection metadata (which would not be possible in IL terms).
/// </summary>
/// <remarks>
/// Only expected to be used during ILScanning when scanning for reflection.
Expand Down Expand Up @@ -48,7 +51,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
{
// A lot of the enum reflection actually happens on top of the respective MethodTable (e.g. getting the underlying type),
// so for enums also include their MethodTable.
dependencies.Add(factory.MaximallyConstructableType(_type), "Reflectable enum");
dependencies.Add(factory.ReflectedType(_type), "Reflectable enum");

// Enums are not useful without their literal fields. The literal fields are not referenced
// from anywhere (source code reference to enums compiles to the underlying numerical constants in IL).
Expand Down
Expand Up @@ -213,7 +213,7 @@ protected override void ProcessField(TypeDesc type, FieldDesc field, XPathNaviga
// LogWarning(nav, DiagnosticId.XmlDuplicatePreserveMember, field.FullName);
}*/

_dependencies.Add(_factory.ReflectableField(field), "field kept due to descriptor");
_dependencies.Add(_factory.ReflectedField(field), "field kept due to descriptor");
}

protected override void ProcessMethod(TypeDesc type, MethodDesc method, XPathNavigator nav, object? customData)
Expand All @@ -228,11 +228,11 @@ protected override void ProcessMethod(TypeDesc type, MethodDesc method, XPathNav
if (customData is bool required && !required)
{
//TODO: Add a conditional dependency if the type is used also mark the method
_dependencies.Add(_factory.ReflectableMethod(method), "method kept due to descriptor");
_dependencies.Add(_factory.ReflectedMethod(method), "method kept due to descriptor");
}
else
{
_dependencies.Add(_factory.ReflectableMethod(method), "method kept due to descriptor");
_dependencies.Add(_factory.ReflectedMethod(method), "method kept due to descriptor");
}
}

Expand Down