[SunSi12138]

FINAL DISPOSITION (2026-10-10): NO-GO — CLOSED WITHOUT MERGE.

The static 1:1 Unary logical async-wrapper specialization in this PR is not being adopted. The ~134–139 B/op reduction for eligible plain suspended Unary calls is real and repeatable in JIT/NativeAOT; however, it applies only to a narrow call shape. The implementation materially expands PendingCall registration/completion ownership, timer-failure and graceful-drain handling. The corresponding full-RPC throughput, CPU and P999 evidence is noisy and mixed, and the risk/maintenance cost outweighs the demonstrated whole-workload benefit.

Final decision: keep the existing production AwaitLogicalInvocationAsync wrapper and do not merge this PR. See #741's final No-Go assessment for detailed reasons. The A/B experiments, source and race-regression tests are retained in this closed PR as research, not as proposed changes to dev.

The following content preserves the previous exploratory evidence and implementation notes. Earlier ALLOCATION GO, MERGE HOLD, checklists and prior review-ready wording are superseded by this final No-Go decision. Running further benchmarks is not a prerequisite for closing this implementation.

---

Historical review-phase introduction (superseded by the final No-Go above). The corrected-head JIT and NativeAOT A/B data below remain valid as allocation evidence, but not sufficient justification to merge.

Latest performance conclusion — corrected implementation (2026-10-10)

Measured outcome: ordinary suspended Unary saves about 134–139 managed B/op in BOTH JIT and NativeAOT, consistent with removal of the 136 B logical async wrapper. The retry-enabled full-RPC test was also executed, including first-attempt success, exactly one forced retry, backoff and exhausted retry; no matching 136 B allocation penalty was found on those fallback paths. The benchmark validates physical attempt counts, not merely enabled retry settings.

Corrected implementation, exact base vs candidate	Allocation Δ	QPS Δ	CPU/op Δ	P99 Δ
JIT TCP plain Unary c128	−136.7 B/op	+3.9%	−2.6%	−6.1%
JIT SharedMemory plain Unary c128	−133.6 B/op	+5.1%	−4.9%	−24.5%
NativeAOT TCP plain Unary c128	−136.0 B/op	+5.4%	−4.0%	−2.4%
NativeAOT SharedMemory plain Unary c128	−135.1 B/op	+2.5%	−2.6%	−15.0%
JIT TCP retry-first c128	−0.4 B/op	−4.8%	+4.8%	+5.7%
NativeAOT TCP retry-first c128	−0.1 B/op	+0.2%	−0.7%	+1.4%

Historical per-dimension assessment (not the final merge decision):

• Allocation savings confirmed: the original 136 B per-call wrapper cost is removed on the eligible plain Unary path. Repeatable across JIT/NativeAOT and TCP/SharedMemory.
• Execution complete (retry): real forced retries ran (1/2/3 physical attempts verified), with near-zero retry-path allocation delta.
• HOLD for merge (performance): JIT TCP plain c32 measured −3.2% QPS / +6.5% CPU, JIT retry-first c128 measured −4.8% QPS / +4.8% CPU, and JIT TCP plain c128 P999 spiked +251.6%. Identical-binary A/A QPS variation reached −3.6% to +4.8%, so the throughput/tail observations are too noisy to declare a regression or statistically significant improvement. Independently replicate these cases before the final GO/No-GO merge decision.

Evidence: #741 balanced full-RPC JIT/NativeAOT A/B, both PASS (6 JIT pairs, 4 NativeAOT pairs, A/A controls; raw JSON, paired tables and build artifacts), and performance interpretation in PR discussion.

The published A/B measurements compare base eb99fe887cf2129d9b88441245ca0a4a6406b6c2 with measured head fecd0eef645d940ec8311af16a7f2deb496c7151. Current PR head 77a087817fd12323efb1d73f1589f51bdd80caca only isolates the benchmark runner to restore other CI workflows; it does not change the measured client implementation. Exact-head CI/A/B is rerunning after that harness-only change; retain this provenance until its new run completes. Full metric table, limitations and historical measurements appear below.

Scope

