Skip to content

Perf: avoid repeated orchestration-history scans for tracing - #788

Merged
Bernd Verst (berndverst) merged 14 commits into
mainfrom
berndverst-fix-trace-history-scan-perf
Nov 3, 2026
Merged

Bernd Verst (berndverst) merged 14 commits into
mainfrom
berndverst-fix-trace-history-scan-perf

Conversation

@berndverst

@berndverst Bernd Verst (berndverst) commented Jul 24, 2026 •

Copy link
Copy Markdown
Member

Summary

What changed?

  • Gate tracing-only execution-start discovery and completion correlation behind TraceHelper.HasListeners.
  • Replace repeated linear history scans with a work-item-local TraceHistoryEventLookup. Both scheduling dictionaries are initialized directly and populated together on the first lookup in one lazy history pass. No NewEvents pre-scan, per-ID registration, duplicate-tracking sets, or custom duplicate exceptions.
  • Address the latest six review threads in 944d4f80: remove obsolete exception documentation and rewrite lookup tests around the current two-index behavior. Extract the existing completion/timer emission loop into the internal TraceHelper.EmitTraceActivitiesForNewEvents helper so tests directly verify zero history enumeration without listeners using strict enumerable mocks, rather than malformed history as a proxy.
  • Retain native Dictionary.Add fail-fast behavior for impossible duplicate scheduling IDs. Separately verify logged ArgumentException, work-item abandonment, and orchestration Activity disposal.
  • Preserve main's rewind/streamed-history integration from b7cf0868, including materialized history, start-event validation without listeners, and fresh rewind trace context.
  • Cover success/failure completions, timer-only lazy behavior, both lookup orders, one shared history pass, cached hits/misses, event-type isolation, and exact integration parent-span relationships.

Why is this change needed?

  • Previous tracing correlation grew as O(new completions × past events), even with tracing disabled. The fast path removes tracing-specific scans; listener-enabled correlation becomes O(past events + new events).
  • Review-directed tradeoff: both scheduling types are now indexed on the first lookup, including IDs/types not referenced by the current completion batch. Temporary storage is O(all past scheduling events). No past-history enumeration occurs for correlation when there are no scheduling lookups.
  • Fresh profiling below measures this implementation. Earlier requested-ID/demand-shaped measurements in PR comments and the prior description are historical, not current-head results.

Issues / work items


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: Intentional behavior for malformed duplicate scheduling-ID history, following the reviewed one-to-one invariant. With listeners, the first scheduling lookup populates both indexes; duplicate IDs within either scheduling-event type raise native ArgumentException and abandon the invalid work item, including unqueried IDs/types. Valid history and public APIs remain unchanged.
      • Migration guidance: No action for valid orchestrations. Diagnose/repair invalid duplicate scheduling history rather than relying on ambiguous first/last-match tracing behavior.

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 follow-up used GPT-6.1 Sol.
  • AI-assisted areas/files: Shared tracing helpers/index, worker processor, lookup/processor tests, tracing integration tests, and session-local benchmark harness/analysis. This follow-up modifies five tracing/worker/test files and reruns the saved whole-worker benchmark harness against current code.
  • What you changed after AI output: Applied direct dictionary initialization, removed stale documentation and demand-shaped test setup, replaced exception-based no-listener inference with direct enumeration assertions, and retained native duplicate-failure/Activity cleanup coverage. Corrected the timer fixture during validation. Documented measured allocation increases rather than reusing older allocation claims. Prior work retained main's rewind integration and the review-directed fail-fast invariant.

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 for 944d4f80, Windows, .NET SDK 10.0.403/runtime 10.0.14, in the existing PR checkout. The unrelated blob-payload checkout was left unchanged.
  • dotnet test test\Worker\Grpc.Tests\Worker.Grpc.Tests.csproj --configuration Release --no-restore --verbosity quiet -p:WarningLevel=0 — passed, 225/225.
  • dotnet test test\Grpc.IntegrationTests\Grpc.IntegrationTests.csproj --configuration Release --no-restore --filter "FullyQualifiedName~TracingIntegrationTests|FullyQualifiedName~OrchestrationPatterns" --verbosity quiet -p:WarningLevel=0 — passed, 55/55, including all 8 tracing tests.
  • dotnet build src\Worker\Grpc\Worker.Grpc.csproj --configuration Release --no-restore --verbosity quiet -p:WarningLevel=0 — passed across all configured target frameworks; 0 errors, 20 repository/source-link metadata warnings.
  • git diff --check — passed before commit.
  • Full solution tests, live DTS rewind workloads, and live-service throughput profiling were not run locally. GitHub checks for the new pushed revision are separate.

