Skip to content

Simplify middleware ownership with structured continuations - #58

Open
dombrovsky wants to merge 5 commits into
mainfrom
perf/structured-middleware-contract
Open

dombrovsky wants to merge 5 commits into
mainfrom
perf/structured-middleware-contract

Conversation

@dombrovsky

@dombrovsky dombrovsky commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Ordinary middleware now uses a structured continuation contract, removing per-layer ownership references and asynchronous observers. Explicit caller-wait boundaries retain operation ownership when a caller finishes before downstream work. This preserves the independent-wait capability required by #24 while reducing the layer-dependent allocation cost.

Contract and compatibility

  • Return or await every ordinary continuation started by a middleware invocation before its Task/ValueTask settles, including failure and cancellation paths. Direct forwarding, awaited post-processing, short-circuiting, sequential retries, and fully joined concurrent attempts remain supported.
  • Saved continuations invoked after middleware completion and detached execution/completion callbacks are intentionally unsupported. No forwarding-marker interfaces or fast/slow middleware categories are introduced.
  • TaskSchedulerEnqueueContext<TResult>.InvokeWithCallerWait(next, callerWait) explicitly retains downstream work. Built-in timeout implementations use it, including the netstandard TimeProvider extension.
  • Nested boundary semantics: the owned continuation's task follows its immediate downstream result. An inner caller wait may complete it before the actual user delegate/completion pipeline terminates. It is not an actual-termination signal. Operation-level references still retain outstanding admission, keyed exclusion and owned resources until underlying work settles.
  • Wait policies must observe late producer failures. Producer cancellation sources and cleanup belong inside the owned continuation rather than the shorter caller-wait scope. No new public termination-tracking API is needed for current built-ins or these narrow public-API prototypes.
  • Third-party enqueue middleware that returned early after directly invoking next must migrate to the explicit boundary or await its producer. Execution/completion middleware must join attempts.
  • Execution continuation invocation can now throw synchronously; protect invocation and awaiting together. Concurrent attempts use an async attempt helper before Task.WhenAll, so a synchronous throw cannot bypass joining an earlier attempt.
  • Direct enqueue forwarding exposes the original downstream task status. Task.FromException<T>(OperationCanceledException) is observed as Faulted inside enqueue middleware; awaiting preserves the exception, and the outer scheduled task still reports cancellation.

Review corrections and restored coverage

  • Corrected the nested-termination promise in XML and Markdown. Added 12 gated nested-boundary cases covering both caller completion orders, inner wait failure, and late producer success/fault/cancellation. They verify early outer task completion, held outstanding capacity, pending owner disposal, exact fault identity, completion processing and once-only cleanup after termination.
  • Linked those cases into the TimeProvider test project, exercising the netstandard2.0 assets on both runtimes.
  • Restored SingleFlight_SharesOneProducerAndAllowsIndependentWaitCancellation through the public ownership boundary, including independent leader/follower cancellation, shared result/failure, one producer execution, completion accounting, retained active key and fresh work after retirement.
  • Restored SharedProducer_DetachesCallerButPreservesOwnershipCancellation for local and shared owners. Gated cancellation proves that notification is not termination: the active key, producer resources and lifetime remain held; no same-key successor executes before termination.
  • Added all-waiters-canceled/late-producer-fault coverage. The prototype observes late failures and owns key retirement and cancellation-source disposal through the whole producer task.
  • Kept the structured single-flight and token-only tests as distinct coverage. The prototype remains Int32-only test scaffolding, not a production implementation of Feature request: Share queued and in-flight operations with leader promotion #10 or leader promotion.
  • Shortened phase-specific XML remarks and moved the structured rule and minimal concurrent-attempt example beside the existing middleware author invariants in docs/customization.md.

Validation and remaining failures

  • Final solution Release build passed for netstandard2.0/net8.0/net10.0: 0 warnings, 0 errors, including the repository public API analyzers. Public API declaration files are unchanged by this follow-up.
  • 235 focused tests passed on each of .NET 8 and .NET 10.
  • Local full solution runs: core 7 failed / 2,443 total on .NET 8, 4 failed / 2,443 total on .NET 10. All 17 new core cases passed.
  • All extension suites passed on both runtimes: DI 30, logging 5, TimeProvider/netstandard 70 cases each (including 12 new nested-boundary cases).
  • Full and targeted runs on untouched pre-PR main 36a1b62 reproduced the cancellation-scope/cancel-previous failures. Every observed failing case reproduced on the same runtime except MultipleScopes_Cancel_ShouldCancelOperationsThatBelongToScope(TaskFlow) on .NET 8. That exact case reproduced on baseline .NET 10 and passed the candidate's isolated .NET 8 rerun; its .NET 8 baseline reproduction remains unconfirmed after targeted retries. Do not treat the local full suite as green or silently classify this gap as proven baseline behavior.
  • Untouched main full core runs themselves failed: 6/2,419 on .NET 8, 8/2,419 on .NET 10. Additional targeted retries produced varying failures in these unchanged cancellation assertions. No unrelated tests or timeouts were changed to obtain a pass.
  • Whitespace fixer, whitespace verification, IDE0161 namespace verification and CRLF-aware diff whitespace check passed. Formatter returned success with workspace-load warnings; private-field grouping reviewed.
  • All 23 benchmark workloads passed twice; all 12 Python reporting checks passed.
  • GitHub Actions on e799b85 passed all nine executed checks (deployment skipped). The full CI build/test workflow passed all 2,443 core tests on each runtime, plus DI 30, logging 5 and TimeProvider/netstandard 70 on each. This separate green CI run does not erase the local failure/reproduction details above.
  • PR remains open and is not merged.

