Merge pull request #1491 from autofac/feature/issue-1461

Cache compiled factory-delegate generators to avoid recompilation across scopes (#1461)

Author: tillig

bench/Autofac.Benchmarks/BenchmarkSet.cs

@@ -34,5 +34,6 @@ public static class BenchmarkSet
         typeof(LambdaResolveBenchmark),
         typeof(RequiredPropertyBenchmark),
         typeof(ModuleRegistrationBenchmark),
+        typeof(GeneratedFactoryChildScopeBenchmark),
     };
 }

bench/Autofac.Benchmarks/GeneratedFactoryChildScopeBenchmark.cs

@@ -0,0 +1,71 @@
+// Copyright (c) Autofac Project. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+
+namespace Autofac.Benchmarks;
+
+/// <summary>
+/// Measures the cost of resolving a component registered in a child lifetime scope that
+/// depends on <c>Func&lt;T&gt;</c>, where <c>T</c> is registered in the root container.
+/// </summary>
+/// <remarks>
+/// See https://github.com/autofac/Autofac/issues/1461. Prior to the fix, each child scope
+/// that registered a consumer of <c>Func&lt;T&gt;</c> caused the factory's expression tree to
+/// be recompiled via <c>Expression.Lambda(...).Compile()</c> on every resolution. The fix
+/// caches the compiled generator keyed on <c>(delegateType, parameterMapping)</c> so the
+/// expensive compile happens once and is reused across all scopes. Compare against a baseline
+/// package version using a command such as
+/// <c>dotnet run -c Release --project bench/Autofac.Benchmarks -- --baseline-version 9.1.0
+/// --filter *GeneratedFactoryChildScopeBenchmark*</c>.
+/// </remarks>
+public class GeneratedFactoryChildScopeBenchmark
+{
+    private IContainer _container = default!;
+
+    [Params(100, 1000)]
+    public int ScopeCount
+    {
+        get; set;
+    }
+
+    [GlobalSetup]
+    public void Setup()
+    {
+        var builder = new ContainerBuilder();
+        builder.RegisterType<Product>();
+        _container = builder.Build();
+    }
+
+    /// <summary>
+    /// Creates a child scope with a consumer of <c>Func&lt;Product&gt;</c> registered locally,
+    /// then resolves that consumer on each iteration. Before the fix, each child-scope
+    /// resolution recompiled the factory expression tree; after the fix, the compiled generator
+    /// is retrieved from a shared cache.
+    /// </summary>
+    [Benchmark]
+    public void ResolveGeneratedFactoryFromChildScope()
+    {
+        for (var i = 0; i < ScopeCount; i++)
+        {
+            using var scope = _container.BeginLifetimeScope(c => c.RegisterType<Consumer>());
+            var consumer = scope.Resolve<Consumer>();
+            GC.KeepAlive(consumer.Factory());
+        }
+    }
+
+    private sealed class Product
+    {
+    }
+
+    private sealed class Consumer
+    {
+        public Func<Product> Factory
+        {
+            get;
+        }
+
+        public Consumer(Func<Product> factory)
+        {
+            Factory = factory;
+        }
+    }
+}

src/Autofac/Core/InternalReflectionCaches.cs

@@ -3,6 +3,7 @@
 
 using System.Reflection;
 using Autofac.Core.Activators.Reflection;
+using Autofac.Features.GeneratedFactories;
 using Autofac.Util;
 using Autofac.Util.Cache;
 
@@ -48,6 +49,20 @@ public InternalReflectionCaches(ReflectionCacheSet set)
         ServiceKeyParameterAttributes = set.GetOrCreateCache<ReflectionCacheParameterDictionary<bool>>(nameof(ServiceKeyParameterAttributes));
         ServiceKeyPropertyAttributes = set.GetOrCreateCache<ReflectionCacheDictionary<PropertyInfo, bool>>(nameof(ServiceKeyPropertyAttributes));
         ServiceKeyUsageByType = set.GetOrCreateCache<ReflectionCacheDictionary<Type, bool>>(nameof(ServiceKeyUsageByType));
+
+        GeneratedFactoryServiceOnlyGenerators = set.GetOrCreateCache(
+            nameof(GeneratedFactoryServiceOnlyGenerators),
+            _ => new ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, IComponentContext, IEnumerable<Parameter>, Delegate>>
+            {
+                Usage = ReflectionCacheUsage.All,
+            });
+
+        GeneratedFactoryServiceRegistrationGenerators = set.GetOrCreateCache(
+            nameof(GeneratedFactoryServiceRegistrationGenerators),
+            _ => new ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, ServiceRegistration, IComponentContext, IEnumerable<Parameter>, Delegate>>
+            {
+                Usage = ReflectionCacheUsage.All,
+            });
     }
 
     /// <summary>
