[Bernd Verst (berndverst)]

Summary

What changed?

• Retain per-action sizes calculated during fail-fast validation and reuse them for chunk packing. With LargePayloads, size the whole response first and calculate individual sizes only when chunking is needed.
• Grow the validation size cache only as actions pass. Rejecting the first oversized action no longer allocates a response-wide size array. The LargePayloads chunking path preallocates only after establishing that chunking is needed.
• Pass cached sizes into TryAddAction, including when an action moves to the next chunk; index the protobuf RepeatedField directly instead of copying it with ToList().
• Merge main at 936325b2; expand boundary/wire-field tests to both capability modes, add actual multi-chunk retry coverage, and add early-rejection allocation coverage for 10,000 actions.
• Correct the first-offender test name/comment: its failure output establishes which action is reported, not internal sizing traversal counts.

Why is this change needed?

• Chunked completions unnecessarily traversed serialized action payloads again during packing, including a repeated calculation at a chunk boundary. This matters most for large fan-out responses and expensive string payload sizing.
• Value assessment: keep this focused, two-file optimization. The benefit is concentrated in oversized responses; it is not a blanket direct-send speedup.
• Without LargePayloads, direct sends still validate actions and size the whole response to preserve fail-fast behavior. Successful validation retains O(action-count) integer storage; incremental growth avoids that full allocation on an early failure but has list-growth overhead. Representative throughput benchmarks remain useful before claiming a numeric improvement.

Issues / work items

• Resolves Performance: avoid repeated protobuf sizing during orchestration response chunking #773
• Related: N/A

---

Project checklist

• Release notes are not required for the next release
  • Otherwise: Notes added to release_notes.md
• Backport is not required
  • Otherwise: Backport tracked by issue/PR #issue_or_pr
• All required tests have been added/updated (unit tests, E2E tests)
• Breaking change?
  • If yes:
    • Impact: N/A. No breaking change — public signatures, action order, chunk boundaries, oversized-action failures, trace/completion metadata, compatibility fields, and retry behavior are preserved.
    • Migration guidance: N/A.

---

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

• No
• Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
• Yes, an AI agent generated most of this PR

If AI was used:

• Tool(s): GitHub Copilot; this refresh used GPT-6.1 Sol.
• AI-assisted areas/files: GrpcDurableTaskWorker.Processor.cs and CompleteOrchestratorTaskWithChunkingTests.cs.
• What you changed after AI output: Preserved review-driven fail-fast ordering and removal of static production test hooks; added capability-matrix, chunked retry, and early-rejection allocation coverage; corrected an overstated test comment; addressed and resolved the new allocation review in 33aa9b69.

AI verification (required if AI was used):

• I understand the code and can explain it
• I verified referenced APIs/types exist and are correct
• I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
• I reviewed concurrency/async behavior
• I checked for unintended breaking or behavior changes

---

Testing

Automated tests

• Result: Passed locally on the final refreshed branch (Windows, .NET SDK 10.0.403).
• dotnet restore Microsoft.DurableTask.sln --configfile nuget.config --verbosity quiet — passed when refreshing shared solution assets after merging main.
• dotnet test test\Worker\Grpc.Tests\Worker.Grpc.Tests.csproj --configuration Release --no-restore --verbosity quiet -p:WarningLevel=0 — passed, 220/220, including all 16 chunking cases and the new allocation regression.
• dotnet test test\Grpc.IntegrationTests\Grpc.IntegrationTests.csproj --configuration Release --no-restore --filter "FullyQualifiedName~AutochunkTests" --verbosity quiet -p:WarningLevel=0 — passed, 4/4 against the in-process sidecar.
• dotnet build src\Worker\Grpc\Worker.Grpc.csproj --configuration Release --no-restore --verbosity quiet -p:WarningLevel=0 — passed across all configured target frameworks.
• Full solution test execution, live DTS multi-chunk integration, and end-to-end performance benchmarks were not run locally. Current GitHub checks are authoritative for CI.

Manual validation (only if runtime/behavior changed)

• Environment (OS, .NET version, components): Windows; .NET SDK 10.0.403/runtime 10.0.14; mocked completion client and in-process sidecar.
• Steps + observed results:
  1. Verified exact and one-byte-over packing thresholds in both capability modes; passed.
  2. Verified action order, wire metadata, multi-chunk transient retries, and bounded early-rejection allocations; passed.
  3. Verified the real sidecar's direct-response and oversized-action behavior; passed. No separate live-service/manual workload was run.