Performance measurements

BenchmarkDotNet 0.15.8; identical middleware harness and Layers 0/1/2/4/8, sequentially on the same Debian 13 / EPYC 9V74 runner, SDK 10.0.402 and runtime .NET 10.0.13. DOTNET_PROCESSOR_COUNT=2 for all builds/runs; CPU quota two CPUs, affinity [0,1,2], concurrent workstation GC, unchanged tiering/GC settings. One launch, three warmups, eight configured measurement iterations. No unrelated tests/builds ran during these measurements.

Layers Pre-change main Candidate 1.0.0 Saved vs main Candidate vs 1.0.0
0 513 B 512 B 512 B 1 B (0.2%) +0 B (0.0%)
1 1,723 B 1,603 B 1,571 B 120 B (7.0%) +32 B (2.0%)
2 2,348 B 2,109 B 2,051 B 239 B (10.2%) +58 B (2.8%)
4 3,598 B 3,118 B 3,016 B 480 B (13.3%) +102 B (3.4%)
8 6,095 B 5,136 B 4,936 B 959 B (15.7%) +200 B (4.1%)

Approximately 120 B saved per layer, or 959 B / 15.7% at eight layers. Marginal allocation cost falls about 19.2% (625 → 505 B/layer). The bare-terminal one-byte difference is measurement/rounding noise. Remaining allocation regression against 1.0.0 is 200 B / 4.1% at eight layers; contexts, closures, completion exception transformation and fixed operation ownership remain.

Layers Main mean ± SD (µs) Candidate mean ± SD (µs) 1.0.0 mean ± SD (µs)
0 1.198 ± 0.080 1.230 ± 0.118 1.499 ± 0.141
1 2.330 ± 0.347 2.184 ± 0.199 2.393 ± 0.195
2 2.930 ± 0.255 2.300 ± 0.316 2.627 ± 0.313
4 4.323 ± 0.862 2.890 ± 0.422 3.293 ± 0.386
8 6.342 ± 1.320 4.115 ± 0.617 4.342 ± 0.709

Times are means ± standard deviation, not per-request latency percentiles. BenchmarkDotNet retained seven/eight samples per case after default outlier processing. The eight-layer 99.9% confidence intervals overlap: main 3.818–8.866 µs, candidate 2.935–5.296 µs, release 2.744–5.940 µs. The mean against main fell about 35%, but no statistically established latency improvement is claimed. Shared-runner samples and one launch are a limitation.

