Fix per-resolve closure allocation in resolve pipeline (#1493)

bench/Autofac.Benchmarks/BenchmarkSet.cs

@@ -29,6 +29,7 @@ public static class BenchmarkSet
         typeof(EnumerableResolveBenchmark),
         typeof(PropertyInjectionBenchmark),
         typeof(RootContainerResolveBenchmark),
+        typeof(ResolvePipelineAllocationBenchmark),
         typeof(OpenGenericBenchmark),
         typeof(MultiConstructorBenchmark),
         typeof(LambdaResolveBenchmark),

bench/Autofac.Benchmarks/ResolvePipelineAllocationBenchmark.cs

@@ -0,0 +1,59 @@
+// 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>
+/// Tests the per-resolve allocation cost of running the resolve pipeline. Each
+/// resolve walks the built middleware chain, so a closure allocated per stage
+/// per invocation (rather than once at pipeline build time) shows up here as
+/// extra allocations that scale with graph depth.
+/// </summary>
+public class ResolvePipelineAllocationBenchmark
+{
+    private IContainer _container = default!;
+
+    [GlobalSetup]
+    public void Setup()
+    {
+        var builder = new ContainerBuilder();
+        builder.RegisterType<Service>().As<IService>();
+        builder.RegisterType<Dependency>().As<IDependency>();
+        builder.RegisterType<Consumer>().As<IConsumer>();
+        _container = builder.Build();
+    }
+
+    [Benchmark(Baseline = true)]
+    public IService NoDependencies() => _container.Resolve<IService>();
+
+    [Benchmark]
+    public IConsumer OneDependency() => _container.Resolve<IConsumer>();
+
+    public interface IService
+    {
+    }
+
+    public interface IDependency
+    {
+    }
+
+    public interface IConsumer
+    {
+    }
+
+    public sealed class Service : IService
+    {
+    }
+
+    public sealed class Dependency : IDependency
+    {
+    }
+
+    public sealed class Consumer : IConsumer
+    {
+        public Consumer(IDependency dependency)
+        {
+            _ = dependency;
+        }
+    }
+}

src/Autofac/Core/Resolving/Pipeline/ResolvePipelineBuilder.cs

@@ -13,18 +13,19 @@ namespace Autofac.Core.Resolving.Pipeline;
 /// </summary>
 /// <remarks>
 /// <para>
-/// The pipeline builder is built as a doubly-linked list; each node in that list is a
-/// <see cref="MiddlewareDeclaration"/>, that holds the middleware instance, and the reference to the next and previous nodes.
+/// The pipeline builder is built as a doubly-linked list; each node in that
+/// list is a <see cref="MiddlewareDeclaration"/>, that holds the middleware
+/// instance, and the reference to the next and previous nodes.
 /// </para>
-///
 /// <para>
-/// When you call one of the Use* methods, we find the appropriate node in the linked list based on the phase of the new middleware
-/// and insert it into the list.
+/// When you call one of the Use* methods, we find the appropriate node in the
+/// linked list based on the phase of the new middleware and insert it into the
+/// list.
 /// </para>
-///
 /// <para>
-/// When you build a pipeline, we walk back through that set of middleware and generate the concrete call chain so that when you execute the pipeline,
-/// we don't iterate over any nodes, but just invoke the built set of methods.
+/// When you build a pipeline, we walk back through that set of middleware and
+/// generate the concrete call chain so that when you execute the pipeline, we
+/// don't iterate over any nodes, but just invoke the built set of methods.
 /// </para>
 /// </remarks>
 internal class ResolvePipelineBuilder : IResolvePipelineBuilder, IEnumerable<IResolveMiddleware>
@@ -108,8 +109,8 @@ public IResolvePipeline Build()
     /// <inheritdoc/>
     public IResolvePipelineBuilder Clone()
     {
-        // To clone a pipeline, we create a new instance, then insert the same stage
-        // objects in the same order.
+        // To clone a pipeline, we create a new instance, then insert the same
+        // stage objects in the same order.
         var newPipeline = new ResolvePipelineBuilder(Type);
         var currentStage = _first;
 
@@ -136,27 +137,41 @@ IEnumerator IEnumerable.GetEnumerator()
 
     private static ResolvePipeline BuildPipeline(MiddlewareDeclaration? lastDecl)
     {
-        // When we build, we go through the set and construct a single call stack, starting from the end.
+        // When we build, we go through the set and construct a single call
+        // stack, starting from the end.
         var current = lastDecl;
         var currentInvoke = _terminateAction;
 
-        Action<ResolveRequestContext> BuildMiddlewareChain(Action<ResolveRequestContext> next, IResolveMiddleware stage)
+        while (current is not null)
         {
-            // MetricsEnabled is static readonly (set once at startup), so checking here
-            // at pipeline build time avoids a per-invocation branch in every middleware.
-            return AutofacMetrics.MetricsEnabled
-                ? BuildMetricsMiddlewareChain(next, stage)
-                : BuildStandardMiddlewareChain(next, stage);
+            var stage = current.Middleware;
+
+            // MetricsEnabled is static readonly (set once at startup), so
+            // checking here at pipeline build time avoids a per-invocation
+            // branch in every middleware.
+            currentInvoke = AutofacMetrics.MetricsEnabled
+                ? BuildMetricsMiddlewareChain(currentInvoke, stage)
+                : BuildStandardMiddlewareChain(currentInvoke, stage);
+            current = current.Previous;
         }
 
-        Action<ResolveRequestContext> BuildMetricsMiddlewareChain(Action<ResolveRequestContext> next, IResolveMiddleware stage)
-        {
-            var stagePhase = stage.Phase;
-            var stageName = stage.ToString()!;
+        return new ResolvePipeline(currentInvoke);
+    }
 
-            // Metrics are captured around each stage execution while preserving
-            // diagnostics callbacks (if enabled for the current request).
-            return context => ExecuteWithDiagnostics(context, stage, () =>
+    [ExcludeFromCodeCoverage]
+    private static Action<ResolveRequestContext> BuildMetricsMiddlewareChain(Action<ResolveRequestContext> next, IResolveMiddleware stage)
+    {
+        var stagePhase = stage.Phase;
+        var stageName = stage.ToString()!;
+
+        // Metrics are captured around each stage execution while preserving
+        // diagnostics callbacks (if enabled for the current request).
+        //
+        // Issue 1493: this lambda must only close over build-time state so it
+        // is allocated once per pipeline build rather than once per resolve.
+        return context =>
+        {
+            if (!context.DiagnosticSource.IsEnabled())
             {
                 context.PhaseReached = stagePhase;
                 var timer = ValueStopwatch.StartNew();
@@ -168,37 +183,68 @@ Action<ResolveRequestContext> BuildMetricsMiddlewareChain(Action<ResolveRequestC
                 {
                     AutofacMetrics.RecordMiddlewareExecution(stageName, timer.GetElapsedTime());
                 }
-            });
-        }
 
-        Action<ResolveRequestContext> BuildStandardMiddlewareChain(Action<ResolveRequestContext> next, IResolveMiddleware stage)
-        {
-            var stagePhase = stage.Phase;
+                return;
+            }
 
-            // Hot path when execution metrics are disabled.
-            return context => ExecuteWithDiagnostics(context, stage, () =>
+            context.DiagnosticSource.MiddlewareStart(context, stage);
+            var succeeded = false;
+            try
             {
                 context.PhaseReached = stagePhase;
-                stage.Execute(context, next);
-            });
-        }
+                var timer = ValueStopwatch.StartNew();
+                try
+                {
+                    stage.Execute(context, next);
+                }
+                finally
+                {
+                    AutofacMetrics.RecordMiddlewareExecution(stageName, timer.GetElapsedTime());
+                }
+
+                succeeded = true;
+            }
+            finally
+            {
+                if (succeeded)
+                {
+                    context.DiagnosticSource.MiddlewareSuccess(context, stage);
+                }
+                else
+                {
+                    context.DiagnosticSource.MiddlewareFailure(context, stage);
+                }
+            }
+        };
+    }
 
-        static void ExecuteWithDiagnostics(ResolveRequestContext context, IResolveMiddleware stage, Action action)
+    private static Action<ResolveRequestContext> BuildStandardMiddlewareChain(Action<ResolveRequestContext> next, IResolveMiddleware stage)
+    {
+        var stagePhase = stage.Phase;
+
+        // Hot path when execution metrics are disabled.
+        //
+        // Issue 1493: this lambda must only close over build-time state so it
+        // is allocated once per pipeline build rather than once per resolve.
+        return context =>
         {
-            // Same basic flow in if/else, but doing a one-time check for diagnostics
-            // and choosing the "diagnostics enabled" version vs. the more common
-            // "no diagnostics enabled" path: hot-path optimization.
+            // Same basic flow in if/else, but doing a one-time check for
+            // diagnostics and choosing the "diagnostics enabled" version vs.
+            // the more common "no diagnostics enabled" path: hot-path
+            // optimization.
             if (!context.DiagnosticSource.IsEnabled())
             {
-                action();
+                context.PhaseReached = stagePhase;
+                stage.Execute(context, next);
                 return;
             }
 
             context.DiagnosticSource.MiddlewareStart(context, stage);
             var succeeded = false;
             try
             {
-                action();
+                context.PhaseReached = stagePhase;
+                stage.Execute(context, next);
                 succeeded = true;
             }
             finally
@@ -212,16 +258,7 @@ static void ExecuteWithDiagnostics(ResolveRequestContext context, IResolveMiddle
                     context.DiagnosticSource.MiddlewareFailure(context, stage);
                 }
             }
-        }
-
-        while (current is not null)
-        {
-            var stage = current.Middleware;
-            currentInvoke = BuildMiddlewareChain(currentInvoke, stage);
-            current = current.Previous;
-        }
-
-        return new ResolvePipeline(currentInvoke);
+        };
     }
 
     private bool InsertRangeWithinExistingStages(

test/Autofac.Test/Core/Pipeline/PipelineBuilderTests.cs

@@ -425,6 +425,39 @@ public void CannotAddBadPhaseToPipelineInUseRangeExistingMiddleware()
         }));
     }
 