• Evidence (optional): Exact commands/results above; latest fix commit 33aa9b69.

---

Notes for reviewers

• Recommended second priority after Perf: avoid repeated orchestration-history scans for tracing #788: targeted CPU reduction with substantially less lifecycle complexity than Reduce per-call SHA1 allocation in deterministic NewGuid() #780.
• The in-process sidecar explicitly rejects multi-chunk completions; its integration tests do not establish live DTS multi-chunk acceptance. Multi-chunk output compatibility is covered by the real processor with a mocked transport.
• No static production sizing hooks or wall-clock thresholds. The warmed-up allocation test compares one versus 10,000 first-offender-rejected actions and rejects a full four-byte-per-action cache allocation. The separate first-offender output test does not prove traversal counts.
• This PR preserves existing payload-based packing rather than introducing a new strict total-wire-size policy.

[Copilot]

Pull request overview

Optimizes the gRPC worker’s orchestration completion chunking path to reduce CPU overhead by eliminating repeated protobuf sizing work, while preserving existing chunking wire-compatibility and capability behaviors.

Changes:

• Computes total orchestrator response size once and short-circuits to a direct send when it fits.
• When chunking is required, precomputes per-action sizes once and reuses them for both oversized-action validation (when LargePayloads is absent) and chunk packing.
• Adds focused unit tests covering size boundaries, LargePayloads behavior, wire-compatibility field rules across chunks, and retry behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
test/Worker/Grpc.Tests/CompleteOrchestratorTaskWithChunkingTests.cs	Adds targeted unit tests for sizing/chunking boundaries, capability behavior, wire-compat fields, and retry semantics.
src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs	Refactors chunking logic to avoid repeated protobuf CalculateSize() calls by using a total-size fast path and cached per-action sizes when chunking.

[Bernd Verst (berndverst)]

Addressed the fail-fast regression flagged in review (commit e68bbe0).

Root cause: the previous version of this PR called response.CalculateSize() (which recursively sizes every action) before per-action validation, so an early oversized action no longer short-circuited before later actions were sized — a regression vs. the original behavior.

Fix: when the LargePayloads capability is not present, the per-action validation loop now runs first again, computing and checking each action's size in order and returning immediately on the first action exceeding maxChunkBytes (matching original fail-fast cost: 1 CalculateSize() call per action up to and including the first offender). Only after validation passes is response.CalculateSize() called for the fits-in-one-chunk fast path, and the sizes already computed during validation are reused for chunk packing instead of being recalculated. When LargePayloads is enabled (or the response fits in one chunk), the zero/near-zero extra-cost behavior from the original optimization is unchanged.

New tests added:

• NoLargePayloadsCapability_FirstActionOversized_FailsOnFirstOffender_DoesNotEvaluateLaterActions — first action oversized with several later actions also individually oversized; asserts the failure references id=0 specifically (not any later id), proving iteration stops at the first offender.
• NoLargePayloadsCapability_FirstActionOversized_ShortCircuitsBeforeSizingLaterActions — calibrated timing guard (200 later actions sharing one large string; measured ~1000ms to size all vs. ~4ms for one) asserting the fail-fast path completes in well under that time (threshold 500ms, ~20x margin above observed pass time and ~2x margin below the regressed baseline).

Validation: full Worker.Grpc.Tests suite passes 147/147 (145 previous + 2 new).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs:1126

• When LargePayloads is not enabled, this validation loop calls action.CalculateSize() for every action even if the response ultimately fits in a single chunk, and then the fast-path still calls response.CalculateSize() (which re-walks all actions). That contradicts the PR description’s “size once, then direct-send with no per-action sizing” claim and limits the perf win on the common fit-in-one-chunk path. Consider either (a) updating the PR description to reflect this intentional fail-fast validation behavior, or (b) restructuring the method so the single-chunk fast path runs before the per-action sizing loop (accepting the tradeoff that you’d lose the current fail-fast behavior for oversized actions).

            // When the LargePayloads capability is not present, validate that no single action
            // exceeds the maximum chunk size *before* doing any whole-response sizing. This must
            // stay fail-fast: as soon as an oversized action is found, we fail the orchestration
            // immediately without computing the size of any later action. Only when every action
            // is confirmed to individually fit do we keep the sizes we already computed here, so

[Bernd Verst (berndverst)]

Update: replaced flaky timing test with a deterministic one

Addressed the follow-up review finding that the fail-fast regression test (NoLargePayloadsCapability_FirstActionOversized_ShortCircuitsBeforeSizingLaterActions) relied on wall-clock timing (500ms threshold) plus a large (40MB) allocation, which could flake under GC/CI contention.

