From 2619d1c8eeef4a881c3910c87c1a8903ed742c24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 3 Jan 2023 15:29:42 +0900 Subject: [PATCH] Generate fewer `ExactMethodInstantiationsHashtable` entries (#80090) This hashtable contains information about unshared generic methods. We really only need it for generic virtual methods. There was some code in the reflection stack that was reading it to support stack traces and `Delegate.GetMethodInfo` but: 1. For stack traces, we have this information in the stack trace metadata table. 2. For `Delegate.GetMethodInfo` we have a rule that all targets of delegates should become reflectable in the compiler. We should have all this data in the reflection invoke table that we also parse for this purpose. --- ...EnvironmentImplementation.MappingTables.cs | 155 ------------------ .../ExactMethodInstantiationsNode.cs | 4 - .../SmokeTests/Reflection/Reflection.cs | 53 ++++++ 3 files changed, 53 insertions(+), 159 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs index 2dc6b35b5d650..0944c4379352c 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs @@ -615,9 +615,7 @@ public bool TryGetOffsetsRange(IntPtr functionPointer, out int firstParserOffset // ldftn reverse lookup hash. Must be cleared and reset if the module list changes. (All sets to // this variable must happen under a lock) private volatile KeyValuePair[] _ldftnReverseLookup_InvokeMap; - private volatile KeyValuePair[] _ldftnReverseLookup_ExactInstantiations; private Func _computeLdFtnLookupInvokeMapInvokeMap = ComputeLdftnReverseLookup_InvokeMap; - private Func _computeLdFtnLookupExactInstantiations = ComputeLdftnReverseLookup_ExactInstantiations; /// /// Initialize a lookup array of module to function pointer/parser offset pair arrays. Do so in a manner that will allow @@ -687,13 +685,6 @@ private KeyValuePair[] GetLdF #pragma warning restore 0420 } - private KeyValuePair[] GetLdFtnReverseLookups_ExactInstantiations() - { -#pragma warning disable 0420 // GetLdFtnReverseLookups_Helper treats its first parameter as volatile by using explicit Volatile operations - return GetLdFtnReverseLookups_Helper(ref _ldftnReverseLookup_ExactInstantiations, _computeLdFtnLookupExactInstantiations); -#pragma warning restore 0420 - } - internal unsafe void GetFunctionPointerAndInstantiationArgumentForOriginalLdFtnResult(IntPtr originalLdFtnResult, out IntPtr canonOriginalLdFtnResult, out IntPtr instantiationArgument) { if (FunctionPointerOps.IsGenericMethodPointer(originalLdFtnResult)) @@ -719,26 +710,6 @@ internal bool TryGetMethodForOriginalLdFtnResult(IntPtr originalLdFtnResult, ref if (TryGetMethodForOriginalLdFtnResult_GenericMethodWithInstantiationArgument(instantiationArgument, ref declaringTypeHandle, out methodHandle, out genericMethodTypeArgumentHandles)) return true; } - else - { - // Search ExactInstantiationsMap - foreach (KeyValuePair perModuleLookup in GetLdFtnReverseLookups_ExactInstantiations()) - { - int startIndex; - int endIndex; - - if (perModuleLookup.Value.TryGetOffsetsRange(canonOriginalLdFtnResult, out startIndex, out endIndex)) - { - for (int curIndex = startIndex; curIndex <= endIndex; curIndex++) - { - uint parserOffset = perModuleLookup.Value.Data[curIndex].Offset; - if (TryGetMethodForOriginalLdFtnResult_ExactInstantiation_Inner(perModuleLookup.Key, forStartAddress: false, canonOriginalLdFtnResult, parserOffset, - ref declaringTypeHandle, out methodHandle, out genericMethodTypeArgumentHandles)) - return true; - } - } - } - } // Search InvokeMap foreach (KeyValuePair perModuleLookup in GetLdFtnReverseLookups_InvokeMap()) @@ -764,27 +735,6 @@ internal bool TryGetMethodForOriginalLdFtnResult(IntPtr originalLdFtnResult, ref internal bool TryGetMethodForStartAddress(IntPtr methodStartAddress, ref RuntimeTypeHandle declaringTypeHandle, out QMethodDefinition methodHandle) { - // Search ExactInstantiationsMap - foreach (KeyValuePair perModuleLookup in GetLdFtnReverseLookups_ExactInstantiations()) - { - int startIndex; - int endIndex; - - if (perModuleLookup.Value.TryGetOffsetsRange(methodStartAddress, out startIndex, out endIndex)) - { - for (int curIndex = startIndex; curIndex <= endIndex; curIndex++) - { - uint parserOffset = perModuleLookup.Value.Data[curIndex].Offset; - if (TryGetMethodForOriginalLdFtnResult_ExactInstantiation_Inner(perModuleLookup.Key, forStartAddress: true, methodStartAddress, parserOffset, ref declaringTypeHandle, out methodHandle, out _)) - { - if (RuntimeAugments.IsGenericType(declaringTypeHandle)) - declaringTypeHandle = RuntimeAugments.GetGenericDefinition(declaringTypeHandle); - return true; - } - } - } - } - // Search InvokeMap foreach (KeyValuePair perModuleLookup in GetLdFtnReverseLookups_InvokeMap()) { @@ -979,111 +929,6 @@ private unsafe bool TryGetMethodForOriginalLdFtnResult_InvokeMap_Inner(NativeFor return true; } - private static FunctionPointersToOffsets ComputeLdftnReverseLookup_ExactInstantiations(NativeFormatModuleInfo mappingTableModule) - { - FunctionPointersToOffsets functionPointerToOffsetInInvokeMap = new FunctionPointersToOffsets(); - - NativeReader methodTemplateMapReader; - if (!TryGetNativeReaderForBlob(mappingTableModule, ReflectionMapBlob.ExactMethodInstantiationsHashtable, out methodTemplateMapReader)) - { - return functionPointerToOffsetInInvokeMap; - } - - ExternalReferencesTable externalReferences = default(ExternalReferencesTable); - externalReferences.InitializeNativeReferences(mappingTableModule); - - NativeParser methodTemplateMapParser = new NativeParser(methodTemplateMapReader, 0); - NativeHashtable invokeHashtable = new NativeHashtable(methodTemplateMapParser); - - LowLevelList functionPointers = new LowLevelList(); - - var lookup = invokeHashtable.EnumerateAllEntries(); - NativeParser entryParser; - while (!(entryParser = lookup.GetNext()).IsNull) - { - uint parserOffset = entryParser.Offset; - - // Declaring Handle - entryParser.SkipInteger(); - - // NameAndSig - entryParser.SkipInteger(); - - // generic method arity - int parsedArity = (int)entryParser.GetSequenceCount(); - - for (int i = 0; i < parsedArity; i++) - { - entryParser.SkipInteger(); - } - - IntPtr functionPointer = externalReferences.GetIntPtrFromIndex(entryParser.GetUnsigned()); - functionPointers.Add(new FunctionPointerOffsetPair(functionPointer, parserOffset)); - } - - functionPointerToOffsetInInvokeMap.Data = functionPointers.ToArray(); - Array.Sort(functionPointerToOffsetInInvokeMap.Data); - - return functionPointerToOffsetInInvokeMap; - } - - private static unsafe bool TryGetMethodForOriginalLdFtnResult_ExactInstantiation_Inner(NativeFormatModuleInfo mappingTableModule, bool forStartAddress, IntPtr canonOriginalLdFtnResult, uint parserOffset, ref RuntimeTypeHandle declaringTypeHandle, out QMethodDefinition methodHandle, out RuntimeTypeHandle[] genericMethodTypeArgumentHandles) - { - methodHandle = default(QMethodDefinition); - genericMethodTypeArgumentHandles = null; - - NativeReader invokeMapReader; - if (!TryGetNativeReaderForBlob(mappingTableModule, ReflectionMapBlob.ExactMethodInstantiationsHashtable, out invokeMapReader)) - { - // This should have succeeded otherwise, how did we get a parser offset as an input parameter? - Debug.Assert(false); - return false; - } - - ExternalReferencesTable externalReferences = default(ExternalReferencesTable); - externalReferences.InitializeNativeReferences(mappingTableModule); - - NativeParser entryParser = new NativeParser(invokeMapReader, parserOffset); - - RuntimeTypeHandle entryTypeHandle = externalReferences.GetRuntimeTypeHandleFromIndex(entryParser.GetUnsigned()); - - // Hash table names / sigs are indirected through to the native layout info - MethodNameAndSignature nameAndSignature; - if (!TypeLoaderEnvironment.Instance.TryGetMethodNameAndSignatureFromNativeLayoutOffset(mappingTableModule.Handle, entryParser.GetUnsigned(), out nameAndSignature)) - return false; - - int parsedArity = (int)entryParser.GetSequenceCount(); - - if (forStartAddress) - { - for (int i = 0; i < parsedArity; i++) - { - entryParser.SkipInteger(); - } - } - else - { - genericMethodTypeArgumentHandles = new RuntimeTypeHandle[parsedArity]; - - for (int i = 0; i < parsedArity; i++) - { - genericMethodTypeArgumentHandles[i] = externalReferences.GetRuntimeTypeHandleFromIndex(entryParser.GetUnsigned()); - } - } - - IntPtr functionPointer = externalReferences.GetIntPtrFromIndex(entryParser.GetUnsigned()); - if (functionPointer != canonOriginalLdFtnResult) - return false; - - if (TypeLoaderEnvironment.Instance.TryGetMetadataForTypeMethodNameAndSignature(entryTypeHandle, nameAndSignature, out methodHandle)) - { - declaringTypeHandle = entryTypeHandle; - return true; - } - - return false; - } - private static unsafe bool TryGetMethodForOriginalLdFtnResult_GenericMethodWithInstantiationArgument(IntPtr instantiationArgument, ref RuntimeTypeHandle declaringTypeHandle, out QMethodDefinition methodHandle, out RuntimeTypeHandle[] genericMethodTypeArgumentHandles) { MethodNameAndSignature nameAndSig; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs index 46f4eed60a429..3d52e0a3a9c22 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs @@ -148,10 +148,6 @@ private static bool IsMethodEligibleForTracking(NodeFactory factory, MethodDesc if (method.IsVirtual) return true; - // The hashtable is also used for reflection - if (!factory.MetadataManager.IsReflectionBlocked(method)) - return true; - // The rest of the entries are potentially only useful for the universal // canonical type loader. return factory.TypeSystemContext.SupportsUniversalCanon; diff --git a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs index 14a419e8d9b4a..e6fab4c2ecd6b 100644 --- a/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs +++ b/src/tests/nativeaot/SmokeTests/Reflection/Reflection.cs @@ -57,6 +57,7 @@ private static int Main() #if !REFLECTION_FROM_USAGE TestNotReflectedIsNotReflectable.Run(); TestGenericInstantiationsAreEquallyReflectable.Run(); + TestStackTraces.Run(); #endif TestAttributeInheritance2.Run(); TestInvokeMethodMetadata.Run(); @@ -1600,6 +1601,58 @@ public static void Run() throw new Exception(); } } + + class TestStackTraces + { + class RefType { } + struct ValType { } + + class C1 + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static string M1(T c1m1param) => Environment.StackTrace; + [MethodImpl(MethodImplOptions.NoInlining)] + public static string M2(T c1m2param) => Environment.StackTrace; + [MethodImpl(MethodImplOptions.NoInlining)] + public static string M3(T c1m3param1, U c1m3param2) => Environment.StackTrace; + [MethodImpl(MethodImplOptions.NoInlining)] + public static string M4(T c1m4param1, U c1m4param2) => Environment.StackTrace; + } + + public static void Run() + { + // These methods are visible to reflection + typeof(C1).GetMethod(nameof(C1.M1)); + typeof(C1).GetMethod(nameof(C1.M3)); + typeof(C1).GetMethod(nameof(C1.M1)); + typeof(C1).GetMethod(nameof(C1.M3)); + + Check("C1", "M1", "c1m1param", true, C1.M1(default)); + Check("C1", "M1", "c1m1param", true, C1.M1(default)); + Check("C1", "M2", "c1m2param", false, C1.M2(default)); + Check("C1", "M2", "c1m2param", false, C1.M2(default)); + Check("C1", "M3", "c1m3param", true, C1.M3(default, default)); + Check("C1", "M3", "c1m3param", true, C1.M3(default, default)); + Check("C1", "M3", "c1m3param", true, C1.M3(default, default)); + Check("C1", "M3", "c1m3param", true, C1.M3(default, default)); + Check("C1", "M4", "c1m4param", false, C1.M4(default, default)); + Check("C1", "M4", "c1m4param", false, C1.M4(default, default)); + Check("C1", "M4", "c1m4param", false, C1.M4(default, default)); + Check("C1", "M4", "c1m4param", false, C1.M4(default, default)); + + static void Check(string type, string method, string param, bool hasParam, string s) + { + if (!s.Contains(type)) + throw new Exception($"'{s}' doesn't contain '{type}'"); + if (!s.Contains(method)) + throw new Exception($"'{s}' doesn't contain '{method}'"); + if (hasParam && !s.Contains(param)) + throw new Exception($"'{s}' doesn't contain '{param}'"); + if (!hasParam && s.Contains(param)) + throw new Exception($"'{s}' contains '{param}'"); + } + } + } #endif class TypeConstructionTest