@@ -179,4 +194,24 @@ public ReflectionCacheDictionary<Type, bool> ModuleOverridesAttachToRegistration
     {
         get;
     }
+
+    /// <summary>
+    /// Gets the cache of compiled factory-delegate generators for
+    /// <see cref="Features.GeneratedFactories.FactoryGenerator"/> instances that resolve via
+    /// <c>ResolveService</c>. Keyed on <c>(delegateType, effectiveParameterMapping)</c>.
+    /// </summary>
+    public ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, IComponentContext, IEnumerable<Parameter>, Delegate>> GeneratedFactoryServiceOnlyGenerators
+    {
+        get;
+    }
+
+    /// <summary>
+    /// Gets the cache of compiled factory-delegate generators for
+    /// <see cref="Features.GeneratedFactories.FactoryGenerator"/> instances that resolve via
+    /// <see cref="IComponentContext.ResolveComponent"/>. Keyed on <c>(delegateType, effectiveParameterMapping)</c>.
+    /// </summary>
+    public ReflectionCacheTypeKeyedDictionary<ParameterMapping, Func<Service, ServiceRegistration, IComponentContext, IEnumerable<Parameter>, Delegate>> GeneratedFactoryServiceRegistrationGenerators
+    {
+        get;
+    }
 }

src/Autofac/Features/GeneratedFactories/FactoryGenerator.cs

@@ -38,25 +38,20 @@ public FactoryGenerator(Type delegateType, Service service, ParameterMapping par
 
         Enforce.ArgumentTypeIsFunction(delegateType);
 
-        _generator = CreateGenerator(
-            (activatorContextParam, resolveParameterArray) =>
-            {
-                // c, service, [new Parameter(name, (object)dps)]*
-                var resolveParams = new[]
-                {
-                    activatorContextParam,
-                    Expression.Constant(service, typeof(Service)),
-                    Expression.NewArrayInit(typeof(Parameter), resolveParameterArray),
-                };
-
-                // c.Resolve(...)
-                // default! here is for reflection only
-                return Expression.Call(
-                ReflectionExtensions.GetMethod<IComponentContext>(cc => cc.ResolveService(default!, default!)),
-                resolveParams);
-            },
-            delegateType,
-            GetParameterMapping(delegateType, parameterMapping));
+        var pm = GetParameterMapping(delegateType, parameterMapping);
+
+        // Retrieve the cached compiled generator for this delegate type and parameter mapping,
+        // or compile it once on first use. The cached delegate is parameterised on 'service'
+        // (passed at invocation time) so it contains no instance-specific captures and is safe
+        // to share across all FactoryGenerator instances with the same structural signature.
+        // Accessing ReflectionCacheSet.Shared at point-of-use (not stored in a field) ensures
+        // correct weak-reference collection of the cache set.
+        var compiledGenerator = ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceOnlyGenerators.GetOrAdd(
+            (delegateType, pm),
+            static key => CreateServiceOnlyGenerator(key.Item1, key.Item2));
+
+        // Close over the specific service so _generator matches the expected signature.
+        _generator = (context, parameters) => compiledGenerator(service, context, parameters);
     }
 
     /// <summary>
@@ -72,27 +67,22 @@ public FactoryGenerator(Type delegateType, Service service, ServiceRegistration
     {
         Enforce.ArgumentTypeIsFunction(delegateType);
 
-        _generator = CreateGenerator(
-            (activatorContextParam, resolveParameterArray) =>
-            {
-                // new ResolveRequest(service, productRegistration, [new Parameter(name, (object)dps)])*)
-                var newExpression = Expression.New(
-                _requestConstructor,
-                Expression.Constant(service, typeof(Service)),
-                Expression.Constant(productRegistration, typeof(ServiceRegistration)),
-                Expression.NewArrayInit(typeof(Parameter), resolveParameterArray),
-                Expression.Constant(null, typeof(IComponentRegistration)));
+        var pm = GetParameterMapping(delegateType, parameterMapping);
 
-                // c.Resolve(...)
-                // default! for reflection only
-                return Expression.Call(
-                activatorContextParam,
-                ReflectionExtensions.GetMethod<IComponentContext>(cc => cc.ResolveComponent(
-                    new ResolveRequest(default!, default, default(Parameter[])!, default))),
-                newExpression);
-            },
-            delegateType,
-            GetParameterMapping(delegateType, parameterMapping));
+        // Retrieve the cached compiled generator for this delegate type and parameter mapping,
+        // or compile it once on first use. The cached delegate is parameterised on both 'service'
+        // and 'productRegistration' (passed at invocation time), so it contains no instance-specific
+        // captures and is safe to share across all FactoryGenerator instances for the same delegate type.
+        // Accessing ReflectionCacheSet.Shared at point-of-use (not stored in a field) ensures
+        // correct weak-reference collection of the cache set.
+        var compiledGenerator = ReflectionCacheSet.Shared.Internal.GeneratedFactoryServiceRegistrationGenerators.GetOrAdd(
+            (delegateType, pm),
+            static key => CreateServiceRegistrationGenerator(key.Item1, key.Item2));
+
+        // Close over the specific service and product registration so _generator matches the
+        // expected signature. These values are instance-specific but are not baked into the
+        // compiled expression tree — they are passed as arguments at each invocation.
+        _generator = (context, parameters) => compiledGenerator(service, productRegistration, context, parameters);
     }
 
     /// <summary>
@@ -144,13 +134,18 @@ private static bool DelegateTypeIsFunc(Type delegateType)
         return delegateType.Name.StartsWith("Func`", StringComparison.Ordinal);
     }
 
