Simplify ldftn reverse lookups (#88719)

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).
dotnet · Jul 14, 2023 · d51a8d5 · d51a8d5
1 parent 7531efc
commit d51a8d5
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 60 deletions.
diff --git a/...ion/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs b/...ion/src/Internal/Reflection/Execution/ExecutionEnvironmentImplementation.MappingTables.cs
@@ -566,74 +566,24 @@ 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<NativeFormatModuleInfo, FunctionPointersToOffsets>[] _ldftnReverseLookup_InvokeMap;
-        private Func<NativeFormatModuleInfo, FunctionPointersToOffsets> _computeLdFtnLookupInvokeMapInvokeMap = ComputeLdftnReverseLookup_InvokeMap;
 
         /// <summary>
-        /// 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.
         /// </summary>
-        /// <param name="ldftnReverseLookupStatic">pointer to static which holds cache value. This is treated as a volatile variable</param>
-        /// <param name="lookupComputer"></param>
-        /// <returns></returns>
-        private KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>[] GetLdFtnReverseLookups_Helper(ref KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>[] ldftnReverseLookupStatic, Func<NativeFormatModuleInfo, FunctionPointersToOffsets> lookupComputer)
+        private KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>[] GetLdFtnReverseLookups_InvokeMap()
         {
-            KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>[] ldFtnReverseLookup = Volatile.Read(ref ldftnReverseLookupStatic);
-
-            if (ldFtnReverseLookup != null)
-                return ldFtnReverseLookup;
-            else
+            KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>[] ldFtnReverseLookup = _ldftnReverseLookup_InvokeMap;
+            if (ldFtnReverseLookup == null)
             {
-                lock (this)
+                var ldFtnReverseLookupBuilder = new ArrayBuilder<KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>>();
+                foreach (NativeFormatModuleInfo module in ModuleList.EnumerateModules())
                 {
-                    ldFtnReverseLookup = Volatile.Read(ref ldftnReverseLookupStatic);
-
-                    // 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<NativeFormatModuleInfo, FunctionPointersToOffsets>[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<NativeFormatModuleInfo, FunctionPointersToOffsets>(module, lookupComputer(module));
-                            index++;
-                        }
-
-                        if (restart)
-                            continue;
-
-                        // unless we need to repeat the module enumeration, only execute the body of this while loop once.
-                        break;
-                    }
-
-                    Volatile.Write(ref ldftnReverseLookupStatic, ldFtnReverseLookup);
-                    return ldFtnReverseLookup;
+                    ldFtnReverseLookupBuilder.Add(new KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>(module, ComputeLdftnReverseLookup_InvokeMap(module)));
                 }
+                ldFtnReverseLookup = ldFtnReverseLookupBuilder.ToArray();
+                _ldftnReverseLookup_InvokeMap = ldFtnReverseLookup;
             }
-        }
-
-        private KeyValuePair<NativeFormatModuleInfo, FunctionPointersToOffsets>[] 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
+            return ldFtnReverseLookup;
         }
 
         internal unsafe void GetFunctionPointerAndInstantiationArgumentForOriginalLdFtnResult(IntPtr originalLdFtnResult, out IntPtr canonOriginalLdFtnResult, out IntPtr instantiationArgument)

diff --git a/...iveaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj b/...iveaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj
@@ -65,6 +65,9 @@
     <Compile Include="$(AotCommonPath)\System\Collections\Generic\LowLevelDictionary.cs">
       <Link>System\Collections\Generic\LowLevelDictionary.cs</Link>
     </Compile>
+    <Compile Include="$(CompilerCommonPath)\System\Collections\Generic\ArrayBuilder.cs">
+      <Link>System\Collections\Generic\ArrayBuilder.cs</Link>
+    </Compile>
     <Compile Include="$(CompilerCommonPath)\Internal\LowLevelLinq\LowLevelEnumerable.cs">
       <Link>Internal\LowLevelLinq\LowLevelEnumerable.cs</Link>
     </Compile>