For a plain Unary call with one physical attempt, no telemetry scope, no interceptors and no endpoint-admission observer, reuse a cached per-client completion observer instead of allocating an outer AwaitLogicalInvocationAsync state-machine box for each suspended call. Retry-capable paths, telemetry, interceptors, admission, OneWay, ClientStreaming and streaming enumerators retain their prior logical-call wrapper/ownership.

Correctness changes from review (2026-10-10)

1. Published-slot ownership is authoritative. PendingRequestTable.Rent propagates registrationPublished from the successful slot CAS, before OnRegistered can throw (e.g. from a custom deadline timer). The caller refunds its admission reservation and decrements the logical owner inline only when publication never occurred. A published call retains exactly-one terminal ownership even when Rent throws afterwards.
2. Drain observes the user-visible lifetime. IPendingCallPostOperationObserver fires only after RpcRequestOperation.CompleteResponse/SetError has entered its terminal state. Existing attempt/admission observers keep their prior ordering. The shape observer does not release the drain count while the user's Unary ValueTask is still pending.
3. Authoritative failures are not hidden. Removed the broad exception handler around CompleteRegistrationIfDisposed's TryComplete. A terminal-owner cleanup/invariant exception must remain visible; capacity release is still protected by the table's existing completion finally/error path.
4. Current dev integrated. The branch is up to date against dev and the change was kept within existing maintainability LOC budgets by extracting the synchronous Rent overloads to a partial file. UTF-8 BOM regressions were also removed.

Follow-up P2: published registration failure and graceful retirement

The first ownership fix prevented double release, but a failing timer/owner registration still left a CAS-published pending slot attached to an already failed logical call until the connection stopped. This follow-up uses the pending table's authoritative terminal transition in the exceptional registration path, before returning a failed Unary operation.

• OnRegistered always signals MarkRegistered from finally, even when the pre-barrier owner callback throws; cancellation/Dispose may therefore wait safely for the registration barrier.
• CompleteFailedRegistration claims the same slot by its captured immutable request ID, not a potentially pooled/reused PendingCall.Id, and completes through CompleteTakenCall. It does not resample an application-provided TimeProvider that may already be throwing.
• The specialized Unary receives the original pooled operation with its terminal failure rather than a second independent failed ValueTask, so its post-operation observer releases logical ownership promptly and the operation is reclaimed when observed.
• Non-Unary/streaming registration still propagates its original registration exception after authoritative cleanup. Exceptions from cancellation, CompleteRegistrationIfDisposed, or a broken terminal owner are not converted into successful registration.
• Post-barrier steps also use the captured request ID/deadline/cancellation token, avoiding an ABA race when a completion returns a pending call to its pool.

Deterministic regressions now cover pre-/post-MarkRegistered failures, a timer that also poisons later timestamp reads, concurrent Dispose waiting for the registration barrier, terminal-owner exceptions, streaming failure propagation, and a real MultiCluster.RemoveClusterAsync retirement with an 8 s grace period that must finish within 2 s rather than wait out the grace period.

Focused regressions

• SharpLinkClientLogicalShapeTests: single-attempt success / remote fault / user cancellation; suspended-call drain visibility and deterministic pause after result publication but before logical decrement; a deadline timer throwing after CAS; no double release on later stop. Tests explicitly invoke the specialization to be independent of process-wide telemetry listeners.
• PendingRequestTablePostOperationObserverTests: post-operation observer sees terminal success/failure; a deterministic Dispose vs published-registration race with a throwing terminal owner verifies that cleanup failure is propagated, the terminal owner is called once and capacity/slots are released.
• Existing retry, interceptor, telemetry and streaming paths remain subject to the full validation matrix.

Validation provenance

