Skip to content

Commit 5988884

Browse files
committed
Address PR feedback: trim and prefix issue-1493 comments
1 parent 66c7093 commit 5988884

2 files changed

Lines changed: 9 additions & 13 deletions

File tree

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,10 @@ private static Action<ResolveRequestContext> BuildMetricsMiddlewareChain(Action<
160160
var stagePhase = stage.Phase;
161161
var stageName = stage.ToString()!;
162162

163-
// Metrics are captured around each stage execution while preserving
164-
// diagnostics callbacks (if enabled for the current request). This lambda
165-
// must only close over build-time state (next, stage, stagePhase, stageName)
166-
// so it is allocated once per pipeline build rather than once per resolve.
167-
// See issue #1493.
163+
// Metrics are captured around each stage execution while preserving diagnostics
164+
// callbacks (if enabled for the current request). Issue 1493: this lambda must
165+
// only close over build-time state so it is allocated once per pipeline build
166+
// rather than once per resolve.
168167
return context =>
169168
{
170169
if (!context.DiagnosticSource.IsEnabled())
@@ -218,9 +217,9 @@ private static Action<ResolveRequestContext> BuildStandardMiddlewareChain(Action
218217
{
219218
var stagePhase = stage.Phase;
220219

221-
// Hot path when execution metrics are disabled. This lambda must only close
222-
// over build-time state (next, stage, stagePhase) so it is allocated once per
223-
// pipeline build rather than once per resolve. See issue #1493.
220+
// Hot path when execution metrics are disabled. Issue 1493: this lambda must only
221+
// close over build-time state so it is allocated once per pipeline build rather
222+
// than once per resolve.
224223
return context =>
225224
{
226225
// Same basic flow in if/else, but doing a one-time check for diagnostics

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -425,11 +425,8 @@ public void CannotAddBadPhaseToPipelineInUseRangeExistingMiddleware()
425425
}));
426426
}
427427

428-
// Regression test for https://github.com/autofac/Autofac/issues/1493. In 9.2 the
429-
// built middleware chain wrapped each stage in a lambda that captured the
430-
// per-invocation ResolveRequestContext, so a fresh closure was allocated for every
431-
// stage on every resolve. The built pipeline should close only over build-time
432-
// state, so invoking it must not allocate on the hot path.
428+
// Issue 1493: a built pipeline must close only over build-time state, so invoking it
429+
// must not allocate a per-invocation closure for each middleware stage on the hot path.
433430
[Fact]
434431
public void InvokingBuiltPipelineDoesNotAllocatePerInvocation()
435432
{

0 commit comments

Comments
 (0)