Repository navigation
Let work done outside the window system wake a waiting guest - #3
Conversation
A guest that waits for input with wguiInstanceWaitEvents only looks at other work, such as a WebGPU event, when its wait ends. If that work completes on another thread while the guest waits, the guest sees it up to 250 ms later, when the host interrupts the wait, or at its timeout. - GuiCtx::event_waker returns a closure that ends the wait of every guest of the context, as wguiInstanceWake does. It can be called from any thread, and holds only a weak reference to the context. It is meant for hooks like wasmer_webgpu's WebGpuCtxBuilder::event_waker. - GuiRuntimeControl::wake_all does the same from a runtime control. A wake is sticky: a guest that isn't waiting gets it at its next wait. No input is made up and no scope is closed. Every guest of the context is woken, whoever the work is for; one with nothing else to look at waits again. The object table is only held to list a scope's instances, never while waking them. The header now says that the host may end a wait with WGUIWaitReasons_Woken when it has other work for the program, not only when the program called wguiInstanceWake. Tests: a wake from another thread ends a blocked wait at once; it reaches each live scope and makes up no input, another context's scopes are left alone; and the waker doesn't keep a dropped context alive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
syrusakbary
left a comment
syrusakbary
left a comment
There was a problem hiding this comment.
Reviewed the callback's ownership, context/scope lifetime, instance enumeration and lock release, sticky wake behavior, guest wait loop, generated contract changes, and the WebGPU event-waker producer. I found no blocking implementation defect in this diff. Merge after #2 as described, so an external wake also closes the registration gap.
Waking the guest to re-check its work, while leaving callback execution on the guest's thread, is an appropriate Wasmer adaptation of Emscripten's side-channel notification model. The weak context reference and releasing the object-table lock before calling wait.wake() keep this hook appropriately narrow.
Validation: all 84 native default-feature unit tests passed; generated files match the spec; all 86 unit tests passed with #2 applied too. I also mutation-tested the latency test: deleting the actual notification from WaitSet::wake still passes it (~58 ms). The inline comment explains how to guard the behavior this feature is intended to provide. The end-to-end WebGPU→GUI wiring remains deferred, as stated in the PR; it was not exercised by these unit tests.
| assert_eq!(rig.read_u32(SIZE), wait_reasons::WOKEN, "took {took:?}"); | ||
| // Long before the host would have ended the wait itself (Interrupted). | ||
| assert!( | ||
| took < std::time::Duration::from_millis(240), | ||
| "took {took:?}" |
There was a problem hiding this comment.
[P3] Make the latency regression fail when unparking is removed
This assertion checks against the 250 ms interrupt, but the wait loop already rechecks the sticky flag every 50 ms (WAIT_SLICE). I removed self.notify() from WaitSet::wake, leaving only woken.store(true, ...), and this test still passed in 58.4 ms: the guest observed the flag after polling, without being unparked. Therefore it does not protect the prompt external wake that this PR adds. Please add a coordinated test using the real event_waker against a registered waiter with a long park timeout (or a test-only wait-slice override), and a bounded completion channel, so removing notification reliably fails. Retain the ABI assertion for WOKEN; avoid simply tightening the wall-clock threshold, which would make CI scheduling affect the result.
The test of GuiCtx::event_waker passed with the unpark taken out of WaitSet::wake: a waiting guest looks at the sticky flag every 50 ms (WAIT_SLICE), well within the 250 ms the test allowed. The guest now waits on a thread of its own, with WAIT_SLICE and INTERRUPT_AFTER stretched to a minute for that thread, and the test wakes it through event_waker once park says the guest is done looking and parks (both test-only). The wait then has to end with WGUIWaitReasons_Woken within 5 seconds; without the unpark it doesn't. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#3 now has main, with #1's wait count and #2's handshake between waiting and waking: - `notify` bumps the counter a suspended wait sleeps on, tells it, then takes #2's fence before it looks at the list of parked threads. - `park` checks a pending wake after the fence, as #2 has it; `wait_step` still takes it. - The parking wait counts the guest's waits once per call, as #1 has it, and the suspending wait now does too, on its first look: otherwise a window redrawn while the guest waits by suspending would be "asked for while drawing" from then on, and `eagerRedraw` would never answer at once. - Cargo.toml and the Makefile keep both sides' test setups (loom and the web wait test). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A guest waiting for input in
wguiInstanceWaitEventsonly looks at other work, such as a WebGPU event, when its wait ends. If that work completes on another thread meanwhile, the guest sees it up to 250 ms later, when the host interrupts the wait, or at its timeout. This lets the embedder end the wait as soon as such work is ready.Best merged after #2: external wakes come often and from other threads, so they hit that race more than
wguiInstanceWakedoes.Change
GuiCtx::event_waker()returns anArc<dyn Fn() + Send + Sync>that ends the wait of every guest of the context, aswguiInstanceWakedoes. It can be called from any thread. It holds only a weak reference to the context, so work that outlives the context can't keep it alive.GuiRuntimeControl::wake_all()does the same from a runtime control.spec/gui.json, regeneratedgui.handsys): a wait can now end withWGUIWaitReasons_Wokenwhen the host has other work for the program, not only afterwguiInstanceWake.How it behaves:
It's meant for
WebGpuCtxBuilder::event_waker, from wasmerio/webgpu#2. Wiring the two together ingui_wasmerwaits for that PR: CI builds against webgpu'swebgpu-guibranch, which doesn't have it yet.Tests
an_external_wake_ends_a_wait_at_once: a guest thread waits withWAIT_FOREVER, its wait slice and interrupt stretched to a minute (a#[cfg(test)]override). Once a#[cfg(test)]hook says it has parked, the test calls the realevent_waker(), and the wait must returnWokenwithin 5 s. So the wake has to unpark the guest: it can't be caught at the next 50 ms look. Withnotify()removed fromWaitSet::wake, it fails (5 of 5 runs); the earlier version of the test still passed.external_wake_is_sticky_for_live_scopes_without_synthetic_input: a wake before the wait is kept and consumed once, and makes up no input. Another context's scopes are left alone, and a closed scope has nothing left to wake.external_waker_does_not_retain_a_retired_context: the waker doesn't keep a dropped context alive, and calling it afterwards is harmless.Locally (macOS arm64, Rust 1.95; also clippy on nightly 1.100):
make check(incl.gen.py --check, check-web)make testan_external_wake_ends_a_wait_at_once, 20 runs alone and 20 beside the whole suite🤖 Generated with Claude Code