Skip to content

Commit

Permalink
Restore EETypeNode to the state before we deleted reflection blocking (
Browse files Browse the repository at this point in the history
…#94287)

Contributes to #91704.

When we deleted reflection blocking in #85810 we had to update `EETypeNode`/`ConstructedEETypeNode` to ensure any `MethodTable` also has metadata (constructed or not). This was needed because of how reflection was structured - we couldn't create a `RuntimeType` without knowing about the metadata. After the refactor in #93440, metadata is no longer a prerequisite to constructing a `RuntimeType`.

The compiler can go back to optimizing away the metadata. This affects things like `typeof(Foo) == bar.GetType()`. No metadata is needed for this (and we do optimize the `Foo` MethodTable to unconstructed one) but we still had to generate metadata.

Besides the rollback of EEType to the previous shape, this also has a bugfix for #91988 that was found later - interface types used in cast/dispatch should be considered constructed.

I'm seeing 0.1 - 0.7% size saving.
  • Loading branch information
MichalStrehovsky authored Nov 6, 2023
1 parent be45599 commit 1ddb513
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,16 @@ public ISymbolNode ComputeConstantLookup(ReadyToRunHelperId lookupKind, object t
case ReadyToRunHelperId.TypeHandleForCasting:
{
var type = (TypeDesc)targetOfLookup;

// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable.
// If this cast happens with an object that implements IDynamicIntefaceCastable, user code will
// see a RuntimeTypeHandle representing this interface.
if (type.IsInterface)
return NodeFactory.MaximallyConstructableType(type);

if (type.IsNullable)
targetOfLookup = type.Instantiation[0];
return NecessaryTypeSymbolIfPossible((TypeDesc)targetOfLookup);
type = type.Instantiation[0];
return NecessaryTypeSymbolIfPossible(type);
}
case ReadyToRunHelperId.MethodDictionary:
return NodeFactory.MethodGenericDictionary((MethodDesc)targetOfLookup);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
// relocs to nodes we emit.
dependencyList.Add(factory.NecessaryTypeSymbol(_type), "NecessaryType for constructed type");

if (_type is MetadataType mdType)
ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencyList, factory, mdType.Module);

DefType closestDefType = _type.GetClosestDefType();

if (_type.IsArray)
Expand Down Expand Up @@ -66,6 +69,9 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
}
}

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

factory.InteropStubManager.AddInterestingInteropConstructedTypeDependencies(ref dependencyList, factory, _type);

return dependencyList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -639,12 +639,16 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
}
}

// Ask the metadata manager
// if we have any dependencies due to presence of the EEType.
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencies, factory, _type);
if (!ConstructedEETypeNode.CreationAllowed(_type))
{
// If necessary MethodTable is the highest load level for this type, ask the metadata manager
// if we have any dependencies due to presence of the EEType.
factory.MetadataManager.GetDependenciesDueToEETypePresence(ref dependencies, factory, _type);

if (_type is MetadataType mdType)
ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencies, factory, mdType.Module);
// If necessary MethodTable is the highest load level, consider this a module use
if (_type is MetadataType mdType)
ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencies, factory, mdType.Module);
}

if (_type.IsFunctionPointer)
FunctionPointerMapNode.GetHashtableDependencies(ref dependencies, factory, (FunctionPointerType)_type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
result.Add(factory.ExternSymbol("RhpInitialDynamicInterfaceDispatch"), "Initial interface dispatch stub");
}

result.Add(factory.NecessaryTypeSymbol(_targetMethod.OwningType), "Interface type");
// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable.
// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will
// see a RuntimeTypeHandle representing this interface.
result.Add(factory.ConstructedTypeSymbol(_targetMethod.OwningType), "Interface type");

return result;
}
Expand All @@ -88,7 +91,10 @@ public override void EncodeData(ref ObjectDataBuilder objData, NodeFactory facto
objData.EmitPointerReloc(factory.ExternSymbol("RhpInitialDynamicInterfaceDispatch"));
}

IEETypeNode interfaceType = factory.NecessaryTypeSymbol(_targetMethod.OwningType);
// We counter-intuitively ask for a constructed type symbol. This is needed due to IDynamicInterfaceCastable.
// If this dispatch cell is ever used with an object that implements IDynamicIntefaceCastable, user code will
// see a RuntimeTypeHandle representing this interface.
IEETypeNode interfaceType = factory.ConstructedTypeSymbol(_targetMethod.OwningType);
if (factory.Target.SupportsRelativePointers)
{
if (interfaceType.RepresentsIndirectionCell)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,11 +660,20 @@ public override IEnumerable<ModuleDesc> GetCompilationModulesWithMetadata()
private IEnumerable<TypeDesc> GetTypesWithRuntimeMapping()
{
// All constructed types that are not blocked get runtime mapping
foreach (var constructedType in GetTypesWithEETypes())
foreach (var constructedType in GetTypesWithConstructedEETypes())
{
if (!IsReflectionBlocked(constructedType))
yield return constructedType;
}

// All necessary types for which this is the highest load level that are not blocked
// get runtime mapping.
foreach (var necessaryType in GetTypesWithEETypes())
{
if (!ConstructedEETypeNode.CreationAllowed(necessaryType) &&
!IsReflectionBlocked(necessaryType))
yield return necessaryType;
}
}

public override void GetDependenciesDueToAccess(ref DependencyList dependencies, NodeFactory factory, MethodIL methodIL, FieldDesc writtenField)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ public static void Run()
Console.WriteLine(s_type == typeof(Never));

#if !DEBUG
ThrowIfPresentWithUsableMethodTable(typeof(TestTypeEquals), nameof(Never));
ThrowIfPresent(typeof(TestTypeEquals), nameof(Never));
#endif
}
}
Expand Down Expand Up @@ -398,7 +398,7 @@ public static void Run()

// We only expect to be able to get rid of it when optimizing
#if !DEBUG
ThrowIfPresentWithUsableMethodTable(typeof(TestBranchesInGenericCodeRemoval), nameof(Unused));
ThrowIfPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(Unused));
#endif
ThrowIfNotPresent(typeof(TestBranchesInGenericCodeRemoval), nameof(Used));

Expand Down

0 comments on commit 1ddb513

Please sign in to comment.