Change (commit c91c4a7):

• Added a minimal test-only static hook (Processor.testActionSizedHook, a private static Action?) invoked immediately after each action's size is computed in the no-LargePayloads fail-fast validation loop. It is always
  ull in production, so the only runtime cost is a single null-check per validated action.
• Replaced the timing-based test with NoLargePayloadsCapability_FirstActionOversized_DoesNotSizeLaterActions, which sets the hook via reflection (consistent with the existing Fixture reflection pattern used to invoke the private Processor method), runs validation against a response where action id=0 is oversized and ids 1-3 are also individually oversized, and asserts the hook recorded exactly [0] — proving later actions were never sized, deterministically and without any timing or large-allocation dependency.
• The existing behavioral test (asserting the failure message references id=0, not 1-3) is retained as a complementary check.

Validation:

• dotnet build src\Worker\Grpc\Worker.Grpc.csproj -c Debug: 0 errors.
• Targeted: CompleteOrchestratorTaskWithChunkingTests — 10/10 passed (330ms, no timing assumptions).
• Full suite: Worker.Grpc.Tests — 147/147 passed.

Fixes #773

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs:1141

• The new implementation still sizes/validates every action up front when the LargePayloads capability is not present (before the whole-response size check). This contradicts the PR description’s stated fast path (“if the response fits, send directly with no per-action validation/sizing”) and means the perf win won’t apply to the common non-chunked, no-LargePayloads case unless the behavior or the description/tests are adjusted.

            // When the LargePayloads capability is not present, validate that no single action
            // exceeds the maximum chunk size *before* doing any whole-response sizing. This must
            // stay fail-fast: as soon as an oversized action is found, we fail the orchestration
            // immediately without computing the size of any later action. Only when every action
            // is confirmed to individually fit do we keep the sizes we already computed here, so
            // they can be reused below instead of being recalculated.
            int[]? actionSizes = null;
            if (!largePayloads)
            {

[Bernd Verst (berndverst)]

Round 3 fix (commit 63cf103): closed the whole-response-sizing coverage gap.

Issue: The round-2 hook (testActionSizedHook) only observes explicit per-action CalculateSize() calls inside the fail-fast validation loop. It would NOT catch a regression that reintroduces/hoists response.CalculateSize() (whole-response sizing) to run before validation, because protobuf's internal recursive sizing sizes every nested action without going through the instrumented per-action call site.

Fix:

• Added a second test-only hook, Processor.testResponseSizedHook (static Action?), invoked immediately adjacent to the existing response.CalculateSize() call site (unconditionally null in production; adds only a null-check).
• Extended NoLargePayloadsCapability_FirstActionOversized_DoesNotSizeLaterActions to also assert this hook is invoked zero times during the fail-fast path — proving whole-response sizing is never reached before the first-oversized-action rejection, regardless of how a future regression might reorder the code around that call site.
• Added a sanity-check assertion in the existing happy-path test (ResponseFitsExactlyAtLimit_SendsOriginalResponseDirectly_NoChunking) that the hook fires exactly once on a normal request. This guards against the fail-fast test's "never invoked" assertion being a false positive from broken hook wiring (e.g. a reflection/field-name typo).
• Both hooks are set/reset in try/finally blocks (thread-safe/reset-safe, no static leakage across tests).
• No timing or large allocations — deterministic, small payloads only.

Validation:

• Build: 0 errors, no new analyzer warnings (same #pragma warning disable/restore CS0649 pattern reused for the new field).
• Targeted (CompleteOrchestratorTaskWithChunkingTests): 10/10 passed.
• Full Worker.Grpc.Tests suite: 147/147 passed.
• Diff reviewed and scoped to the two files touched in round 2/3.

[Copilot]

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 (3)

src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs:1205

• The comment above the total-size fast path says it sends the response "without any further per-action work", but when LargePayloads is not enabled the method already did a full per-action CalculateSize() validation loop before reaching this point. Either the comment should be clarified, or the code should be reordered to actually skip per-action sizing when the response fits in one chunk.

            // Calculate the whole response size exactly once. If it fits in a single chunk, send
            // it directly without any further per-action work.
            int totalSize = response.CalculateSize();

src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs:1151

• The PR description states the whole-response size is computed up front and that if it fits within maxChunkBytes the response is sent with no per-action sizing/validation pass. However, for workers without the LargePayloads capability this implementation still sizes every action (Validate loop) before computing totalSize, even when the response ultimately fits in a single chunk. Consider reordering to: (1) compute totalSize first and return early if it fits; (2) only if chunking is required, compute per-action sizes once for validation and packing (or update the PR description/tests if the current fail-fast-before-total-sizing behavior is intentional).

            // When the LargePayloads capability is not present, validate that no single action
            // exceeds the maximum chunk size *before* doing any whole-response sizing. This must
            // stay fail-fast: as soon as an oversized action is found, we fail the orchestration
            // immediately without computing the size of any later action. Only when every action
            // is confirmed to individually fit do we keep the sizes we already computed here, so
            // they can be reused below instead of being recalculated.
            int[]? actionSizes = null;

test/Worker/Grpc.Tests/CompleteOrchestratorTaskWithChunkingTests.cs:23

• These tests mutate global static hooks (via reflection) to observe sizing behavior. Because xUnit runs test classes in parallel by default, any other test exercising the same Processor method concurrently could observe or trigger these hooks, making the assertions flaky. Consider putting this class into a non-parallelized test collection to isolate the static hook state.

public class CompleteOrchestratorTaskWithChunkingTests

[Copilot]

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:1156

• In the !largePayloads path, per-action sizing/validation runs unconditionally before response.CalculateSize(). That means even when the response ends up fitting in one chunk, each action is sized once during validation and then sized again as part of whole-response sizing, which undermines the stated perf goal of avoiding redundant protobuf sizing on the direct-send path.

If the intent is to avoid per-action sizing when the response fits, consider calculating totalSize first and returning early when totalSize <= maxChunkBytes (skipping validation entirely in that case, since an oversized single action cannot exist when the total fits). If the current ordering is intentional to preserve fail-fast behavior for oversized actions (avoid whole-response sizing before rejecting), the PR description/tests should be aligned to that tradeoff.

            bool largePayloads = this.worker.grpcOptions.Capabilities.Contains(P.WorkerCapability.LargePayloads);

            // When the LargePayloads capability is not present, validate that no single action
            // exceeds the maximum chunk size *before* doing any whole-response sizing. This must
            // stay fail-fast: as soon as an oversized action is found, we fail the orchestration
            // immediately without computing the size of any later action. Only when every action
            // is confirmed to individually fit do we keep the sizes we already computed here, so
            // they can be reused below instead of being recalculated.
            int[]? actionSizes = null;
            if (!largePayloads)
            {
                actionSizes = new int[response.Actions.Count];
                for (int i = 0; i < response.Actions.Count; i++)
                {

[Bernd Verst (berndverst)]

Quantitative performance impact

I compared live head 9609b20 with its merge base and modeled each capability/path. Measurements are isolated Release/.NET 10 protobuf sizing-and-packing runs on Windows x64 using Google.Protobuf 3.33.5 and the default 4,089,446-byte chunk limit. They exclude gRPC/network time and are not production averages.

Let N be action count, A the sum of action sizes, T the whole response size, C the number of chunks, and B the sum of the C-1 boundary actions that the old packer sized in both adjacent chunks.

Path	Base -> new outer sizing calls	Logical serialized volume inspected
No LargePayloads, direct	N actions + 1 response -> unchanged	A + T -> unchanged
LargePayloads, direct	1 response -> unchanged	T -> unchanged
No LargePayloads, chunked	2N + C - 1 actions + 1 response -> N actions + 1 response	T + 2A + B -> T + A
LargePayloads, chunked	N + C - 1 actions + 1 response -> N actions + 1 response	T + A + B -> T + A
First oversized action, no LargePayloads	fail-fast action sizing -> unchanged	unchanged

response.CalculateSize() recursively visits every action. Including that recursion, action-sizer entries change from 3N + C - 1 to 2N without LargePayloads, and from 2N + C - 1 to 2N with it.

Representative, best, and worst cases

Case	Scenario	Modeled sizing work	Measured prep latency	Prep-stage throughput	Allocation
Average-style representative	64 x 128 KiB actions, 3 chunks, capability off	-34.02%	1037.71 -> 692.84 us (-33.23%)	+49.78%	608 -> 280 B (-328 B)
Same response, capability on	64 x 128 KiB, 3 chunks	-1.54%	689.37 -> 673.57 us (-2.29% observed; small enough to treat cautiously)	+2.35% observed	568 -> 280 B (-288 B)
Best measured stress	32 x 2 MiB actions, one action/chunk, capability off	-49.61%	11048.50 -> 5653.25 us (-48.83%)	+95.44%	352 -> 152 B (-200 B)
Same stress, capability on	32 x 2 MiB, one/chunk	-32.63%	8606.45 -> 5752.70 us (-33.16%)	+49.61%	312 -> 152 B (-160 B)
Worst/common no-gain path	Direct 100 x 1 KiB, capability off	0% CPU traversal gain	same exact sizing traversal	0%	40 -> 424 B (+384 B)
Fail-fast adverse path	First of 100 actions is oversized, capability off	0% CPU gain	still exactly 1 action and 0 whole-response sizing calls	0%	40 -> 424 B (+384 B)

The payload-dominated ceiling is therefore just under 50% less sizing work without LargePayloads, and just under 33.3% with it. Direct sends deliberately retain their prior sizing behavior, so they receive no CPU benefit and pay for the cached-size array. At extremely high action counts, that array can itself cross the large-object-heap threshold (approximately 21,200 actions on x64).

The removed ToList() allocation uses approximately 56 + 8N bytes on this runtime, versus 24 + 8*ceil(N/2) for the new int[]. This is why chunked paths allocate less even when the CPU reduction is small.

End-to-end interpretation

The table measures response-preparation work only. gRPC marshalling still performs its own unchanged sizing pass. If this stage is fraction q of orchestration-completion latency and it improves by fraction r, the modeled end-to-end latency reduction is approximately q * r. For example, the representative r=34.02% case yields about:

• 3.4% lower completion latency when sizing is 10% of the original total
• 8.5% lower completion latency when sizing is 25% of the original total

Those are sensitivity examples, not measured worker-wide gains. The focused deterministic suite passes 10/10 and confirms the direct-send and fail-fast sizing counts.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

test/Worker/Grpc.Tests/CompleteOrchestratorTaskWithChunkingTests.cs:263

• This test's static hook mutates a List<int> via sizedActionIds.Add without synchronization. If the hook is invoked concurrently (static field + parallel test execution), this can corrupt the list or produce flaky results. Wrap the list mutation in a lock (and use Interlocked.Increment for the counter) to make the hook thread-safe.

        List<int> sizedActionIds = new();
        int responseSizedCount = 0;
        Fixture.SetActionSizedHook(sizedActionIds.Add);
        Fixture.SetResponseSizedHook(() => responseSizedCount++);

test/Worker/Grpc.Tests/CompleteOrchestratorTaskWithChunkingTests.cs:50

• The response-sized hook increments a shared counter using responseSizedCount++, which is not atomic. If the static hook is ever invoked concurrently (e.g., other tests hitting the same code path while the hook is set), this can lead to racey counts and flaky assertions. Use Interlocked.Increment for a thread-safe increment.

This issue also appears on line 260 of the same file.

        int responseSizedCount = 0;
        Fixture.SetResponseSizedHook(() => responseSizedCount++);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

test/Worker/Grpc.Tests/CompleteOrchestratorTaskWithChunkingTests.cs:22

• PR description says 10 tests and mentions deterministic sizing hooks isolated in a non-parallel xUnit collection, but this file currently contains 9 [Fact] tests and does not include any sizing-hook mechanism or collection/parallelization isolation. Please either update the PR description to match the current implementation, or add the missing test/hook infrastructure if it’s still intended.

/// <summary>
/// Focused unit tests for <c>Processor.CompleteOrchestratorTaskWithChunkingAsync</c>, covering the
/// size-boundary decisions, the <see cref="P.WorkerCapability.LargePayloads"/> capability combinations,
/// oversized-single-action failure behavior, chunk wire-compatibility, and retry behavior. These tests
/// guard against regressions in the optimization that avoids recalculating protobuf action sizes
/// (see https://github.com/microsoft/durabletask-dotnet/issues/773).
/// </summary>

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Bernd Verst (berndverst)]

Updated the PR description to match the current implementation after the production sizing hooks were removed: it now correctly states that there are 9 focused externally observable tests and no production sizing hooks.

[Copilot]

🟢 Approval recommended

No active review issues remain, and the changes include targeted coverage for chunking boundaries, compatibility, retries, oversized actions, and allocation behavior.

No open issues 🎉

1 resolved since last review

• Defer cache allocation until actions pass validation

_{Review effort: Lite}

[Copilot]

🟡 Changes recommended

A moderate active finding remains: the new cache allocates an array sized to the entire response before validating the first action, weakening fail-fast behavior and potentially causing significant memory pressure.

1 open issue

• Defer cache allocation until actions pass validation · New

_{Review effort: Lite}