Perf: materialize streamed orchestration history incrementally#783
Open
berndverst wants to merge 2 commits into
Open
Perf: materialize streamed orchestration history incrementally#783berndverst wants to merge 2 commits into
berndverst wants to merge 2 commits into
Conversation
Replace the lazy Concat/Select chain in BuildRuntimeStateAsync with direct List<HistoryEvent> accumulation. Previously, each streamed history chunk extended a lazily-evaluated IEnumerable via Concat, which is re-walked and re-converted from scratch every time it is enumerated or chained onto again, making history reconstruction quadratic in the number of chunks for long-running orchestrations. Now each chunk's events are converted and appended directly into a single accumulating list (with a capacity hint sized per chunk), and new events are added to the runtime state via a direct foreach instead of a lazy Select. Event conversion, ordering, new-vs-past runtime state classification, cancellation propagation, and the public API surface are all unchanged. Added regression tests covering multi-chunk streamed history materialization (ordering + new/past classification) and cancellation propagation mid-stream. Fixes #772 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fdfdae0-4987-4043-b522-7229e63810d2
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves worker-side orchestration replay performance by replacing lazy IEnumerable accumulation (Concat/Select) in BuildRuntimeStateAsync with eager, incremental materialization into a single List<HistoryEvent>, preventing quadratic behavior when history is streamed in many chunks.
Changes:
- Materialize streamed and non-streamed
PastEventsinto a singleList<HistoryEvent>as events arrive. - Add
NewEventstoOrchestrationRuntimeStatevia direct iteration and conversion (no lazy projections). - Add regression tests covering multi-chunk streaming order/classification and cancellation propagation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs | Reworks history accumulation to eagerly append converted events instead of chaining lazy Concat/Select. |
| test/Worker/Grpc.Tests/GrpcDurableTaskWorkerTests.cs | Adds targeted tests for multi-chunk history materialization correctness and cancellation behavior. |
Comment on lines
+280
to
+284
| pastEvents.Capacity = Math.Max(pastEvents.Capacity, pastEvents.Count + chunk.Events.Count); | ||
| foreach (P.HistoryEvent protoEvent in chunk.Events) | ||
| { | ||
| pastEvents.Add(converter(protoEvent)); | ||
| } |
Address review feedback on PR #783 (issue #772): setting List<HistoryEvent>.Capacity to the exact running total on every streamed chunk reallocates and copies the whole list whenever capacity needs to grow, since the Capacity setter allocates exactly the requested size rather than growing geometrically. With many single-event chunks this made history materialization quadratic again - the same class of bug the original fix addressed, reintroduced via a different mechanism. Remove the explicit Capacity assignment and rely on List<T>.Add's own amortized (doubling) growth strategy, which gives O(n) total allocation across n appends regardless of how many chunks they arrive in. Added a regression test with 5,000 single-event chunks that asserts on bytes allocated (via GC.GetAllocatedBytesForCurrentThread) rather than wall-clock time, to avoid timing-based flakiness while still reliably distinguishing O(n) amortized growth from O(n^2) per-chunk reallocation. Verified the test fails against the prior buggy code (~100 MB allocated for n=5,000, chunk copied) and passes against the fix (well under the 50 MB threshold). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fdfdae0-4987-4043-b522-7229e63810d2
| // Warm up once (JIT/type initialization) before measuring allocations for the real run. | ||
| await RunAsync(); | ||
|
|
||
| GC.Collect(); |
|
|
||
| GC.Collect(); | ||
| GC.WaitForPendingFinalizers(); | ||
| GC.Collect(); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs:287
- In the streaming path,
pastEventsstarts at the default capacity and grows via repeatedAddcalls. For large streamed chunks, this can still trigger multiple intermediate reallocations/copies within a single chunk. Sincechunk.Events.Countis available, consider callingList<T>.EnsureCapacity(pastEvents.Count + chunk.Events.Count)per chunk to reduce reallocations while still preserving the intended geometric growth behavior (and it would also align with the PR description’s mention of a per-chunk capacity hint).
pastEvents = new List<HistoryEvent>();
await foreach (P.HistoryChunk chunk in streamResponse.ResponseStream.ReadAllAsync(cancellation))
{
foreach (P.HistoryEvent protoEvent in chunk.Events)
{
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.
Summary
Replaces the lazy
Concat/Selecthistory accumulation inBuildRuntimeStateAsync(src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs) with directList<HistoryEvent>accumulation.Previously, each streamed history chunk extended a lazily-evaluated
IEnumerable<HistoryEvent>viaConcat. BecauseConcatchains are re-walked (and re-converted) from scratch every time they are enumerated or chained onto again, reconstructing history for a streamed orchestration was effectively quadratic in the number of chunks — a real problem for long-running orchestrations with large or heavily-chunked history.Change
RequiresHistoryStreaming) and non-streaming (PastEvents) branches now build a singleList<HistoryEvent>directly, appending converted events as they arrive (with a capacity hint sized per chunk in the streaming path) instead of chaining lazyConcat/Selectoperations.NewEventsare now added to the runtime state via a directforeach+runtimeState.AddEvent(...)instead of a lazySelectprojection.converter/ProtoUtils.ConvertHistoryEvent/ entity conversion), event ordering, new-vs-past runtime state classification, cancellation propagation, and the public API surface are all unchanged.Tests
Added two focused regression tests to
test/Worker/Grpc.Tests/GrpcDurableTaskWorkerTests.cs, following existing test conventions in the file:BuildRuntimeStateAsync_MultiChunkHistoryStream_MaterializesEventsInOrderAndClassifiesNewVsPast— streams history across three chunks and verifies events are materialized in order intoPastEvents,NewEventsclassification is preserved, andExecutionStartedEventis populated correctly.BuildRuntimeStateAsync_HistoryStreamCanceledMidStream_PropagatesCancellation— verifies cancellation observed mid-stream propagates asOperationCanceledExceptionrather than being swallowed.Targeted run:
→ 2/2 passed.
Full project suite:
→ 139/139 passed, 0 failures.
Fixes #772