Manual validation (only if runtime/behavior changed)

  • Environment (OS, .NET version, components): Windows 11 Hyper-V VM; AMD EPYC 7763, 32 logical processors; .NET SDK 10.0.403/runtime 10.0.14; mocked transport plus real in-process sidecar integration tests.
  • Steps + observed results:
    1. Verified direct no-listener enumeration assertions, success/failure correlation, timer-only lazy behavior, and single-pass cached lookup behavior; passed.
    2. Verified native duplicate rejection in either scheduling index, logged failure, abandonment, and Activity cleanup; passed.
    3. Reran matched whole-worker profiling sequentially against saved main and a freshly built current worker; all 10 benchmark fixtures completed with expected orchestration outputs.
  • Evidence (optional): Commit 944d4f80b843091e7635dcb4965fe122d440d926; session-local harness, full BenchmarkDotNet JSON, logs, environment/assembly fingerprints, source patch, and comparison JSON retained.

Fresh whole-worker performance profiling — November 3, 2026

These measurements cover the implementation in 944d4f80. Baseline is the saved main 936325b2 assembly bundle, freshly rerun in the same environment. Current worker was rebuilt from the exact production sources committed in this follow-up. Worker SHA256: main 204F1757C224A361CF1F5070257EC6A2A5FA94DFF7272299F963B18D99D0DED6; current 134FC4266B2CCB00CBFC6CFC4D5AC2CCAD31034ABE216B54C67B07CA26D53A01.

BenchmarkDotNet 0.15.8, Release, in-process toolchain, workstation GC, tiered compilation/PGO disabled, 7 warmups, 30 measured iterations, 500 ms target. Baseline/current ran sequentially, not concurrently. Error is the 99.9% confidence-interval half-width. Allocation is managed KiB per processor turn, not retained memory.

The benchmark invokes real OnRunOrchestratorAsync, including history conversion, fan-in replay, tracing, and response construction. Fixture setup/reflection binding, service latency, and gRPC transport serialization are outside timing; the completion RPC is an already-completed fake.

Scheduled tasks / new completions Listeners Main mean +/- error (ms) Current mean +/- error (ms) Time reduction Main -> current allocation (KiB/turn)
64 / 8 None 0.2118 +/- 0.0041 0.1844 +/- 0.0014 12.9% 133.01 -> 130.70
4,096 / 64 None 31.4157 +/- 1.2850 17.9011 +/- 0.6357 43.0% 8,202.88 -> 8,188.24
4,096 / 64 Recording 29.3365 +/- 0.6226 14.1138 +/- 0.2046 51.9% 8,255.91 -> 8,685.48
4,096 / 512 None 85.3452 +/- 0.4980 17.6119 +/- 0.3445 79.4% 8,300.86 -> 8,184.48
4,096 / 512 Recording 88.6950 +/- 0.6378 20.1920 +/- 0.7293 77.2% 8,718.39 -> 9,067.22
  • The listener-enabled full-history index trades additional temporary allocation for less CPU: approximately +430 KiB/turn for the 64-completion batch and +349 KiB/turn for the 512-completion batch. No-listener allocation decreases in every measured shape.
  • These are local processor-turn measurements, not DTS capacity forecasts. Small/single early-match sub-orchestration cases may regress because the former first-match search could exit early while the index scans all history. This run covers task fan-in with materialized history, not streaming, rewind, mixed/sub-orchestration/failure batches, or production workload distributions.

