ILC: Bottleneck in Compilation.GetMethodIL

[filipnavara]

The method: https://github.com/dotnet/runtime/blob/07d363739f74477b854be3614ee063d6e94bbf90/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs#L78-L85

Profile when compiling large app:

  100%   GetMethodIL  •  28,751 ms  •  ILCompiler.Compilation.GetMethodIL(MethodDesc)
    99.9%   CreateValueAndEnsureValueIsInTable  •  28,745 ms  •  Internal.TypeSystem.LockFreeReaderHashtable`2.CreateValueAndEnsureValueIsInTable(TKey)
      56.9%   AddOrGetExistingInner  •  16,355 ms  •  Internal.TypeSystem.LockFreeReaderHashtable`2.AddOrGetExistingInner(TValue, out Boolean)
      ► 54.9%   Expand  •  15,773 ms  •  Internal.TypeSystem.LockFreeReaderHashtable`2.Expand(TValue[])
      ► 1.74%   TryAddOrGetExisting  •  500 ms  •  Internal.TypeSystem.LockFreeReaderHashtable`2.TryAddOrGetExisting(TValue, out Boolean)
        0.11%   [Unknown]  •  31 ms
        <0.01%   ntoskrnl.exe  •  1.5 ms
    ► 38.7%   CreateValueFromKey  •  11,126 ms  •  ILCompiler.Compilation+ILCache.CreateValueFromKey(MethodDesc)
      4.30%   [Unknown]  •  1,237 ms
      <0.01%   coreclr.dll  •  0.2 ms
    0.02%   [Unknown]  •  5.8 ms

Apparently, we end up more time resizing the cache than actually using it efficiently. The initial size of the LockFreeReaderHashtable is 16 elements and it's currently not configurable. We throw it away here every time it reaches more than 1000 elements. This has additional negative effect of creating memory pressure.

I tried removing the cache entirely which resulted in 18s spent in ILCompiler.Compilation.GetMethodIL generating the values. Obviously, that's not representative for every workload but it shows how poorly the cache performs.

I also tried to replace it with ConcurrentDictionary (which performed extremely poorly because of its implementation of Count property), ConcurrentDictionary + counter (which performed well), and dumb usage of locked Dictionary (simplest to reason about).

Here's a profile for the Dictionary version:

  100%   GetMethodIL  •  14,393 ms  •  ILCompiler.Compilation.GetMethodIL(MethodDesc)
  ► 68.3%   GetMethodIL  •  9,835 ms  •  ILCompiler.Compilation+CombinedILProvider.GetMethodIL(MethodDesc)
  ► 22.2%   coreclr.dll  •  3,192 ms
    5.02%   [Unknown]  •  723 ms
  ► 2.08%   FindValue  •  299 ms  •  System.Collections.Generic.Dictionary`2.FindValue(TKey)
  ► 0.80%   GetMethodIL  •  116 ms  •  ILCompiler.Compilation.GetMethodIL(MethodDesc)
  ► 0.62%   TryInsert  •  89 ms  •  System.Collections.Generic.Dictionary`2.TryInsert(TKey, TValue, InsertionBehavior)
    0.09%   ClearWithReferences  •  13 ms  •  System.SpanHelpers.ClearWithReferences(ref IntPtr, UIntPtr)
    0.02%   Clear  •  3.2 ms  •  System.Array.Clear(Array)
    0.01%   ntoskrnl.exe  •  1.8 ms

The diff:

--- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs
+++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs
@@ -70,18 +70,32 @@ public abstract class Compilation : ICompilation
                 ilProvider = new CombinedILProvider(ilProvider, PInvokeILProvider);
             }

-            _methodILCache = new ILCache(ilProvider);
+            _ilProvider = ilProvider;
+            _methodILCache = new Dictionary<MethodDesc, MethodIL>();
         }

-        private ILCache _methodILCache;
+        private ILProvider _ilProvider;
+        private Dictionary<MethodDesc, MethodIL> _methodILCache;

         public virtual MethodIL GetMethodIL(MethodDesc method)
         {
-            // Flush the cache when it grows too big
-            if (_methodILCache.Count > 1000)
-                _methodILCache = new ILCache(_methodILCache.ILProvider);
+            lock (_methodILCache)
+            {
+                if (_methodILCache.TryGetValue(method, out var methodIL))
+                    return methodIL;
+
+                // Flush the cache when it grows too big
+                if (_methodILCache.Count > 1000)
+                    _methodILCache.Clear();
+            }

-            return _methodILCache.GetOrCreateValue(method).MethodIL;
+            var newMethodIL = _ilProvider.GetMethodIL(method);
+            lock (_methodILCache)
+            {
+                _methodILCache[method] = newMethodIL;
+            }
+
+            return newMethodIL;
         }

         protected abstract void ComputeDependencyNodeDependencies(List<DependencyNodeCore<NodeFactory>> obj);
@@ -528,43 +542,6 @@ CompilationResults ICompilation.Compile(string outputFile, ObjectDumper dumper)
             return new CompilationResults(_dependencyGraph, _nodeFactory);
         }

-        private sealed class ILCache : LockFreeReaderHashtable<MethodDesc, ILCache.MethodILData>
-        {
-            public ILProvider ILProvider { get; }
-
-            public ILCache(ILProvider provider)
-            {
-                ILProvider = provider;
-            }
-
-            protected override int GetKeyHashCode(MethodDesc key)
-            {
-                return key.GetHashCode();
-            }
-            protected override int GetValueHashCode(MethodILData value)
-            {
-                return value.Method.GetHashCode();
-            }
-            protected override bool CompareKeyToValue(MethodDesc key, MethodILData value)
-            {
-                return ReferenceEquals(key, value.Method);
-            }
-            protected override bool CompareValueToValue(MethodILData value1, MethodILData value2)
-            {
-                return ReferenceEquals(value1.Method, value2.Method);
-            }
-            protected override MethodILData CreateValueFromKey(MethodDesc key)
-            {
-                return new MethodILData() { Method = key, MethodIL = ILProvider.GetMethodIL(key) };
-            }
-
-            internal sealed class MethodILData
-            {
-                public MethodDesc Method;
-                public MethodIL MethodIL;
-            }
-        }
-
         private sealed class CombinedILProvider : ILProvider
         {
             private readonly ILProvider _primaryILProvider;

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas See info in area-owners.md if you want to be subscribed.