• Measured feature implementation fecd0eef645d: [NO-GO / Performance] Remove client logical-invocation async wrapper only when lifecycle and throughput evidence justify it #741 JIT and NativeAOT full-RPC A/B both completed successfully (run 38033967753); PR Fast, CodeQL and pending/codec also passed.
• Latest PR head 77a087817fd12323efb1d73f1589f51bdd80caca: restored the shared BenchmarkDotNet entrypoint and moved the [NO-GO / Performance] Remove client logical-invocation async wrapper only when lifecycle and throughput evidence justify it #741 JIT probe to the dedicated AOT-compatible project so existing performance workflows can build their exact-base controls. This changes the test harness / CI wiring only, not client hot-path code. Current-head workflows have been triggered again; use PR checks for the definitive latest status.
• Known unrelated-workflow failure on the preceding head: RPC Session Flush Performance Evidence failed to build its base benchmark because its pinned Program.cs referenced the newly added [NO-GO / Performance] Remove client logical-invocation async wrapper only when lifecycle and throughput evidence justify it #741 runner missing from that base worktree (CS0103), not because the client failed the RPC flush benchmark. The latest harness-isolation change addresses this issue; confirm its rerun before merging.
• The current-head [NO-GO / Performance] Remove client logical-invocation async wrapper only when lifecycle and throughput evidence justify it #741 summary below reports corrected-implementation performance, not a guarantee of statistically significant throughput/latency improvements.

Full #741 JIT/NativeAOT measurements — measured head fecd0ee (2026-10-10)

• Run: Issue 741 Unary Logical Lifetime A/B — both JIT PASS and NativeAOT PASS, with raw JSON and paired summary artifacts.
• Exact base: eb99fe887cf2129d9b88441245ca0a4a6406b6c2; exact candidate: fecd0eef645d940ec8311af16a7f2deb496c7151.
• Same benchmark source copied onto base/head before building; real generated idempotent AddAsync RPC over TCP and SharedMemory; JIT uses 6 balanced AB/BA pairs per configuration, NativeAOT uses 4. Negative A/A controls compare the candidate build against itself.
• Explicit physical attempt counts are asserted for retry enabled with first-attempt success (1), forced one retry (2), and retry exhausted (3); 2 ms backoff is also measured. Process-wide CPU and B/op include both client and server.

Mode	Transport / scenario	c	Base → Candidate B/op	QPS Δ	CPU/op Δ	P99 Δ
JIT	TCP plain Unary	1	392.3 → 256.9	+2.0%	−2.4%	−1.5%
JIT	TCP plain Unary	32	394.4 → 258.5	−3.2%	+6.5%	+2.6%
JIT	TCP plain Unary	128	398.7 → 261.9	+3.9%	−2.6%	−6.1%
JIT	SharedMemory plain Unary	128	421.5 → 287.6	+5.1%	−4.9%	−24.5%
JIT	TCP retry first success	128	766.6 → 766.3	−4.8%	+4.8%	+5.7%
JIT	TCP retry once	128	4441.9 → 4444.3	+1.9%	−0.5%	−10.1%
NativeAOT	TCP plain Unary	1	392.0 → 256.3	+0.8%	−2.1%	−0.6%
NativeAOT	TCP plain Unary	128	394.4 → 258.4	+5.4%	−4.0%	−2.4%
NativeAOT	SharedMemory plain Unary	128	409.0 → 273.7	+2.5%	−2.6%	−15.0%
NativeAOT	TCP retry first success	128	762.8 → 762.8	+0.2%	−0.7%	+1.4%
NativeAOT	TCP retry once	32	2774.7 → 2774.7	+2.1%	−0.5%	−6.3%

Interpretation. Across both modes the ordinary Unary specialization saves about 134–139 managed B/op with no corresponding allocation increase in the retry-first/forced-retry paths. Throughput and tail-latency deltas are substantially noisier: same-binary A/A QPS controls registered approximately −3.6% (JIT retry-once c32) and +4.8% (NativeAOT plain c32). Do not claim a statistically significant QPS gain or blanket non-regression from a green evidence workflow. Specifically, JIT TCP plain c32 showed −3.2% QPS/+6.5% CPU, JIT retry-first c128 showed −4.8% QPS/+4.8% CPU, and JIT TCP plain c128 P999 showed +251.6% in the short hosted-runner samples. These outliers merit independent, targeted replication before performance merge approval.

Historical performance motivation (original candidate on dev 56c643c, Apple M4)

Wrapper cost is a constant 136 B per logical call, independent of concurrency and transport, and its allocation share grows with concurrency:

transport	c1	c8	c32	c128
TCP	15.9%	30.7%	36.2%	37.7%
SharedMemory	10.4%	28.7%	37.4%	37.5%

