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

Since 9.2, the built middleware chain wrapped each stage in an
Action passed to a shared ExecuteWithDiagnostics helper. That inner
lambda captured the per-invocation ResolveRequestContext, so a fresh
closure was allocated for every middleware stage on every resolve,
regressing allocations vs. 9.1.

Inline the diagnostics/metrics logic into each build-time lambda so
they close only over build-time state (next, stage, stagePhase,
stageName) and are allocated once per pipeline build, restoring 9.1
allocation behavior. Both the standard and metrics-enabled chains are
fixed.

Add a ResolvePipelineAllocationBenchmark mirroring the reported repro,
and a PipelineBuilderTests regression test asserting a built pipeline
allocates zero bytes per invocation.

Author: tillig

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. See issue #1493.
+/// </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

@@ -140,23 +140,34 @@ private static ResolvePipeline BuildPipeline(MiddlewareDeclaration? lastDecl)
         var current = lastDecl;
         var currentInvoke = _terminateAction;
 
-        Action<ResolveRequestContext> BuildMiddlewareChain(Action<ResolveRequestContext> next, IResolveMiddleware stage)
+        while (current is not null)
         {
+            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.
-            return AutofacMetrics.MetricsEnabled
-                ? BuildMetricsMiddlewareChain(next, stage)
-                : BuildStandardMiddlewareChain(next, stage);
+            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, () =>
+    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). This lambda
+        // must only close over build-time state (next, stage, stagePhase, stageName)
+        // so it is allocated once per pipeline build rather than once per resolve.
+        // See issue #1493.
+        return context =>
+        {
+            if (!context.DiagnosticSource.IsEnabled())
             {
                 context.PhaseReached = stagePhase;
                 var timer = ValueStopwatch.StartNew();
@@ -168,37 +179,66 @@ 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. This lambda must only close
+        // over build-time state (next, stage, stagePhase) so it is allocated once per
+        // pipeline build rather than once per resolve. See issue #1493.
+        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.
             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 +252,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,40 @@ public void CannotAddBadPhaseToPipelineInUseRangeExistingMiddleware()
         }));
     }
 
+    // Regression test for https://github.com/autofac/Autofac/issues/1493. In 9.2 the
+    // built middleware chain wrapped each stage in a lambda that captured the
+    // per-invocation ResolveRequestContext, so a fresh closure was allocated for every
+    // stage on every resolve. The built pipeline should close only over build-time
+    // state, so invoking it must not allocate on the hot path.
+    [Fact]
+    public void InvokingBuiltPipelineDoesNotAllocatePerInvocation()
+    {
+        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
     {