Repository navigation
Simplify middleware ownership with structured continuations - #58
Open
dombrovsky wants to merge 5 commits into
Open
dombrovsky wants to merge 5 commits into
dombrovsky wants to merge 5 commits into
Conversation
…ing compatibility
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
TaskSchedulerEnqueueContext<TResult>.InvokeWithCallerWait(next, callerWait)explicitly retains downstream work. Built-in timeout implementations use it, including the netstandard TimeProvider extension.nextmust migrate to the explicit boundary or await its producer. Execution/completion middleware must join attempts.Task.WhenAll, so a synchronous throw cannot bypass joining an earlier attempt.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
SingleFlight_SharesOneProducerAndAllowsIndependentWaitCancellationthrough 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.SharedProducer_DetachesCallerButPreservesOwnershipCancellationfor 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.docs/customization.md.Validation and remaining failures
36a1b62reproduced the cancellation-scope/cancel-previous failures. Every observed failing case reproduced on the same runtime exceptMultipleScopes_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.e799b85passed 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.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=2for 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.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.
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 candidate4d766de; 1.0.00800af84229d7beb3a6fa507d236d0ff533b0c92. 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 arec4b6dad2a7f11ee83eb4c0a13783d0192703365e.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.
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.