Reproduction ($perf is this follow-up's session-local evidence directory; $old is the prior evidence directory containing the unchanged saved main bundle):

dotnet restore "$perf\BenchmarkHarness.csproj" --configfile .\nuget.config --verbosity quiet
dotnet build "$perf\BenchmarkHarness.csproj" --configuration Release --no-restore --output "$perf\bin\current" --verbosity quiet -p:WarningLevel=0
$env:DOTNET_TieredCompilation = '0'
$env:DOTNET_TieredPGO = '0'
$env:DOTNET_gcServer = '0'
dotnet "$old\bin\main\BenchmarkHarness.dll" --inProcess --filter '*TraceProcessing*' --warmupCount 7 --iterationCount 30 --iterationTime 500 --artifacts "$perf\main-fresh"
dotnet "$perf\bin\current\BenchmarkHarness.dll" --inProcess --filter '*TraceProcessing*' --warmupCount 7 --iterationCount 30 --iterationTime 500 --artifacts "$perf\current-fresh"

All commands above passed. Reproduction elsewhere requires the saved harness/baseline bundle and adjustment of the harness project-reference path.

Historical evidence: November 1 measurements for requested-ID revision 81d05c10 and the earlier performance package remain historical. They must not be used as timing/allocation measurements of ed2109fa or 944d4f80. No fresh profile was run for ed2109fa; the fresh table above replaces the old table for current-head evaluation.


Notes for reviewers

  • All six remaining review threads are addressed in 944d4f80; the current implementation does not pre-scan new events or select scheduling-event types before initializing the dictionaries.
  • No-listener tests directly verify that the correlation helper never enumerates either history input. The separate worker fast-path test uses valid history; duplicate-history tests cover the actual fail-fast/cleanup contract.
  • Both scheduling types are indexed on the first lookup, including unqueried IDs/types. Timer-only/no-completion turns do not enumerate past history for correlation.
  • HasListeners is not a sampling-decision check: a listener sampling nothing can still cause indexing work.
  • Public signatures, wire fields, serialization settings, and valid-history replay output remain unchanged. The optimization does not skip history retrieval or validation required for execution/rewind.

Breaking Change

Type: behavioral (invalid duplicate scheduling-ID histories only).
Impact: With listeners enabled, a scheduling lookup populates both dictionaries; duplicate IDs within either scheduling-event type throw native ArgumentException and abandon the work item, including unqueried IDs/types. No effect on valid histories or public API/wire compatibility.
Migration: No action for valid histories. Diagnose/repair invalid duplicate scheduling histories instead of relying on ambiguous first/last-match tracing correlation.

OnRunOrchestratorAsync previously scanned orchestration history to
correlate tracing events unconditionally, and for every qualifying
new event it rescanned the full past-events list to find the
originating TaskScheduled/SubOrchestrationInstanceCreated event -
O(new events x past events) work, even when no tracing listener was
registered.

- Add TraceHelper.HasListeners, a cheap check backed by
  ActivitySource.HasListeners(), and gate all tracing-correlation
  work in OnRunOrchestratorAsync behind it so nothing is scanned when
  no listener is registered.
- Replace the LINQ Concat-based scan for the ExecutionStarted event
  with a single-pass local function.
- Add TraceHistoryEventLookup, which builds one dictionary index per
  work item (built lazily, once) for O(1) repeated lookups instead of
  rescanning PastEvents per new event, while preserving the exact
  original first-wins (SubOrchestrationInstanceCreated) and last-wins
  (TaskScheduled) semantics for duplicate event IDs.

No public API changes; trace output, error handling, and replay
determinism are unaffected.

Adds regression tests for the no-listener fast path, first/last-wins
correlation semantics end-to-end, and direct unit tests for
TraceHistoryEventLookup.

Fixes #769

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9c538f3e-308d-48a3-af7f-b3baf90e97b2
Copilot AI lite review requested due to automatic review settings July 24, 2026 22:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves the gRPC worker’s tracing correlation performance by avoiding unnecessary orchestration-history scans when tracing is disabled and by indexing past history events for O(1) correlation lookups when tracing is enabled. This aligns with the SDK’s goal of keeping worker-side orchestration processing efficient, especially for long-running instances with large histories.

Changes:

  • Added a cheap listener check (TraceHelper.HasListeners) and gated trace-correlation work in OnRunOrchestratorAsync behind it.
  • Replaced repeated past-history scans with a per-work-item indexed lookup (TraceHistoryEventLookup) that preserves prior first/last-wins semantics for duplicate event IDs.
  • Added unit and worker-level tests covering listener/no-listener behavior and duplicate-ID correlation semantics.

Reviewed changes

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

Show a summary per file
File Description
test/Worker/Grpc.Tests/TraceHistoryEventLookupTests.cs Adds focused unit tests for the new indexed lookup, including duplicate-ID first/last-wins semantics and filtering by event type.
test/Worker/Grpc.Tests/GrpcDurableTaskWorkerTests.cs Adds end-to-end worker tests for the no-listener fast path and for correlation semantics under a real ActivityListener.
src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs Gates tracing correlation behind HasListeners, replaces LINQ concat scan with a single-pass helper, and uses the indexed lookup for repeated correlation.
src/Shared/Grpc/Tracing/TraceHistoryEventLookup.cs Introduces a lazily-built, cached dictionary index for past events to avoid O(N×M) repeated scans.
src/Shared/Grpc/Tracing/TraceHelper.cs Exposes HasListeners backed by ActivitySource.HasListeners() to allow callers to skip trace-correlation work when tracing is disabled.

Copilot AI review requested due to automatic review settings July 27, 2026 18:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@berndverst

Copy link
Copy Markdown
Member Author

Quantitative performance impact

I compared live head d223fc2 with its merge base in a focused Release/.NET 10 harness on Windows x64 (AMD EPYC 7763). It used the actual TraceHistoryEventLookup/TraceHelper.HasListeners code and the exact old lookup algorithm. Activity creation, the rest of worker execution, and gRPC were excluded, so these are correlation-stage measurements rather than fleet-wide orchestration averages.

Let H be past-event count and N_t/N_s qualifying task/sub-orchestration completion counts.

Path Old past-event inspections New work
No tracing listener execution-start search + up to N_t*H + sum(sub match positions) one HasListeners() check; zero history enumeration
Listener, task completions N_t*H one H-event task index + N_t dictionary lookups
Listener, sub-orchestration completions sum(sub match positions) one H-event sub index + N_s dictionary lookups

Representative, best, and adverse cases

“Representative” below means a selected ordinary shape, not production telemetry.

Case Listener / shape Correlation latency, old -> new Allocation/work item Interpretation
Average-style representative Off; H=100, 5 task completions 4.40 us -> ~6 ns 1,056 -> 0 B The history work is eliminated; the new value is near harness overhead, so the ratio is not meaningful
Listener-on ordinary task case H=100, 5 task completions 4.38 -> 2.00 us (-54.3%) 1,056 -> 4,704 B (+3,648 B) Faster CPU path, higher temporary allocation
Listener-on ordinary mixed case H=100, 3 task + 3 early sub matches 2.55 -> 3.43 us (~+34.5%) 1,024 -> 4,280 B (+3,256 B) Small/early-match histories can regress
Best measured large replay Off; H=10,000, 1,000 task completions 79.8 ms -> near harness overhead 160,256 -> 0 B Removes the entire quadratic correlation scan
Listener-on large fan-out H=10,000, 1,000 task completions 80.46 ms -> 333.5 us (-99.585%, ~241x) 160,256 -> 451,592 B (+291,336 B) Large CPU/latency gain traded for a temporary index
Listener-on mixed fan-out H=10,000, 500 task + 500 sub completions 36.98 ms -> 490.7 us (-98.673%, ~75x) 128,256 -> 431,438 B (+303,182 B) Same tradeoff with two lazy indexes
Constructed adverse case Listener on; one sub match at position 2 of H=10,000 166 ns -> 95.7 us (~576x as long) 352 -> 296 B Old FirstOrDefault stops at event 2; new code builds the full lazy index

The no-listener ~6 ns figures are close to benchmark-harness cost and should be read as “the scan was removed,” not as a credible millions-of-operations throughput claim.

Memory and GC

The lazy indexes reference existing events; they do not copy protobuf messages. On this x64 runtime, estimated incremental dictionary storage while live is approximately 28-56 B per unique indexed ID, including capacity slack/array headers but excluding the existing event objects.

  • 50 task entries: about 2.7 KiB retained, 4.7 KiB cumulatively allocated
  • 5,000 task entries: about 236 KiB retained, 452 KiB allocated
  • Two 3,333-entry indexes: about 227 KiB retained, 431 KiB allocated

Thus:

  • No listener: both scan CPU and correlation allocations fall to zero.
  • Listener + repeated lookups: CPU falls from O(N*H) to O(H+N), but temporary memory becomes O(unique history IDs).
  • Listener + one early sub lookup: CPU can regress from O(1) expected early exit to O(H) index construction.
  • A listener that samples every activity out still makes HasListeners() true and pays indexing costs.

Whole-worker throughput interpretation

Correlation-stage speedup does not translate directly to orchestration throughput. Using Amdahl's law:

  • If the old correlation work is 10% of turn CPU and the new path removes it, modeled turn throughput improves by about 11.1%.
  • If a large fan-out replay spends 50% of turn CPU in old correlation scans and that work becomes negligible, the modeled ceiling is about 2x turn throughput.

Those are sensitivity examples. Actual gains require telemetry for listener prevalence and the distributions of H, qualifying new events, event mix, and match positions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 01dc5b7c-c742-45b7-ae8c-5cda9bcfbd2c
Copilot AI review requested due to automatic review settings August 4, 2026 19:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 01dc5b7c-c742-45b7-ae8c-5cda9bcfbd2c
Copilot AI review requested due to automatic review settings August 13, 2026 01:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (3)

test/Worker/Grpc.Tests/TraceHistoryEventLookupTests.cs:47

  • This test currently asserts that duplicate SubOrchestrationInstanceCreated event IDs throw, but the prior linear scan behavior (and the PR description) expects first-wins semantics for duplicate IDs to preserve correlation behavior.
        act.Should().Throw<InvalidOperationException>()
            .WithMessage("*'SubOrchestrationInstanceCreated'*event ID '2'*");
    }

