Skip to content

perf(runtime): wake the send pump through a reusable claim-token signal (issue #240) - #241

Merged
SunSi12138 merged 5 commits into
devfrom
feature/issue-240-sendpump-wakeup-signal
Aug 18, 2026
Merged

SunSi12138 merged 5 commits into
devfrom
feature/issue-240-sendpump-wakeup-signal

Conversation

@SunSi12138

@SunSi12138 SunSi12138 commented Aug 18, 2026 •

Copy link
Copy Markdown
Owner

Closes #240(分配回归排查的落地修复部分)

背景

issue #240 的排查经专家复核 + GetAllocatedBytesForCurrentThread 探针闭环,终审结论:
回归 100% 归因于 8d4a1c9(issue #163 进度隔离)引入的双读 idle-wakeup 路径——
WaitToReadAsync().AsTask() ×2 + Task.WhenAny + AwaitFirstReadAsync 异步助手,
每次泵唤醒固定分配 ~312B(探针实测 802,148 次唤醒 × 312B = 250MB),低并发 unary 下
每个 RPC 触发 ~2 次唤醒。排水/Flush/TryPeek 均经探针证伪排除。

修改

  • 新增顶层 WakeupSignal(src/SharpLink.Runtime/WakeupSignal.cs,internal,可直接单测):
    DeadlineReadRace 同款 claim-token + ManualResetValueTaskSourceCore 无定时器变体。
    最终协议(8b6c5c9):整个协议编码进单一原子状态字(0 = idle,1 = 已闩锁信号,
    偶数 ≥2 = 已武装 arm(++generation << 1),bit 0 专属闩锁位)。WaitAsync 用一次
    原子 exchange 同时发布 arm 并消费先到闩锁;writer 要么一次 CAS 认领存活 arm,要么
    Interlocked.Or 写闩锁——Or 落在武装 arm 上得到 arm | 1,仍是同一个 arm,不会与任何
    其它代的武装值别名;随后的复查循环认领闩锁落位到的任何 arm,因此闩锁写在 arm 发布
    之后才落地
    (评审指出的丢唤醒交叉窗口)也仍会完成该 pending arm 而非丢失;认领一律
    把状态归零,成功唤醒不为下一臂残留陈旧闩锁。泵永远 await、永不弃置;每 arm 恰好一次
    SetResult(初版 CI Load Smoke 的 CompletionSentinel 崩溃已由 09ae81c 修复;claim-first
    协议由 15cef89 重构为原子状态,8b6c5c9 再按评审意见消除 token+1 编码的跨代别名)。
  • 新增 WakeupSignalTests 4 个确定性回归测试:成功认领不留残留、arm 前信号被闩锁
    同步消费、并发双写不双完成、晚到闩锁跨越 arm 发布仍必须完成该 arm(经测试专用
    BeforeLatchWrite 接缝精确复现评审给出的交叉交错)。测试对每个已完成的 arm 都先
    await 消费再 re-arm,与生产泵的 always-await MRVTSC 契约一致。
  • 删除 WaitForFramesAsync/AwaitFirstReadAsync 与"被弃读观测"复杂度(issue Chaos smoke intermittently fails with unobserved SendPump "Transport output completed." tasks during server restart #216)。
  • 自定义 MaxLatency deadline 竞态路径、双队列隔离语义原样保留。

实测(242 裸机,taskset 8-15,gcServer=1,A/B ×2,最终 head 干净构建 + 校验和验证)

场景 dev HEAD 本 PR(最终 head) v1.1.1
echo 16B c1 B/op 1,480 856(−42%) 1,070
echo 256B c512 B/op 864 835(−3.4%) 855
BDN Rpc_Add Allocated 1,384 B 760 B(−45%) 952 B

(最终 head 8b6c5c9:c1 两轮 856/856;c512 两轮 828/841,均值 834.5;BDN 16B/256B 四轮均为 760 B。)
全面反超 v1.1.1;与修复前协议持平——唤醒协议改动不改变稳态 B/op。

验证

  • 泵系 5 测试类 + WakeupSignalTests 共 41/41;全量单测 1025/1025;
  • CI(quick/Analyze C#/CodeQL)在最终 head 上重跑,全部通过。

请 review。

sunsi added 2 commits August 18, 2026 15:45
The pump's idle wake-up raced two retained channel reads with
Task.WhenAny, allocating two AsTask wrappers, the WhenAny promise, and
continuation closures on every wake. Replace it with a reusable
WakeupSignal (the DeadlineReadRace claim-token pattern without the
timer): the pump publishes an arm token and re-checks both queues before
sleeping, writers claim the token and complete the value-task source, and
Stop/fault paths signal the pump to exit. The dual-queue protocol-progress
isolation and the deadline-race path for custom MaxLatency are unchanged.
…token

The initial signal design let the pump abandon an armed value task when the
post-arm queue re-check found frames and re-arm immediately. With
RunContinuationsAsynchronously, the abandoned arm's continuation could
still be queued on the thread pool when the core was reset, tripping the
ManualResetValueTaskSourceCore completion sentinel
(InvalidOperationException, SIGABRT) — observed as the CI Load Smoke crash.

Every completion is now arbitrated by one per-arm token CompareExchange:
writers claim the token before SetResult, and WaitAsync consumes any signal
that arrived before the arm was published (latched) through the same
exchange. The pump always awaits the armed value task, so a continuation
fully runs before the next arm, each arm completes exactly once, and a
pre-arm signal can never leave the await hanging. Verified with 10x the
CI sharedmemory smoke on the baremetal host plus the pump test classes and
the full unit suite.

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

No blocking findings. The second commit fixes the abandoned-arm MRVTSC reset hazard by making the pump always await the armed ValueTask and arbitrating completion through _armToken; the current PR Quick and CodeQL runs are green.

One non-blocking performance/correctness-of-the-primitive observation: Signal() always writes _signaled = 1 before attempting to claim the current _armToken, but a successful current-arm claim never consumes/clears that latch. Therefore every normal writer wake that completes an already-armed waiter leaves _signaled == 1; after the pump drains back to empty, the next WaitAsync() consumes that stale latch and completes synchronously, producing one deterministic extra empty pump iteration per real idle wake. This does not lose frames or double-complete the same arm, but it is unnecessary work in a PR whose purpose is to optimize the wake path. Please consider making the latch represent only a pre-arm/unclaimed signal (while preserving the signal-before-arm race), and add a deterministic regression test for the wake primitive/race if practical.

Also, the PR description still says the pump does a post-arm queue re-check, but the current head deliberately removed that re-check and relies on the latched pre-arm signal instead; the description should be updated to match the final protocol. Since the performance numbers were collected before the second commit changed the wake protocol, re-running the key c512 allocation measurement on the final head would make the evidence fully closed.

…nal residue

- Move the nested private WakeupSignal out of RpcSession.SendPump into
  src/SharpLink.Runtime/WakeupSignal.cs as a top-level internal type so
  the wake primitive can be unit-tested directly.
- Signal() now claims the arm token first and only writes the _signaled
  latch when no arm is claimable. A successful claim therefore leaves no
  stale latch behind, so the next WaitAsync can no longer complete
  spuriously and force an extra empty pump iteration per real wake
  (review feedback on #241).
- Add WakeupSignalTests with three deterministic regression tests:
  claimed-arm residue, signal-before-arm latch, concurrent writers.
@SunSi12138

SunSi12138 commented Aug 18, 2026 •

Copy link
Copy Markdown
Owner Author

@SunSi12138 三条意见均已落实(commit 90de99c,验证已在最终 head 上闭环):

  1. 闩锁语义:Signal() 改为先 CAS 认领存活 arm,成功即完成、不触碰闩锁;仅在无 arm 可认领时才写闩锁,由下一次 WaitAsync 消费。闩锁现在只表示"arm 前的信号",成功唤醒不再残留陈旧闩锁——每次真实唤醒后那轮确定性空转泵迭代已消除。WakeupSignal 同时提为顶层 internal 类型(src/SharpLink.Runtime/WakeupSignal.cs),可直接单测,新增 3 个确定性回归测试:ClaimedArmMustNotLatchAStaleSignalForTheNextWait(成功认领不留残留,旧实现会挂)、SignalArrivingBeforeTheArmIsLatchedAndConsumedSynchronously(arm 前信号闩锁同步消费)、TwoWritersCannotDoubleCompleteOneArm(并发双写恰好完成一次,败者闩锁是真实排队唤醒)。3/3 通过。

  2. PR 描述:已重写——删除过时协议描述,明确最终协议是"依赖闩锁的 arm 前信号 + 泵永远 await",无武装后复查。

  3. 最终 head 重测(242 裸机,taskset 8-15,gcServer=1,90de99c 干净构建 ×2):

    • echo 16B c1 B/op:856 / 856(dev HEAD 1480,v1.1.1 1070)
    • echo 256B c512 B/op:832 / 841(dev 864,v1.1.1 855)
    • BDN Rpc_Add Allocated:760 B ×4(16B/256B 各两轮;dev 1384 B,v1.1.1 952 B)

    与修复前协议持平——闩锁修复是协议卫生(移除每真实唤醒后的空转迭代),不改变稳态 B/op;描述中的实测表已换成最终 head 数字。

    验证:泵系 5 测试类 + WakeupSignalTests 共 40/40,全量单测 1024/1024,CI(quick/Analyze C#/CodeQL)在 90de99c 上已全部通过。

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Blocking finding on the new 90de99c WakeupSignal protocol: changing Signal() to claim-first / latch-second re-opens a lost-wakeup window between the pump's empty-queue check and WaitAsync() arm publication.

Concrete interleaving:

  1. Pump observes both queues empty and is about to call WaitAsync().
  2. A producer enqueues a frame and enters Signal(), reads _armToken == 0, then pauses before writing _signaled = 1.
  3. Pump enters WaitAsync(): resets the core, publishes arm token T, then Interlocked.Exchange(ref _signaled, 0) reads 0.
  4. Producer resumes, writes _signaled = 1, and returns without re-checking the newly published arm.
  5. WaitAsync() returns a pending ValueTask backed by arm T; the frame is already queued, but nobody completed T. With no later send/stop/fault signal, the pump can sleep indefinitely.

So the PR description's guarantee that a signal arriving with no claimable arm is consumed by the next WaitAsync() is not true for this crossing race: the latch write can occur just after that next wait has already consumed the latch. The new sequential SignalArrivingBeforeTheArm... test does not cover this interleaving.

Please make arm publication + pending-signal state participate in one atomic protocol (for example a single atomic state representing idle/latched/armed), or use an equivalent retry handshake after latching that cannot double-complete an arm. Add a deterministic regression for the exact crossing race if practical.

The previous stale-latch issue is otherwise addressed, the PR description/performance evidence is substantially improved, and the final-head PR Quick + CodeQL runs are green. Minor doc-only note: the description still says CI is '待结算', but both runs have completed successfully.

The claim-first/latch-second protocol introduced a lost-wakeup window:
a writer could observe the idle state, pause, and write the latch just
after WaitAsync had published the arm and consumed the (still empty)
latch, leaving the queued frame with a pending arm that nobody ever
completes.

Encode the whole protocol in a single atomic state word: 0 = idle,
1 = a latched signal, and any value >= 2 = an armed waiter (arm token
+ 1). WaitAsync publishes the arm and consumes a pending latch in one
atomic exchange; a writer either claims the live arm with one CAS or
latches with Interlocked.Or, which keeps a concurrently published arm
intact, and a re-check loop claims any arm the latch lands on, so a
signal crossing the arm-publication boundary still completes the
pending arm instead of being lost. Claiming always returns the state
to idle, preserving the no-stale-latch property for the next arm.

Add a deterministic regression test for the exact crossing interleaving
via a test-only BeforeLatchWrite seam.
@SunSi12138

Copy link
Copy Markdown
Owner Author

@SunSi12138 丢唤醒交叉窗口已确认属实,15cef89 按建议重构为单一原子状态协议并闭环:

  1. 协议重构:WakeupSignal 整个协议现在编码进一个原子状态字:0 = idle,1 = 已闩锁信号,≥2 = 已武装 arm(token+1)。
    • WaitAsync() 用一次原子 exchange 同时发布 arm 并消费先到闩锁——"arm 发布 + 消费闩锁"不再分离,交叉窗口在 waiter 侧关闭。
    • Signal() 快路径一次 CAS 认领存活 arm(不碰闩锁,无残留);认领失败则 Interlocked.Or 写闩锁——Or 不破坏并发发布的 arm——随后复查循环认领闩锁落位到的任何 arm。你给的精确交错(writer 读到 idle → 泵 exchange 发布 arm 且消费空闩锁 → writer 闩锁落地)现在由复查循环完成该 pending arm,信号不丢失。
    • 认领一律把状态归零:成功唤醒不残留陈旧闩锁(上一轮修复保留),每 arm 恰好一次 SetResult(CAS 唯一仲裁,不会双完成)。
  2. 确定性回归测试:新增 LateLatchCrossingArmPublicationStillCompletesTheArm——通过测试专用 BeforeLatchWrite 接缝把 writer 精确停在"观察到 idle 之后、写闩锁之前",主线程发布 arm(assert pending),再放行 writer,断言该 pending arm 必须被晚到闩锁完成且下一臂无残留。该交错在 90de99c 协议下必然挂。
  3. 最终 head 15cef89 重测(242 裸机,taskset 8-15,gcServer=1,干净构建 ×2):
    • echo 16B c1 B/op:856 / 856(dev HEAD 1480,v1.1.1 1070)
    • echo 256B c512 B/op:841 / 841(dev 864,v1.1.1 855)
    • BDN Rpc_Add Allocated:760 B ×4(dev 1384 B,v1.1.1 952 B)
    • 泵系 5 测试类 + WakeupSignalTests 共 41/41;全量单测 1025/1025——与重构前持平,零性能回归。
  4. 文档:描述已更新为原子状态协议;"CI 待结算"已改为全部通过(quick / Analyze C# / CodeQL 在 15cef89 重跑中,90de99c 上已全绿)。

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-review of final head 15cef89: the previous blocking lost-wakeup crossing is fixed. Folding latch + arm into one atomic state closes the writer-observes-idle / arm-publishes / late-latch window, and LateLatchCrossingArmPublicationStillCompletesTheArm deterministically exercises that exact interleaving. I do not see a remaining lost-wakeup, same-arm double-SetResult, or MRVTSC reset-before-completion blocker. PR Quick (including Load/Chaos Smoke) and CodeQL are green on this head.

One non-blocking state-encoding concern remains. _state documents arm values as token + 1, while the latch operation is Interlocked.Or(ref _state, 1). For an even arm value, e.g. generation 1 => state 2, OR-ing the latch bit changes it to 3 — which is also the nominal state of generation 2. So the latch write does not literally keep the arm token intact, and there is a small ABA/spurious-wake scenario: a delayed latch-path writer can turn arm 2 into 3, another writer claims/completes that arm, the pump later re-arms generation 2 as state 3, and the delayed writer's re-check can then CAS that new arm to idle and complete it even if its queued work was already drained. I don't see this causing a lost wake or duplicate completion of the same arm, so I am not treating it as blocking, but it weakens the generation-token invariant and the stated no-spurious-wake property. A cleaner encoding would reserve bit 0 exclusively for Latched, e.g. publish arms as (++generation << 1) and let OR 1 mark the same arm without aliasing the next generation.

Minor test-hygiene note: the direct WakeupSignal tests currently re-arm after checking IsCompletedSuccessfully without consuming the prior ValueTask via GetResult/await. Production does consume every arm before re-arming, so consuming the completed ValueTask in tests would model the actual MRVTSC contract more closely.

…ve bit 0 for the latch

With token + 1 arm values, OR-ing the latch bit onto an even arm aliased the
next generation's nominal armed value (gen1 | latch == 3 == gen2 nominal),
weakening the per-generation token invariant. Publishing arms as
++generation << 1 keeps every armed value even, so arm | 1 stays that same
arm and can never alias another generation.

Tests now consume every completed arm via await before re-arming, matching
the production pump's always-await MRVTSC contract.
@SunSi12138

Copy link
Copy Markdown
Owner Author

@SunSi12138 两条意见均已落实(8b6c5c9):

  1. 编码别名:arm 改为 ++generation << 1 发布——所有武装值恒为偶数,bit 0 专属闩锁位。Or 1 落在武装 arm 上得到 arm | 1,仍是同一个 arm,不可能与任何其它代的武装值别名(原 token+1 下 gen1|latched == 3 == gen2 名义值的 ABA 已从编码层面消除);延迟 writer 的复查 CAS 只能认领它闩锁真正落位的那个 arm。类注释与 Signal() 内注释已同步改写。
  2. 测试卫生:4 个直接测试现在对每个已完成的 arm 都先 await 消费再 re-arm(IsCompletedSuccessfully 断言保留),与生产泵 always-await 的 MRVTSC 契约一致。

最终 head 8b6c5c9 重测(242 裸机,taskset 8-15,gcServer=1,干净构建 ×2):

  • echo 16B c1 B/op:856 / 856(dev HEAD 1480,v1.1.1 1070)
  • echo 256B c512 B/op:828 / 841(dev 864,v1.1.1 855)
  • BDN Rpc_Add Allocated:760 B ×4(dev 1384 B,v1.1.1 952 B)
  • 泵系 5 测试类 + WakeupSignalTests 41/41;全量单测 1025/1025——编码改动零性能影响。

PR 描述已同步(协议段落、数字表、测试说明)。CI(quick/Analyze C#/CodeQL)在 8b6c5c9 重跑中。

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-review of final head 8b6c5c9: no blocking findings.

The previous arm/latch alias concern is resolved cleanly: armed states are now ++generation << 1, so bit 0 is reserved exclusively for the latch. arm | 1 remains tied to the same generation and cannot alias another generation's nominal arm. The prior crossing lost-wakeup invariant remains intact because the latch write still preserves a concurrently published arm and the re-check loop claims it. The tests also now consume each completed ValueTask before re-arming, matching the production pump's always-await MRVTSC contract.

PR Quick (including unit/integration/load/chaos smoke) and CodeQL are green on this head, and dev has not moved.

One documentation-only nit: the performance table still lists c512 as 836 B/op, while the final-head note below reports runs 828/841; their arithmetic mean is 834.5 B/op (roughly 835), so those two figures should be made consistent. This is non-blocking.

@SunSi12138

Copy link
Copy Markdown
Owner Author

@SunSi12138 文档 nit 已修:性能表 c512 改为 835(−3.4%),与下方注释的 828/841(均值 834.5)一致;仅更新 PR 描述,无新提交。

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.

1 participant