From 2936ede0b90573891002385d08a69e543c4b9997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Wed, 12 Jul 2023 16:09:29 +0900 Subject: [PATCH 1/5] Simplify ldftn reverse lookups This used a delegate because in the past we had multiple possible callbacks. Now the delegate callback is unnecessary. Saves 12 kB on Hello World with stack traces disabled (the delegate was the only thing keeping the `GetLdFtnReverseLookups_InvokeMap` method alive). --- ...EnvironmentImplementation.MappingTables.cs | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 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 35d9cde9222fa..51e301d49abd1 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 @@ -566,18 +566,14 @@ 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 Func _computeLdFtnLookupInvokeMapInvokeMap = ComputeLdftnReverseLookup_InvokeMap; /// /// Initialize a lookup array of module to function pointer/parser offset pair arrays. Do so in a manner that will allow /// future work which will invalidate the cache (by setting it to null) /// - /// pointer to static which holds cache value. This is treated as a volatile variable - /// - /// - private KeyValuePair[] GetLdFtnReverseLookups_Helper(ref KeyValuePair[] ldftnReverseLookupStatic, Func lookupComputer) + private KeyValuePair[] GetLdFtnReverseLookups_InvokeMap() { - KeyValuePair[] ldFtnReverseLookup = Volatile.Read(ref ldftnReverseLookupStatic); + KeyValuePair[] ldFtnReverseLookup = _ldftnReverseLookup_InvokeMap; if (ldFtnReverseLookup != null) return ldFtnReverseLookup; @@ -585,7 +581,7 @@ private KeyValuePair[] GetLdF { lock (this) { - ldFtnReverseLookup = Volatile.Read(ref ldftnReverseLookupStatic); + ldFtnReverseLookup = _ldftnReverseLookup_InvokeMap; // double checked lock, safe due to use of volatile on s_ldftnReverseHashes if (ldFtnReverseLookup != null) @@ -612,7 +608,7 @@ private KeyValuePair[] GetLdF break; } - ldFtnReverseLookup[index] = new KeyValuePair(module, lookupComputer(module)); + ldFtnReverseLookup[index] = new KeyValuePair(module, ComputeLdftnReverseLookup_InvokeMap(module)); index++; } @@ -623,19 +619,12 @@ private KeyValuePair[] GetLdF break; } - Volatile.Write(ref ldftnReverseLookupStatic, ldFtnReverseLookup); + _ldftnReverseLookup_InvokeMap = ldFtnReverseLookup; return ldFtnReverseLookup; } } } - private KeyValuePair[] GetLdFtnReverseLookups_InvokeMap() - { -#pragma warning disable 0420 // GetLdFtnReverseLookups_Helper treats its first parameter as volatile by using explicit Volatile operations - return GetLdFtnReverseLookups_Helper(ref _ldftnReverseLookup_InvokeMap, _computeLdFtnLookupInvokeMapInvokeMap); -#pragma warning restore 0420 - } - internal unsafe void GetFunctionPointerAndInstantiationArgumentForOriginalLdFtnResult(IntPtr originalLdFtnResult, out IntPtr canonOriginalLdFtnResult, out IntPtr instantiationArgument) { if (FunctionPointerOps.IsGenericMethodPointer(originalLdFtnResult)) From 37f6474f861ce74fbabf5c77f5994c83881414d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 13 Jul 2023 13:48:53 +0900 Subject: [PATCH 2/5] PR feedback --- ...EnvironmentImplementation.MappingTables.cs | 55 +++---------------- 1 file changed, 8 insertions(+), 47 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 51e301d49abd1..93146f66ed83d 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 @@ -568,61 +568,22 @@ public bool TryGetOffsetsRange(IntPtr functionPointer, out int firstParserOffset private volatile KeyValuePair[] _ldftnReverseLookup_InvokeMap; /// - /// Initialize a lookup array of module to function pointer/parser offset pair arrays. Do so in a manner that will allow - /// future work which will invalidate the cache (by setting it to null) + /// Initialize a lookup array of module to function pointer/parser offset pair arrays. /// private KeyValuePair[] GetLdFtnReverseLookups_InvokeMap() { KeyValuePair[] ldFtnReverseLookup = _ldftnReverseLookup_InvokeMap; - - if (ldFtnReverseLookup != null) - return ldFtnReverseLookup; - else + if (ldFtnReverseLookup == null) { - lock (this) + ldFtnReverseLookupBuilder = new ArrayBuilder>(); + foreach (NativeFormatModuleInfo module in ModuleList.EnumerateModules()) { - ldFtnReverseLookup = _ldftnReverseLookup_InvokeMap; - - // double checked lock, safe due to use of volatile on s_ldftnReverseHashes - if (ldFtnReverseLookup != null) - return ldFtnReverseLookup; - - // FUTURE: add a module load callback to invalidate this cache if a new module is loaded. - while (true) - { - int size = 0; - foreach (NativeFormatModuleInfo module in ModuleList.EnumerateModules()) - { - size++; - } - - ldFtnReverseLookup = new KeyValuePair[size]; - int index = 0; - bool restart = false; - foreach (NativeFormatModuleInfo module in ModuleList.EnumerateModules()) - { - // If the module list changes during execution of this code, rebuild from scratch - if (index >= ldFtnReverseLookup.Length) - { - restart = true; - break; - } - - ldFtnReverseLookup[index] = new KeyValuePair(module, ComputeLdftnReverseLookup_InvokeMap(module)); - index++; - } - - if (restart) - continue; - - // unless we need to repeat the module enumeration, only execute the body of this while loop once. - break; - } - - _ldftnReverseLookup_InvokeMap = ldFtnReverseLookup; - return ldFtnReverseLookup; + ldFtnReverseLookupBuilder.Add(new KeyValuePair(module, ComputeLdftnReverseLookup_InvokeMap(module))); } + ldFtnReverseLookup = ldFtnReverseLookupBuilder.ToArray(); + _ldftnReverseLookup_InvokeMap = ldFtnReverseLookup; } + return ldFtnReverseLookup; } internal unsafe void GetFunctionPointerAndInstantiationArgumentForOriginalLdFtnResult(IntPtr originalLdFtnResult, out IntPtr canonOriginalLdFtnResult, out IntPtr instantiationArgument) From 27fed90312b1ca81a933e398c9ef3e6fe3bb9fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 13 Jul 2023 15:05:10 +0900 Subject: [PATCH 3/5] Update ExecutionEnvironmentImplementation.MappingTables.cs --- .../ExecutionEnvironmentImplementation.MappingTables.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 93146f66ed83d..7e3209b56e8f7 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 @@ -575,7 +575,7 @@ private KeyValuePair[] GetLdF KeyValuePair[] ldFtnReverseLookup = _ldftnReverseLookup_InvokeMap; if (ldFtnReverseLookup == null) { - ldFtnReverseLookupBuilder = new ArrayBuilder>(); + var ldFtnReverseLookupBuilder = new ArrayBuilder>(); foreach (NativeFormatModuleInfo module in ModuleList.EnumerateModules()) { ldFtnReverseLookupBuilder.Add(new KeyValuePair(module, ComputeLdftnReverseLookup_InvokeMap(module))); From 77a3811ff1d4ccaff969971c1ccebf889afd0829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 13 Jul 2023 16:28:03 +0900 Subject: [PATCH 4/5] Update System.Private.Reflection.Execution.csproj --- .../src/System.Private.Reflection.Execution.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj index 76d3934794fd2..e862d3e243262 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj @@ -65,6 +65,9 @@ System\Collections\Generic\LowLevelDictionary.cs + + ArrayBuilder.cs + Internal\LowLevelLinq\LowLevelEnumerable.cs From 20a9b2e06f553565d38985dc710aff5e3e4816e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 13 Jul 2023 16:28:47 +0900 Subject: [PATCH 5/5] Update System.Private.Reflection.Execution.csproj --- .../src/System.Private.Reflection.Execution.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj index e862d3e243262..5527d5920f601 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj @@ -66,7 +66,7 @@ System\Collections\Generic\LowLevelDictionary.cs - ArrayBuilder.cs + System\Collections\Generic\ArrayBuilder.cs Internal\LowLevelLinq\LowLevelEnumerable.cs