-    private static Func<IComponentContext, IEnumerable<Parameter>, Delegate> CreateGenerator(Func<Expression, Expression[], Expression> makeResolveCall, Type delegateType, ParameterMapping pm)
+    /// <summary>
+    /// Creates a compiled generator for the overload that resolves via <c>ResolveService</c>.
+    /// The returned delegate accepts <paramref name="delegateType"/>-specific parameters at runtime and contains
+    /// no instance-specific compile-time captures, making it safe to cache and share across all
+    /// <see cref="FactoryGenerator"/> instances with the same (<paramref name="delegateType"/>, <paramref name="pm"/>) pair.
+    /// </summary>
+    private static Func<Service, IComponentContext, IEnumerable<Parameter>, Delegate> CreateServiceOnlyGenerator(Type delegateType, ParameterMapping pm)
     {
-        // (c, p) => ([dps]*) => (drt)Resolve(c, productRegistration, [new NamedParameter(name, (object)dps)]*)
-        // (c, p)
+        // Outer parameters: (Service svc, IComponentContext c, IEnumerable<Parameter> p)
+        var serviceParam = Expression.Parameter(typeof(Service), "svc");
         var activatorContextParam = Expression.Parameter(typeof(IComponentContext), "c");
         var activatorParamsParam = Expression.Parameter(typeof(IEnumerable<Parameter>), "p");
-        var activatorParams = new[] { activatorContextParam, activatorParamsParam };
 
         var invoke = delegateType.GetDeclaredMethod("Invoke");
 
@@ -163,18 +158,81 @@ private static Func<IComponentContext, IEnumerable<Parameter>, Delegate> CreateG
         Expression? resolveCast = null;
         if (DelegateTypeIsFunc(delegateType) && pm == ParameterMapping.ByType)
         {
-            // Issue #269:
-            // If we're resolving a Func<X1...XN>() and there are duplicate input parameter types
-            // and the parameter mapping is by type, we shouldn't be able to resolve it.
+            // Issue #269: duplicate input parameter types are not allowed for ByType mapping.
             var arguments = delegateType.GenericTypeArguments;
             var returnType = arguments[arguments.Length - 1];
+            Array.Resize(ref arguments, arguments.Length - 1);
+            if (arguments.Distinct().Count() != arguments.Length)
+            {
+                object[] argumentsArray = arguments.ToArray();
+                var message = string.Format(CultureInfo.CurrentCulture, GeneratedFactoryRegistrationSourceResources.DuplicateTypesInTypeMappedFuncParameterList, returnType.AssemblyQualifiedName, string.Join(", ", argumentsArray));
+                resolveCast = Expression.Throw(Expression.Constant(new DependencyResolutionException(message)), invoke.ReturnType);
+            }
+        }
+
+        if (resolveCast == null)
+        {
+            var resolveParameterArray = MapParameters(creatorParams, pm);
 
-            // Remove the return type to check the list of input types only.
+            // c.ResolveService(svc, [new Parameter(...)]*) — 'svc' is supplied as a runtime parameter.
+            var resolveParams = new Expression[]
+            {
+                activatorContextParam,
+                serviceParam,
+                Expression.NewArrayInit(typeof(Parameter), resolveParameterArray),
+            };
+
+            var resolveCall = Expression.Call(
+                ReflectionExtensions.GetMethod<IComponentContext>(cc => cc.ResolveService(default!, default!)),
+                resolveParams);
+
+            resolveCast = Expression.Convert(resolveCall, invoke.ReturnType);
+        }
+
+        // ([dps]*) => (drt)c.ResolveService(svc, [...])
+        var creator = Expression.Lambda(delegateType, resolveCast, creatorParams);
+
+        // (svc, c, p) => ([dps]*) => ...
+        var activator = Expression.Lambda<Func<Service, IComponentContext, IEnumerable<Parameter>, Delegate>>(
+            creator,
+            serviceParam,
+            activatorContextParam,
+            activatorParamsParam);
+
+        return activator.Compile();
+    }
+
+    /// <summary>
+    /// Creates a compiled generator for the overload that resolves via <see cref="IComponentContext.ResolveComponent"/>.
+    /// The returned delegate accepts <paramref name="delegateType"/>-specific parameters at runtime and contains
+    /// no instance-specific compile-time captures, making it safe to cache and share across all
+    /// <see cref="FactoryGenerator"/> instances with the same (<paramref name="delegateType"/>, <paramref name="pm"/>) pair.
+    /// </summary>
+    private static Func<Service, ServiceRegistration, IComponentContext, IEnumerable<Parameter>, Delegate> CreateServiceRegistrationGenerator(Type delegateType, ParameterMapping pm)
+    {
+        // Outer parameters: (Service svc, ServiceRegistration sr, IComponentContext c, IEnumerable<Parameter> p)
+        var serviceParam = Expression.Parameter(typeof(Service), "svc");
+        var serviceRegistrationParam = Expression.Parameter(typeof(ServiceRegistration), "sr");
+        var activatorContextParam = Expression.Parameter(typeof(IComponentContext), "c");
+        var activatorParamsParam = Expression.Parameter(typeof(IEnumerable<Parameter>), "p");
+
+        var invoke = delegateType.GetDeclaredMethod("Invoke");
+
+        // [dps]*
+        var creatorParams = invoke
+            .GetParameters()
+            .Select(pi => Expression.Parameter(pi.ParameterType, pi.Name))
+            .ToList();
+
+        Expression? resolveCast = null;
+        if (DelegateTypeIsFunc(delegateType) && pm == ParameterMapping.ByType)
+        {
+            // Issue #269: duplicate input parameter types are not allowed for ByType mapping.
+            var arguments = delegateType.GenericTypeArguments;
+            var returnType = arguments[arguments.Length - 1];
             Array.Resize(ref arguments, arguments.Length - 1);
             if (arguments.Distinct().Count() != arguments.Length)
             {
-                // There are duplicate input types - that's a problem. Throw
-                // when the function is invoked.
                 object[] argumentsArray = arguments.ToArray();
                 var message = string.Format(CultureInfo.CurrentCulture, GeneratedFactoryRegistrationSourceResources.DuplicateTypesInTypeMappedFuncParameterList, returnType.AssemblyQualifiedName, string.Join(", ", argumentsArray));
                 resolveCast = Expression.Throw(Expression.Constant(new DependencyResolutionException(message)), invoke.ReturnType);
@@ -183,23 +241,36 @@ private static Func<IComponentContext, IEnumerable<Parameter>, Delegate> CreateG
 
         if (resolveCast == null)
         {
-            // Issue #269: There aren't duplicate parameter types in the generated
-            // factory, so in the case of Func<X1...XN>() typed parameter mapping,
-            // if there are duplicate types in the target constructor, both constructor
-            // parameters will get the same value passed in. (We don't know
-            // the activator, so we can't do much more about it than that.)
-            //
-            // (drt)
             var resolveParameterArray = MapParameters(creatorParams, pm);
-            var resolveCall = makeResolveCall(activatorContextParam, resolveParameterArray);
+
+            // new ResolveRequest(svc, sr, [...], null) — 'svc' and 'sr' are runtime parameters.
+            var newExpression = Expression.New(
+                _requestConstructor,
+                serviceParam,
+                serviceRegistrationParam,
+                Expression.NewArrayInit(typeof(Parameter), resolveParameterArray),
+                Expression.Constant(null, typeof(IComponentRegistration)));
+
+            // c.ResolveComponent(new ResolveRequest(...))
+            var resolveCall = Expression.Call(
+                activatorContextParam,
+                ReflectionExtensions.GetMethod<IComponentContext>(cc => cc.ResolveComponent(
+                    new ResolveRequest(default!, default, default(Parameter[])!, default))),
+                newExpression);
+
             resolveCast = Expression.Convert(resolveCall, invoke.ReturnType);
         }
 
-        // ([dps]*) => c.Resolve(service, [new Parameter(name, dps)]*)
+        // ([dps]*) => (drt)c.ResolveComponent(new ResolveRequest(svc, sr, [...]))
         var creator = Expression.Lambda(delegateType, resolveCast, creatorParams);
 
-        // (c, p) => (
-        var activator = Expression.Lambda<Func<IComponentContext, IEnumerable<Parameter>, Delegate>>(creator, activatorParams);
+        // (svc, sr, c, p) => ([dps]*) => ...
+        var activator = Expression.Lambda<Func<Service, ServiceRegistration, IComponentContext, IEnumerable<Parameter>, Delegate>>(
+            creator,
+            serviceParam,
+            serviceRegistrationParam,
+            activatorContextParam,
+            activatorParamsParam);
 
         return activator.Compile();
     }