diff --git a/bench/Autofac.Benchmarks/BenchmarkSet.cs b/bench/Autofac.Benchmarks/BenchmarkSet.cs index 7583dc043..316b1a0e7 100644 --- a/bench/Autofac.Benchmarks/BenchmarkSet.cs +++ b/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), diff --git a/bench/Autofac.Benchmarks/ResolvePipelineAllocationBenchmark.cs b/bench/Autofac.Benchmarks/ResolvePipelineAllocationBenchmark.cs new file mode 100644 index 000000000..82c9a2e98 --- /dev/null +++ b/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; + +/// +/// 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. +/// +public class ResolvePipelineAllocationBenchmark +{ + private IContainer _container = default!; + + [GlobalSetup] + public void Setup() + { + var builder = new ContainerBuilder(); + builder.RegisterType().As(); + builder.RegisterType().As(); + builder.RegisterType().As(); + _container = builder.Build(); + } + + [Benchmark(Baseline = true)] + public IService NoDependencies() => _container.Resolve(); + + [Benchmark] + public IConsumer OneDependency() => _container.Resolve(); + + 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; + } + } +} diff --git a/src/Autofac/Core/Resolving/Pipeline/ResolvePipelineBuilder.cs b/src/Autofac/Core/Resolving/Pipeline/ResolvePipelineBuilder.cs index b58ba64b8..22f77055a 100644 --- a/src/Autofac/Core/Resolving/Pipeline/ResolvePipelineBuilder.cs +++ b/src/Autofac/Core/Resolving/Pipeline/ResolvePipelineBuilder.cs @@ -13,18 +13,19 @@ namespace Autofac.Core.Resolving.Pipeline; /// /// /// -/// The pipeline builder is built as a doubly-linked list; each node in that list is a -/// , 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 , that holds the middleware +/// instance, and the reference to the next and previous nodes. /// -/// /// -/// 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. /// -/// /// -/// 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. /// /// internal class ResolvePipelineBuilder : IResolvePipelineBuilder, IEnumerable @@ -108,8 +109,8 @@ public IResolvePipeline Build() /// 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 BuildMiddlewareChain(Action 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 BuildMetricsMiddlewareChain(Action 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 BuildMetricsMiddlewareChain(Action 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,29 +183,59 @@ Action BuildMetricsMiddlewareChain(Action BuildStandardMiddlewareChain(Action 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 BuildStandardMiddlewareChain(Action 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; } @@ -198,7 +243,8 @@ static void ExecuteWithDiagnostics(ResolveRequestContext context, IResolveMiddle 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( diff --git a/test/Autofac.Test/Core/Pipeline/PipelineBuilderTests.cs b/test/Autofac.Test/Core/Pipeline/PipelineBuilderTests.cs index a8686a1ee..56cd097e3 100644 --- a/test/Autofac.Test/Core/Pipeline/PipelineBuilderTests.cs +++ b/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 {