Skip to content

Commit

Permalink
[release/8.0] Revert "Emit less metadata for not-reflection-visible t…
Browse files Browse the repository at this point in the history 
…ypes (#91660)" (#91989)

* Revert "Emit less metadata for not-reflection-visible types (#91660)"

This reverts commit 0bfc0c9.

* Regression test

---------

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>
  • Loading branch information
@github-actions @MichalStrehovsky @carlossanlop
3 people authored Sep 13, 2023
1 parent f490d66 commit a303a07
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 100 deletions.
3 changes: 1 addition & 2 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,7 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact

// Ask the metadata manager
// if we have any dependencies due to presence of the EEType.
bool isFullType = factory.MaximallyConstructableType(_type) == this;
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencies, factory, _type, isFullType);
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencies, factory, _type);

if (_type is MetadataType mdType)
ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencies, factory, mdType.Module);
Expand Down
2 changes: 1 addition & 1 deletion .../tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
DependencyList dependencyList = null;

// Ask the metadata manager if we have any dependencies due to the presence of the EEType.
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencyList, factory, _type, isFullType: true);
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencyList, factory, _type);

return dependencyList;
}
Expand Down
17 changes: 1 addition & 16 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,12 +458,7 @@ private void CreateNodeCaches()

_typesWithMetadata = new NodeCache<MetadataType, TypeMetadataNode>(type =>
{
return new TypeMetadataNode(type, includeCustomAttributes: true);
});

_typesWithMetadataWithoutCustomAttributes = new NodeCache<MetadataType, TypeMetadataNode>(type =>
{
return new TypeMetadataNode(type, includeCustomAttributes: false);
return new TypeMetadataNode(type);
});

_methodsWithMetadata = new NodeCache<MethodDesc, MethodMetadataNode>(method =>
Expand Down Expand Up @@ -1161,16 +1156,6 @@ internal TypeMetadataNode TypeMetadata(MetadataType type)
return _typesWithMetadata.GetOrAdd(type);
}

private NodeCache<MetadataType, TypeMetadataNode> _typesWithMetadataWithoutCustomAttributes;

internal TypeMetadataNode TypeMetadataWithoutCustomAttributes(MetadataType type)
{
// These are only meaningful for UsageBasedMetadataManager. We should not have them
// in the dependency graph otherwise.
Debug.Assert(MetadataManager is UsageBasedMetadataManager);
return _typesWithMetadataWithoutCustomAttributes.GetOrAdd(type);
}

private NodeCache<MethodDesc, MethodMetadataNode> _methodsWithMetadata;