test/Worker/Grpc.Tests/TraceHistoryEventLookupTests.cs:28

  • These tests currently assert that duplicate event IDs throw. Per the original implementation (TaskScheduled = last-wins, SubOrchestrationInstanceCreated = first-wins) and the PR description, duplicates should be tolerated with defined first/last-wins behavior rather than causing failures during tracing correlation.

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

        act.Should().Throw<InvalidOperationException>()
            .WithMessage("*'TaskScheduled'*event ID '1'*");
    }

test/Worker/Grpc.Tests/GrpcDurableTaskWorkerTests.cs:377

  • These two tests rely on TraceHistoryEventLookup throwing on duplicate event IDs (and assert the work item is abandoned when a listener is registered). That behavior conflicts with the stated goal of preserving the original duplicate-ID correlation semantics (TaskScheduled last-wins, SubOrchestrationInstanceCreated first-wins). If duplicate IDs are expected to be tolerated, these tests should be reworked to validate the no-listener fast path without depending on duplicate-ID exceptions, and the listener-enabled path should validate the preserved first/last-wins correlation rather than abandonment.
        // Arrange: duplicate event IDs cause TraceHistoryEventLookup to throw when it builds an index. If this
        // invalid history still completes, the no-listener fast path did not invoke either lookup method.
        TraceHelper.HasListeners.Should().BeFalse();

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 01dc5b7c-c742-45b7-ae8c-5cda9bcfbd2c
Copilot AI review requested due to automatic review settings August 13, 2026 02:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