Per logical call, not per attempt (136 B at 1, 2 and 3 attempts). 0 B for sync-completed / already-faulted / already-cancelled invocations, and 0 B for plain OneWay.

PR branch vs a clean dev build, 6 interleaved reps:

c	B/op dev → PR	QPS	p99	p999	Gen0
1	856.1 → 720.2 (−15.9%)	−2.2%	+1.6%	+2.5%	17 → 14
32	377.4 → 241.1 (−36.1%)	+2.6%	−4.0%	−54.7%	142 → 92
128	361.4 → 225.0 (−37.7%)	+2.5%	−8.1%	−13.1%	270 → 172

Allocation, Gen0 and tail latency improve repeatably. The QPS delta is within noise at 6 reps (p ≈ 0.20) and is not claimed as significant.

The measurements above predate the review fixes and the integration of the newer dev base. They explain why the specialization was attempted; they are not a fresh throughput/latency result for the current PR head. No statistically significant QPS improvement is claimed.

Historical review checklist (superseded; no further work on this candidate)

• Terminalize CAS-published registration errors promptly, including pre-/post-barrier races and real MultiCluster graceful retirement.
• Specialization-specific success/fault/cancel and suspended drain regression.
• Post-publication timer/registration ownership regression.
• Dispose-vs-registration with throwing terminal owner, preserving failure visibility.
• Remove premature drain release and broad TryComplete exception suppression.
• Run full-RPC measurements on a retry-enabled workload (first-success / forced-retry / exhausted / backoff). Data collected; noisy throughput non-regression remains under review.
• Re-run equal-source JIT/NativeAOT full-RPC A/B on the corrected head with CPU, throughput, allocation, P99/P999 and A/A controls; correctness/drain stress remains covered by focused CI.
• Independently replicate JIT TCP plain c32 and retry-first c128 throughput/CPU deviations and the JIT c128 P999 outlier before final performance GO/merge sign-off.

Changed files

• src/SharpLink.Client/SharpLinkClient.LogicalScope.cs
• src/SharpLink.Client/SharpLinkClient.Invokers.cs
• src/SharpLink.Client/PendingRequestTable.cs
• src/SharpLink.Client/PendingRequestTable.Rent.cs
• src/SharpLink.Client/PendingRequestTable.Registration.cs
• test/SharpLink.UnitTests/Client/SharpLinkClientLogicalShapeTests.cs
• test/SharpLink.UnitTests/Runtime/PendingRequestTablePostOperationObserverTests.cs

FINAL DISPOSITION: NO-GO. This PR is closed without merge; existing logical lifetime wrapper remains the production implementation.

[SunSi12138]

Review result: changes needed.

The performance evidence for the static 1:1 specialization is convincing enough to keep pursuing this direction, but the current ownership transfer is not yet exactly-once.

Two correctness issues are blocking: (1) attemptRegistered is set only after PendingCalls.Rent() returns even though the pending slot/observer ownership transfers earlier, so a post-publication registration failure can release the logical counter twice; and (2) the shape observer decrements the logical count before the returned operation is made terminal, which breaks the existing ISharpLinkClientDrainInspector contract used by multi-cluster graceful retirement.

CompleteRegistrationIfDisposed also catches the whole authoritative TryComplete() transition. That suppresses failures from operation completion, owner cleanup, pool/capacity accounting, etc., not just completion-observer diagnostics; please isolate only the specific callback that is allowed to fail.

Current validation is also incomplete: PR Fast run 36089835555 fails formatting because PendingRequestTable.cs and SharpLinkClient.Invokers.cs gained a BOM, so build/unit/generator/load-test steps never ran. The branch is also behind current dev; after the ownership fixes, remove the BOMs, update to current dev, add the dedicated specialized-path/drain/registration-race tests listed in the PR body, and rerun the current validation matrix.

[SunSi12138]

[P2] Releasing here happens before the caller's ValueTask is made terminal. PendingRequestTable.CompleteTakenCall invokes the completion observer before RpcRequestOperation.CompleteResponse/SetError.

_activeLogicalInvocations is the ISharpLinkClientDrainInspector.ActiveCallCount used by multi-cluster replace/remove graceful drain, and the existing ownership contract defines it through completion of the user-visible ValueTask. With this ordering there is a real window where the count is 0, retirement starts StopAsync(), and the caller's unary ValueTask is still pending.