Sources: pre-change main 36a1b62 (merged #56); measured candidate 4d766de; 1.0.0 0800af84229d7beb3a6fa507d236d0ff533b0c92. Subsequent follow-ups change tests and documentation, including XML comments; runtime method bodies are unchanged from the measured candidate. Only the common harness was overlaid on the release checkout; release production sources were unchanged. All three middleware harness blob hashes are c4b6dad2a7f11ee83eb4c0a13783d0192703365e.

DOTNET_PROCESSOR_COUNT=2 dotnet run --project TaskFlow.Benchmarks -c Release --no-build -- \
  --filter '*MiddlewareBenchmarks*' --warmupCount 3 --iterationCount 8 --launchCount 1 \
  --artifacts <separate-output-directory>

Raw JSON/CSV/Markdown reports, environment/provenance, commands and test evidence are retained in the execution workspace, rather than committed to the repository. An exploratory baseline that overlapped preparation was discarded and excluded from these numbers.

This review follow-up adds no public API, activity reference, observer, closure, or async wrapper to production execution. A source comparison excluding XML comments confirms unchanged runtime text in all four edited production files. No new manual timing run was warranted for documentation/test-only changes; all 23 modern benchmark workloads were validated twice again.

Complete A–D audit of original PR test changes

Audit baseline: merge-base/pre-PR main 36a1b62, compared with original PR head e51b75b and this follow-up.

A = intentionally unsupported ordinary detached/late continuation usage.
B = supported behavior preserved with structured invocation or an explicit boundary.
C = supported capability that must remain; missing coverage restored here.
D = obsolete implementation detail, not a public invariant.

File / changed assertion or helper Classification Disposition / equivalent coverage
CompletionContinuationLifetime: IndependentCases/IndependentCompletion renamed to StructuredCases/StructuredCompletion A, D Names and helper now describe joined children; the removed ordinary detached implementation is not preserved.
AwaitedChildren: success 17 or outer exception before child release becomes IsCompleted == false A for early settlement; B for result/failure Original result 17 and exact outer failure remain asserted after release, as do child count, replacement/caller/producer tokens, finished count and reverse cleanup.
ReturnedSubmissionFailure: result 17 moves after gated completion A for early settlement; B otherwise Pending-before-release, recovered result 17, child FormatException and one downstream completion/cleanup are retained.
ChildFinishesFirst: uses joined helper B Child completion alone still cannot release a held parent; pending disposal and reverse cleanup assertions remain.
NoNext: remove LateNext throw and unchanged-child-count after illegal late call A, D Saved invocation after middleware completion is unsupported. No exhaustive runtime rejection is promised. Zero downstream calls (or failure fallback once), outcome and reverse cleanup remain tested.
OutstandingAdmission: early completed result changes to pending A for detached completion; B for accounting Full rejection, waiting admission, resumed results and final finished count still asserted. Nested explicit-boundary tests now additionally cover early caller completion with outstanding capacity held.
Remove RetainedContinuation_CanStillStartWhileAnIndependentChildIsActive A The entire late-invocation scenario is unsupported. Its safety assertions (wait for active children, no early cleanup, final cleanup order) remain covered by joined-child, disposal-race and new boundary tests.
DisposalRacingChildCompletion: create pending operation before race, await result after release B Result 17 and exact finish/cleanup sequence remain.
Completion helper removes saved LateNext and adds WhenAll A, D for saved continuation; B for joining No universal activity tracking restored; structured completion joins every started child.
FuturePolicy: replace independent-caller single-flight test with leader-awaiting test C Original capability restored in SingleFlight_SharesOneProducerAndAllowsIndependentWaitCancellation using an explicit boundary. Also tests follower cancellation, shared success/fault identity, one execution, completion accounting, retained key and later fresh execution. Structured leader-awaiting test kept as distinct B coverage.
FuturePolicy: replace ownership single-flight scenario with token-only scenario; drop canceled leader, pending follower, canceled follower and ActiveCount assertions C SharedProducer_DetachesCallerButPreservesOwnershipCancellation restored for local and shared owners. Includes independent leader wait, producer cancellation notification separated from actual termination, canceled followers, retained key/no successor execution, zero early cleanup, exact release/cleanup order. Token-only test remains distinct B coverage.
FuturePolicy: old OwnershipSingleFlightMiddleware removed; original single-flight helper now awaits Produce D for prototype mechanics; C for capability New narrow OwnedSingleFlight prototype places Produce (including token-source disposal and key retirement) inside the owned continuation. Observes late faults after every caller cancels. No production single-flight/promotion implementation added.
LifetimeTracking: IndependentEnqueueTask changes early zero result to pending; helper awaits Underlying A for ordinary detached branch; B for resource/admission safety Both child-first and parent-first orders retained, with concurrent Next variants. Final zero result, Full rejection, no early cleanup and cleanup completion still asserted. Explicit early caller completion is covered by new nested-boundary cases.
LifetimeTracking: immediate next task containing OCE now Faulted instead of Canceled D for per-layer async normalization; B for external cancellation/exception identity Direct task status is deliberately documented. The original exception and canceled outer operation assertions remain. Original 1.0.0 forwarding also lacked the later per-layer normalizing observer.
ForwardingConformance: ordinary ThrowAfterForward changed to explicit boundary A for ordinary detached usage; B for accepted-work ownership Same exact synchronous exception, accepted child, disposal held pending, no early cleanup, one completion and final cleanup remain asserted through the public boundary.
StructuredMiddleware: joined concurrent attempts (including synchronous fault/OCE) B, new regression coverage Every attempt is joined before pending operation/disposal settle. Exact fault identity, cancellation, call count and cleanup are asserted.
StructuredMiddleware: timeout with immediate/delayed admission C, new regression coverage Caller timeout still leaves producer, outstanding capacity and resources alive until actual termination.
New CallerWaitOwnership: nested boundaries, both wait orders, success/fault/cancel producer and inner-wait failure B, C Distinguishes outer owned-task completion from actual termination. Checks Full admission, original late fault, once-only completion/cleanup and exact delegate/completion/cleanup order.
New SingleFlight_AllWaitersCancel_LateProducerFaultIsObservedBeforeCleanup C Both callers may finish canceled before producer failure; failure remains observed, key stays active until termination, owner disposal drains work and cleanup occurs once.

No supported original capability was found to require automatic tracking of arbitrary detached continuations. Independent wait cancellation and shared ownership remain expressible with the existing public boundary. Full issue #10 leader promotion remains separate; the prototype does not claim nested downstream caller tasks are termination signals. For a future policy, retain the relevant execution scope or use coordinated operation ownership instead.

No negative runtime test was added for unsupported saved-continuation use: forcing rejection would promise the universal detection deliberately removed by this PR. The contract states the restriction explicitly.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant