perf: eliminate TimedBatch deadline-wait allocations (AsTask + WhenAny) — issue-204 - #205
perf: eliminate TimedBatch deadline-wait allocations (AsTask + WhenAny) — issue-204#205SunSi12138 wants to merge 3 commits into
Conversation
…rm, and stop (issue-204)
…imer, dropping AsTask/WhenAny allocations (issue-204)
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b1f7f582d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| private void OnTimerFired() | ||
| { | ||
| if (Interlocked.Exchange(ref _readAbandoned, 1) != 0) |
There was a problem hiding this comment.
When a read wins near its deadline, disposing its ITimer does not guarantee that an already queued callback has finished. If the pump starts another wait first, that stale callback observes the newly reset _readAbandoned, disposes the new arm's _timer, and completes the new _core as timed out, causing a premature batch flush. Capture and validate an arm generation (or use callback-specific state) before a timer callback touches the reusable race state.
Useful? React with 👍 / 👎.
| lock (_gate) | ||
| _armedDueTimes.Add(dueTime); | ||
| return inner.CreateTimer(callback, state, dueTime, period); |
There was a problem hiding this comment.
The new tests use WasArmed as a synchronization point before advancing the manual clock, but this publishes the due time before inner.CreateTimer has actually installed the timer. If the test thread observes the list entry in that window and advances the clock, the timer is subsequently scheduled relative to the advanced timestamp, so the final expected advance does not fire it and TimedBatchShouldExtendBatchForFrameArrivingBeforeDeadline can time out nondeterministically. Publish the entry only after inner.CreateTimer returns, as the existing observing provider above does.
Useful? React with 👍 / 👎.
…st timer arms after install (issue-204 codex review)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 443f4f731a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _timer = _timeProvider.CreateTimer( | ||
| state => OnTimerFired(generation), this, timeout, Timeout.InfiniteTimeSpan); |
There was a problem hiding this comment.
With a valid short MaxLatency (for example, one tick), TimeProvider.System can execute the callback before CreateTimer returns and this assignment completes. OnTimerFired then evaluates _timer!.Dispose() while _timer is still null on the first arm, producing an unhandled timer-callback exception that can terminate the process; on later arms it may instead dispose the previous timer and leak the new one. Create and store a disabled timer first, then arm it with Change, or otherwise give the callback an already-published per-arm timer reference.
Useful? React with 👍 / 👎.
| if (!ReferenceEquals(read, _read)) | ||
| return; // Stale completion from a previous arm: never touch the current cycle's state. | ||
|
|
||
| if (Interlocked.Exchange(ref _readAbandoned, 1) != 0) |
There was a problem hiding this comment.
When an old read continuation passes this identity check and is suspended before the exchange, its timer can win and the pump can re-arm this object, replacing _read and resetting _readAbandoned to zero. The old continuation then resumes, claims the new arm, disposes its timer, and completes its _core with the old read's result; that false read win clears the new _pendingReadWait, abandoning the actual channel waiter so a subsequently queued frame may never wake the pump. Use immutable per-arm state, or atomically validate the arm generation as part of claiming it, rather than separating validation from the shared abandon flag.
Useful? React with 👍 / 👎.
1 participant
Closes #204.
问题
SendPumpTimedBatchDeadlineCycle在 deadline 等待周期分配 566 B/cycle（issue #204 数据 547-576 区间），远高于正常 idle→wake 周期的 496 B。开销来自WaitForMoreUntilDeadlineAsync里的：WaitToReadAsync(_sessionCancellation)：可取消 token 使 unbounded channel 无法复用池化 waiter，每次分配WaitingReadAsyncOperation+CancellationTokenRegistration（.NET 10UnboundedChannel.WaitToReadAsync源码确认：CanBeCanceled == false才走_waiterSingleton.TryOwnAndReset()零分配路径）waitToRead.AsTask()：分配ValueTaskSourceAsTask（channel 的AsyncOperation不实现ITaskCompletionAction；ValueTask<bool>无公开OnCompleted，这一步无法省）Task.Delay(...)：DelayPromise+ 每次一个 timerTask.WhenAny(...)：WhenAnyPromiseCancellationTokenSource复用/取消逻辑方案
新增
DeadlineReadRace（IValueTaskSource<bool>，内嵌ManualResetValueTaskSourceCore<bool>）：WaitToReadAsync(CancellationToken.None)（池化 waiter，与 perf: non-cancellable SendPump channel waits (issue 157) #203 主等待路径一致）AsTask()一次 + 每次等待一个TimeProvider.CreateTimer（tick 精确，超时即Dispose）+ 一个闭包Task.WhenAny、Task.Delay、_delayCancellationCTS保留的语义（#157 约束）：
_pendingReadWait保留并在下一轮WaitToReadAsync()复用batchDeadline不重置）；writer 关闭 →ReadClosed清保留退出；超长 deadline 按MaximumTimerDelay分块重挂并发正确性（关键点）：
UnsafeOnCompleted对已完成任务会内联同步调用）ReferenceEquals身份校验：上一轮的迟到 continuation 不可能作用到当前轮状态Interlockedabandon 标志裁决；RunContinuationsAsynchronously防重入验证（192.168.31.242，SDK 10.0.110，taskset -c 2,3）
测试
新增 4 例 bounded-completion 测试（
SendPumpTests）：TimedBatchShouldDeliverFrameSentAfterDeadlineFlushThroughRetainedRead—— 保留语义回归（timer 赢后读复用；丢弃读会使后续帧永远无法唤醒泵）TimedBatchShouldExtendBatchForFrameArrivingBeforeDeadline—— read 赢合批（两帧一次 flush，记录型 provider 同步）TimedBatchShouldRearmAcrossMaximumTimerDelayChunks—— 超长 deadline 分块重挂且不提前 flushTimedBatchDeadlineWaitShouldExitWhenSessionIsDisposed—— 等待期停止（ReadClosed）远程 943 总/942 过：唯一失败为
PumpBlockedInFlushExitsWhenTransportOutputFaults——dev 既有问题（隔离复跑 5/5 确定性 5s 超时，本改动不触及 Balanced flush 故障路径；本地 macOS 全套 943/943 全绿）。CI 上ManifestSourceIsolationTests的 poison flake 为已知既有竞态。