+    [Fact]
+    public void InvokingBuiltPipelineDoesNotAllocatePerInvocation()
+    {
+        // Issue 1493: a built pipeline must close only over build-time state,
+        // so invoking it must not allocate a per-invocation closure for each
+        // middleware stage on the hot path.
+        var pipelineBuilder = new ResolvePipelineBuilder(PipelineType.Service);
+        pipelineBuilder.Use(PipelinePhase.ResolveRequestStart, (context, next) => next(context));
+        pipelineBuilder.Use(PipelinePhase.ScopeSelection, (context, next) => next(context));
+        pipelineBuilder.Use(PipelinePhase.Sharing, (context, next) => next(context));
+
+        var built = pipelineBuilder.Build();
+        var context = new PipelineRequestContextStub();
+
+        // Warm up so JIT compilation and any first-run allocations happen
+        // before we measure.
+        for (var i = 0; i < 100; i++)
+        {
+            built.Invoke(context);
+        }
+
+        const int Iterations = 1000;
+        var before = GC.GetAllocatedBytesForCurrentThread();
+        for (var i = 0; i < Iterations; i++)
+        {
+            built.Invoke(context);
+        }
+
+        var allocated = GC.GetAllocatedBytesForCurrentThread() - before;
+
+        Assert.Equal(0, allocated);
+    }
+
     [SuppressMessage("CA1001", "CA1001", Justification = "This is an expedient test stub; we don't really care if proper disposal for internal stubs happens.")]
     private class PipelineRequestContextStub : ResolveRequestContext
     {