test/Grpc.IntegrationTests/TracingIntegrationTests.cs:97

  • This new test is named/positioned as validating span correlation, but the assertions only check that client Activities with specific OperationName values exist. That doesn’t actually validate the correlation behavior the PR is changing (i.e., that the server activity/sub-orchestration spans are parented to the correct client scheduled spans, and that the two scheduled operations remain distinct). Strengthen the assertions to check ParentSpanId relationships (similar to the other tests in this file) so the test fails if the history lookup mis-correlates events.
        // Assert
        metadata.RuntimeStatus.Should().Be(OrchestrationRuntimeStatus.Completed);
        activities.Should().ContainSingle(
            activity => activity.Kind == ActivityKind.Client
                && activity.Source.Name == CoreActivitySourceName

Bernd Verst added 2 commits August 12, 2026 19:33
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 01dc5b7c-c742-45b7-ae8c-5cda9bcfbd2c
Copilot AI review requested due to automatic review settings August 13, 2026 03:05
@berndverst

Copy link
Copy Markdown
Member Author

Updated quantitative impact after demand-shaped indexing

This supersedes the listener-on index measurements in my earlier comment. Head 3830a05 now pre-registers only correlation IDs referenced by the current work item's NewEvents, scans PastEvents once for both scheduling-event types, and retains only those requested IDs.

I measured the old repeated-scan algorithm, the prior full-history index, and the current demand-shaped index in a focused Release/.NET 10 harness on Windows x64 (AMD EPYC 7763), using real protobuf HistoryEvent objects. These numbers isolate correlation work; they are not whole-worker throughput measurements.

Scenario Shape Old repeated scan Prior full index Current demand-shaped index Current vs old
Average-style task case H=100, 50 scheduled tasks, 5 completions 3.160 us / 832 B 1.377 us / 4,736 B 0.793 us / 616 B -74.9% latency, -26.0% allocation, ~4.0x correlation throughput
Average-style mixed case H=100, 33 task + 33 sub events, 3 lookups each 2.516 us / 1,024 B 1.845 us / 4,344 B 0.888 us / 648 B -64.7% latency, -36.7% allocation, ~2.8x throughput
Best listener-on fan-in H=10,000, 5,000 scheduled tasks, 1,000 completions 54.33 ms / 160,034 B 147.1 us / 451,641 B 80.1 us / 102,368 B -99.853% latency, -36.0% allocation, ~678x throughput
Large mixed fan-in H=10,000, 3,333 task + 3,333 sub events, 500 lookups each 35.57 ms / 160,066 B 210.4 us / 431,611 B 91.1 us / 96,408 B -99.744% latency, -39.8% allocation, ~390x throughput
Constructed worst case listener on; one sub match at position 2 of H=10,000 89.8 ns / 184 B 35.67 us / 320 B 39.00 us / 352 B ~434x latency, +168 B

Compared with the prior full index, the current large task case reduces index latency another 45.5% and allocation 77.3%; the large mixed case reduces latency 56.7% and allocation 77.7%. Storage is now O(unique correlation IDs in NewEvents) rather than O(all scheduling IDs in PastEvents).

The no-listener path is unchanged from the earlier measurement: it performs one HasListeners() check and zero tracing-history enumeration or correlation allocation. The earlier representative no-listener case (H=100, 5 task completions) fell from 4.40 us and 1,056 B to approximately harness overhead and 0 B; the large no-listener replay (H=10,000, 1,000 completions) removes roughly 79.8 ms of correlation work.

The constructed single-early-sub case remains the adverse boundary. The old FirstOrDefault could return at position 2, while the reviewer-requested one-to-one invariant requires scanning the applicable history to detect a later duplicate and fail rather than silently selecting one. That O(H) validation cost cannot retain the old early-exit complexity while also guaranteeing duplicate detection. The current design removes the avoidable all-history storage and keeps that cost confined to listener-enabled tracing.

Reciprocal latency is used only as a correlation-stage throughput proxy. Whole-worker gains depend on the fraction of turn CPU previously spent in tracing correlation, listener prevalence, history sizes, and completion-batch distributions.

Preserve rewind and streamed-history behavior, verify parent-span correlation, and close abandoned work-item traces on lookup failure.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are identified in the supplied evidence.

No open issues 🎉

Review effort: Lite

Copilot AI lite review requested due to automatic review settings November 3, 2026 17:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are identified, and the changes are supported by expanded unit, processor, streamed-history, and integration coverage.

No open issues 🎉

Review effort: Lite

Remove per-ID placeholders and duplicate-tracking sets. Populate only needed scheduling-event type indexes in one lazy pass using Dictionary.Add. Align regression coverage and documentation with whole-type indexing and native duplicate-key failures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings November 3, 2026 18:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Maintainer confirmation is warranted because the PR changes malformed duplicate scheduling-ID histories from ambiguous tracing correlation to fail-fast work-item abandonment in a core worker path, despite focused test coverage.

No open issues 🎉

Review effort: Lite

Remove the new-event pre-scan and populate both scheduling indexes lazily in one history pass. Extract trace correlation for direct no-listener enumeration assertions and refresh duplicate, failure, timer, and caching regressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings November 3, 2026 19:06
@berndverst

Bernd Verst (berndverst) commented Nov 3, 2026 •

Copy link
Copy Markdown
Member Author

Additional context for the allocation increases in the fresh profiling results: these are increases on top of the whole worker turn baseline, not allocations from a near-zero baseline.

For 4,096 scheduled tasks:

New completions Tracing Baseline allocation/turn Updated allocation/turn Change
64 Enabled 8,255.91 KiB 8,685.48 KiB +429.57 KiB (+5.20%)
512 Enabled 8,718.39 KiB 9,067.22 KiB +348.83 KiB (+4.00%)
64 Disabled 8,202.88 KiB 8,188.24 KiB -14.63 KiB (-0.18%)
512 Disabled 8,300.86 KiB 8,184.48 KiB -116.38 KiB (-1.40%)

The previously reported approximately 349 KiB increase therefore adds to a baseline of about 8.51 MiB per turn, bringing it to about 8.85 MiB (+4%). Totals include history conversion, orchestration replay, tracing, and response construction, not just the lookup. These are managed bytes allocated per turn, not permanently retained memory or peak live memory.

The additional allocation is still meaningful GC pressure: at 100 turns/second, +349 KiB/turn is approximately 34 MiB/second of additional allocation. The listener-enabled wide fan-in case reduced processing time from 88.69 ms to 20.19 ms, but this does not establish acceptable GC behavior under concurrent production workloads or justify the index for all sparse-completion histories. The full-history index is a review-directed simplicity/CPU tradeoff, not a memory optimization.

These figures compare the freshly rerun saved main baseline (936325b) with the implementation in 944d4f8. Raw measurements and profiling limitations are documented in the PR description.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are identified for the tracing performance changes, which include focused handling for lazy lookups, listener gating, cleanup, rewinds, and span relationships.

No open issues 🎉

Review effort: Lite

Copilot AI lite review requested due to automatic review settings November 3, 2026 19:32
@berndverst
Bernd Verst (berndverst) merged commit 12353cb into main Nov 3, 2026
9 of 10 checks passed
@berndverst
Bernd Verst (berndverst) deleted the berndverst-fix-trace-history-scan-perf branch November 3, 2026 19:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active issues were identified, and the tracing optimizations are accompanied by expanded unit, worker, and integration coverage for correlation, rewind handling, cleanup, and span relationships.

No open issues 🎉

Review effort: Lite

@berndverst Bernd Verst (berndverst) mentioned this pull request Nov 4, 2026
1 of 15 tasks
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.

Performance: avoid repeated orchestration-history scans for tracing

3 participants