internal MethodMetadataNode MethodMetadata(MethodDesc method)
Expand Down
49 changes: 22 additions & 27 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeMetadataNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@ namespace ILCompiler.DependencyAnalysis
internal sealed class TypeMetadataNode : DependencyNodeCore<NodeFactory>
{
private readonly MetadataType _type;
private readonly bool _includeCustomAttributes;

public TypeMetadataNode(MetadataType type, bool includeCustomAttributes)
public TypeMetadataNode(MetadataType type)
{
Debug.Assert(type.IsTypeDefinition);
_type = type;
_includeCustomAttributes = includeCustomAttributes;
}

public MetadataType Type => _type;
Expand All @@ -39,21 +37,13 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
{
DependencyList dependencies = new DependencyList();

if (_includeCustomAttributes)
CustomAttributeBasedDependencyAlgorithm.AddDependenciesDueToCustomAttributes(ref dependencies, factory, ((EcmaType)_type));
CustomAttributeBasedDependencyAlgorithm.AddDependenciesDueToCustomAttributes(ref dependencies, factory, ((EcmaType)_type));

DefType containingType = _type.ContainingType;
if (containingType != null)
{
TypeMetadataNode metadataNode = _includeCustomAttributes
? factory.TypeMetadata((MetadataType)containingType)
: factory.TypeMetadataWithoutCustomAttributes((MetadataType)containingType);
dependencies.Add(metadataNode, "Containing type of a reflectable type");
}
dependencies.Add(factory.TypeMetadata((MetadataType)containingType), "Containing type of a reflectable type");
else
{
dependencies.Add(factory.ModuleMetadata(_type.Module), "Containing module of a reflectable type");
}

var mdManager = (UsageBasedMetadataManager)factory.MetadataManager;

Expand Down Expand Up @@ -110,7 +100,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
/// Decomposes a constructed type into individual <see cref="TypeMetadataNode"/> units that will be needed to
/// express the constructed type in metadata.
/// </summary>
public static void GetMetadataDependencies(ref DependencyList dependencies, NodeFactory nodeFactory, TypeDesc type, string reason, bool isFullType = true)
public static void GetMetadataDependencies(ref DependencyList dependencies, NodeFactory nodeFactory, TypeDesc type, string reason)
{
MetadataManager mdManager = nodeFactory.MetadataManager;

Expand All @@ -120,13 +110,13 @@ public static void GetMetadataDependencies(ref DependencyList dependencies, Node
case TypeFlags.SzArray:
case TypeFlags.ByRef:
case TypeFlags.Pointer:
GetMetadataDependencies(ref dependencies, nodeFactory, ((ParameterizedType)type).ParameterType, reason, isFullType);
GetMetadataDependencies(ref dependencies, nodeFactory, ((ParameterizedType)type).ParameterType, reason);
break;
case TypeFlags.FunctionPointer:
var pointerType = (FunctionPointerType)type;
GetMetadataDependencies(ref dependencies, nodeFactory, pointerType.Signature.ReturnType, reason, isFullType);
GetMetadataDependencies(ref dependencies, nodeFactory, pointerType.Signature.ReturnType, reason);
foreach (TypeDesc paramType in pointerType.Signature)
GetMetadataDependencies(ref dependencies, nodeFactory, paramType, reason, isFullType);
GetMetadataDependencies(ref dependencies, nodeFactory, paramType, reason);
break;

case TypeFlags.SignatureMethodVariable:
Expand All @@ -136,30 +126,35 @@ public static void GetMetadataDependencies(ref DependencyList dependencies, Node
default:
Debug.Assert(type.IsDefType);

var typeDefinition = (MetadataType)type.GetTypeDefinition();
TypeDesc typeDefinition = type.GetTypeDefinition();
if (typeDefinition != type)
{
if (mdManager.CanGenerateMetadata((MetadataType)typeDefinition))
{
dependencies ??= new DependencyList();
dependencies.Add(nodeFactory.TypeMetadata((MetadataType)typeDefinition), reason);
}

foreach (TypeDesc typeArg in type.Instantiation)
{
GetMetadataDependencies(ref dependencies, nodeFactory, typeArg, reason, isFullType);
GetMetadataDependencies(ref dependencies, nodeFactory, typeArg, reason);
}
}

if (mdManager.CanGenerateMetadata(typeDefinition))
else
{
dependencies ??= new DependencyList();
TypeMetadataNode node = isFullType
? nodeFactory.TypeMetadata(typeDefinition)
: nodeFactory.TypeMetadataWithoutCustomAttributes(typeDefinition);
dependencies.Add(node, reason);
if (mdManager.CanGenerateMetadata((MetadataType)type))
{
dependencies ??= new DependencyList();
dependencies.Add(nodeFactory.TypeMetadata((MetadataType)type), reason);
}
}
break;
}
}

protected override string GetName(NodeFactory factory)
{
return $"Reflectable type: {_type}{(!_includeCustomAttributes ? " (No custom attributes)" : "")}";
return "Reflectable type: " + _type.ToString();
}

protected override void OnMarked(NodeFactory factory)
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,13 +478,13 @@ protected virtual void GetMetadataDependenciesDueToReflectability(ref Dependency
/// <summary>
/// This method is an extension point that can provide additional metadata-based dependencies to generated EETypes.
/// </summary>
public virtual void GetDependenciesDueToEETypePresence(ref DependencyList dependencies, NodeFactory factory, TypeDesc type, bool isFullType)
public virtual void GetDependenciesDueToEETypePresence(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
{
MetadataCategory category = GetMetadataCategory(type);

if ((category & MetadataCategory.Description) != 0)
{
GetMetadataDependenciesDueToReflectability(ref dependencies, factory, type, isFullType);
GetMetadataDependenciesDueToReflectability(ref dependencies, factory, type);
}
}

Expand All @@ -493,7 +493,7 @@ internal virtual void GetDependenciesDueToModuleUse(ref DependencyList dependenc
// MetadataManagers can override this to provide additional dependencies caused by using a module
}

protected virtual void GetMetadataDependenciesDueToReflectability(ref DependencyList dependencies, NodeFactory factory, TypeDesc type, bool isFullType)
protected virtual void GetMetadataDependenciesDueToReflectability(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
{
// MetadataManagers can override this to provide additional dependencies caused by the emission of metadata
// (E.g. dependencies caused by the type having custom attributes applied to it: making sure we compile the attribute constructor
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ internal override void GetDependenciesDueToModuleUse(ref DependencyList dependen
}
}

protected override void GetMetadataDependenciesDueToReflectability(ref DependencyList dependencies, NodeFactory factory, TypeDesc type, bool isFullType)
protected override void GetMetadataDependenciesDueToReflectability(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
{
TypeMetadataNode.GetMetadataDependencies(ref dependencies, factory, type, "Reflectable type", isFullType);
TypeMetadataNode.GetMetadataDependencies(ref dependencies, factory, type, "Reflectable type");

if (type.IsDelegate)
{
Expand Down Expand Up @@ -385,9 +385,9 @@ private static bool IsTrimmableAssembly(ModuleDesc assembly)
return false;
}

public override void GetDependenciesDueToEETypePresence(ref DependencyList dependencies, NodeFactory factory, TypeDesc type, bool isFullType)
public override void GetDependenciesDueToEETypePresence(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
{
base.GetDependenciesDueToEETypePresence(ref dependencies, factory, type, isFullType);
base.GetDependenciesDueToEETypePresence(ref dependencies, factory, type);

DataflowAnalyzedTypeDefinitionNode.GetDependencies(ref dependencies, factory, FlowAnnotations, type);
}
Expand Down Expand Up @@ -970,7 +970,7 @@ public bool GeneratesMetadata(MethodDesc methodDef)

public bool GeneratesMetadata(MetadataType typeDef)
{
return _factory.TypeMetadata(typeDef).Marked || _factory.TypeMetadataWithoutCustomAttributes(typeDef).Marked;
return _factory.TypeMetadata(typeDef).Marked;
}

public bool GeneratesMetadata(EcmaModule module, CustomAttributeHandle caHandle)
Expand Down
30 changes: 30 additions & 0 deletions src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ private static int Main()
TestByRefTypeLoad.Run();
TestGenericLdtoken.Run();
TestAbstractGenericLdtoken.Run();
TestTypeHandlesVisibleFromIDynamicInterfaceCastable.Run();

//
// Mostly functionality tests
Expand Down Expand Up @@ -2457,6 +2458,35 @@ public static void Run()
}
}

class TestTypeHandlesVisibleFromIDynamicInterfaceCastable
{
class MyAttribute : Attribute { }

[My]
interface IUnusedInterface { }

class Foo : IDynamicInterfaceCastable
{
public RuntimeTypeHandle GetInterfaceImplementation(RuntimeTypeHandle interfaceType) => throw new NotImplementedException();

public bool IsInterfaceImplemented(RuntimeTypeHandle interfaceType, bool throwIfNotImplemented)
{
Type t = Type.GetTypeFromHandle(interfaceType);
if (t.GetCustomAttribute<MyAttribute>() == null)
throw new Exception();

return true;
}
}

public static void Run()
{
object myObject = new Foo();
if (myObject is not IUnusedInterface)
throw new Exception();
}
}

class TestEntryPoint
{
public static void Run()
Expand Down
46 changes: 0 additions & 46 deletions src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public static int Run()
TestStaticVirtualMethodOptimizations.Run();
TestTypeEquals.Run();
TestBranchesInGenericCodeRemoval.Run();
TestLimitedMetadataBlobs.Run();

return 100;
}
Expand Down Expand Up @@ -379,51 +378,6 @@ public static void Run()
}
}

class TestLimitedMetadataBlobs
{
class MyAttribute : Attribute
{
public MyAttribute([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type t) { }
}

class ShouldNotBeNeeded
{
}

[My(typeof(ShouldNotBeNeeded))]
interface INeverImplemented
{
void Never();
}

static INeverImplemented s_instance;
#if !DEBUG
static Type s_type;
#endif

internal static void Run()
{
Console.WriteLine("Testing generation of limited metadata blobs");

// Force a reference to the interface from a dispatch cell
if (s_instance != null)
s_instance.Never();

// Following is for release only since it relies on optimizing the typeof into an unconstructed
// MethodTable.
#if !DEBUG
// Force another reference from an LDTOKEN
if (s_type == typeof(INeverImplemented))
s_type = typeof(object);
#endif

ThrowIfPresent(typeof(TestLimitedMetadataBlobs), nameof(ShouldNotBeNeeded));
ThrowIfPresent(typeof(TestLimitedMetadataBlobs), nameof(MyAttribute));
ThrowIfNotPresent(typeof(TestLimitedMetadataBlobs), nameof(INeverImplemented));
}
}


[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
Justification = "That's the point")]
private static Type GetTypeSecretly(Type testType, string typeName) => testType.GetNestedType(typeName, BindingFlags.NonPublic | BindingFlags.Public);
Expand Down

0 comments on commit a303a07

Please sign in to comment.