Please keep the zero-allocation specialization but move this release to a boundary after the operation has entered its terminal state. RpcRequestOperation already uses RunContinuationsAsynchronously = true, so publishing the operation result first does not require synchronously running user continuation code. Add a deterministic drain regression that pauses between the two terminal steps.

[SunSi12138]

Fixed by moving only the shape observer behind the operation terminal publication: IPendingCallPostOperationObserver runs after RpcRequestOperation.CompleteResponse/SetError, while admission/attempt observers retain prior ordering. DrainInspectorMustNotReachZeroBeforeUnaryOperationBecomesTerminal pauses in the post-operation hook, awaits the user-visible unary result while the hook remains blocked, asserts the drain counter is still one, then releases the hook and checks a single transition to zero.

[SunSi12138]

Follow-up on the post-publication registration-failure / graceful-drain P2 (reported after the initial Ready-for-review transition):

Fixed in the current branch (head 2b6dc150306e26dbebae2eec144dfd8466a70519).

• Registration setup faults now claim the published slot by the saved request ID and finish through PendingRequestTable.CompleteTakenCall, instead of returning a separate failed Unary ValueTask while the slot remains live.
• MarkRegistered() is published from finally even when the registration owner throws; post-barrier cancellation and disposal use captured immutable values so a pooled PendingCall cannot route cleanup to a newer request.
• The specialized Unary returns its original terminal operation and releases logical ownership promptly, while non-Unary registration still rethrows after cleanup. Exceptions from authoritative terminal-owner and disposal paths are not suppressed.
• The registration-failure terminal path deliberately avoids resampling a potentially failing application TimeProvider.

Regression coverage: ThrowingDeadlineTimerAfterSlotPublicationMustNotDoubleRelease, FailedRegistrationMustNotWaitForMultiClusterGracefulRetirementTimeout (8-second grace must complete within 2 seconds), pre-/post-MarkRegistered owner faults, competing Dispose, repeat-throwing provider, streaming failure propagation and terminal-owner failure visibility. On the preceding correctness head 22176908, PR Fast passed 1,921/1,921 unit tests, 269/269 generator tests, 77/77 load tests, plus formatting, Release build, maintainability and allocation gates.

Merged the newest dev (#753) into the current head; CI for that exact final head is running. Performance acceptance for forced-retry full-RPC and the corrected JIT/NativeAOT CPU/QPS/latency matrix remains explicitly open before merge and should not be inferred from the earlier benchmark values.

[SunSi12138]

#741 corrected-head full-RPC performance evidence — executed

Run: https://github.com/SunSi12138/SharpLink/actions/runs/38033967753 (JIT PASS, NativeAOT PASS)
Base: eb99fe887cf2129d9b88441245ca0a4a6406b6c2
Candidate: fecd0eef645d940ec8311af16a7f2deb496c7151

Both revisions used an identical-source generated Unary RPC harness on the same runner per mode, with balanced AB/BA order and candidate-vs-candidate A/A controls. Actual retry work was verified (1/2/3 physical attempts for first-success / forced-once / exhausted). The raw metrics and pairing files were uploaded as Actions artifacts.

Repeatable allocation win on the corrected client: plain TCP Unary saves 135–137 B/op in JIT and 136 B/op in NativeAOT; SharedMemory c128 saves 134 B/op JIT / 135 B/op NativeAOT. Retry-first and forced-retry scenarios have approximately zero allocation delta.

Do not merge on QPS evidence alone: the hosted-runner throughput and far-tail results are noisy (A/A throughput noise up to roughly 4.8%). JIT plain TCP c32 gave −3.2% QPS / +6.5% CPU, retry-first c128 −4.8% QPS / +4.8% CPU, and plain TCP c128 had a large P999 outlier. These differ from NativeAOT and other JIT concurrency results; they require targeted independent replication before asserting the strict 2–3% retry non-regression or a significant throughput gain.

I updated the PR description with the JIT/NativeAOT table and marked the execution of both outstanding performance suites complete, while leaving a separate performance sign-off item open for those noisy regressions.