runtime/wasm: add bounded browser workers with realm-safe callbacks - #2661
Merged
Merged
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: wasm workers support
This is a large, carefully-engineered change. The _single/_workers/_stub build-tag split is disciplined, the STW epoch protocol and the double-checked wait-queue locking are correct, the lost-wakeup guard (snapshot the wake sequence before checking) is right, and the CI supply-chain hardening (in-repo pinned checksums, regex-validated, verified against a locally-computed hash) is a real improvement. Test coverage is solid and the runtime code is unusually well-commented.
Findings below. The one worth resolving before merge is the llgo run exit-code regression (inline). Everything else is minor / latent / perf-tuning.
Additional (not anchored inline):
runtime/internal/runtime/scheduler_events_wasm_workers.go(loadWasmEventHooks) — per-iteration global lock.waitWasmWorkerRunqandcooperativeSafepointSlowcallloadWasmEventHooks()on every pass, each acquiring the sharedwasmSchedulerEventHooks.lockand copying the struct. The three hook pointers are written only at startup, so every worker's idle loop serializes through one mutex on the busiest path. Publishing/reading the pointers via atomics (or copy-on-write) would make the read lock-free.runtime/internal/lib/syscall/js/emval_release_workers.go— finalizer wake storm. EachreleaseEmvalfans outWakeWasmCallbackPoll→wakeAllWasmWorkers, so a GC cycle finalizing N JS values wakes every worker N times;pollEmvalReleasesalso scans the entire pending list per worker (O(W·N)). Batching releases (drain-and-wake once) and using a per-owner queue would remove the per-handle, per-worker storm.runtime/internal/runtime/proc_wasm_workers.go(wakeWasmWorker) — unconditional notify.Wakeis issued on every enqueue even when the target worker is already running. Theworker.wakesequence counter already provides lost-wake protection, so a "parked" flag would let enqueues skip the notify for running workers (the common case).doc/wasm-proposal.md:88(and CN mirror ~208) —GoIndependentwording. "starts work in the pool only when its closure carries no JS value or thread-local C state" reads as a runtime decision, butSpawnIndependentWasmGalways dispatches; "no JS value / no thread-local C state" is a caller obligation. The Go doc comment ingo_workers.gophrases this correctly ("The closure must not carry..."); align the proposal.runtime/internal/runtime/tinygogc/gc_wasm_js_workers.go— empty__libc_free. Intentionally-empty (non-moving GC sweeps) but, unlike the rest of this PR, has no comment. A one-line note would prevent a future "fix".
No high- or medium-severity security findings; the env-var, module-name, path, checksum, and command-execution trust boundaries are all validated correctly. The --no-sandbox Chrome runner is acceptable for CI-scoped, self-built fixtures only — do not reuse it for untrusted content.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
This was referenced Sep 24, 2026
cpunion
force-pushed
the
codex/wasm-workers-config
branch
from
September 26, 2026 14:39
22b45fc to
fa62323
Compare
cpunion
force-pushed
the
codex/wasm-workers-config
branch
from
September 27, 2026 15:06
48d850e to
0f7cb03
Compare
Collaborator
Author
|
Thanks for the detailed review. Commit The event-hook read lock, emval-release fan-out, and worker wake-on-enqueue are valid performance follow-ups. I am leaving those out of this functional PR pending contention measurements, rather than changing three scheduler paths without evidence of the best batching/parking strategy. |
visualfc
reviewed
Sep 28, 2026
visualfc
left a comment
visualfc
left a comment
Collaborator
There was a problem hiding this comment.
Follow-up review on the current head. The earlier blocking item (guest exit code on llgo run) is fixed, and the STW / waiter / realm-affinity design still looks sound. Two functional gaps below; neither needs to block this opt-in merge.
The event-hook read lock, emval-release wake storm, and unconditional worker notify remain valid performance follow-ups.
visualfc
approved these changes
Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With one browser M, goroutines cannot run in parallel even when Emscripten has Web Workers.
LLGO_WASM_WORKERS=2through16now creates a fixed pool of physical workers, each with an M/P and run queue, and schedules many Fiber-backed goroutines per worker. The implementation covers cross-worker wakeups, stop-the-world linear GC, TLS/GLS ownership, finalizers, and Node/Chrome execution for J32 and J64. One-worker mode remains available.syscall/js.FuncOfkeeps synchronous results for nested and nonblocking external callbacks; suspended external callbacks resume after the JS event returns. Emscripten emval handles remain in their owning realm. Descendants of a goroutine usingsyscall/jsstay on that worker;runtime/wasmworkers.GoIndependentexplicitly distributes work whose closure has no JS value or thread-local C state. Passingjs.Valuebetween unrelated workers remains unsupported, so bounded mode stays opt-in pending that compatibility work (#2632).The Binaryen pin (#2652) and runner recovery (#2660) are merged. This branch was rebased onto current
main(f59bac1ea), leaving only the browser-worker changes in its diff. Before the pin update, the acceptance script passed locally with Binaryenllgo-v132.2and Emscripten 6.0.8: single-worker J32/J64 and W32 regressions, bounded J32/J64 runtime and hardening fixtures, publicllgo test, and four Chrome COOP/COEP runs. The browser-only instantiation-failure test invokes the runner in the mode where it installsinstantiateWasm.With
llgo-v132.3, a repeated Memory64 callback test exposed a rareinvalid WebAssembly scheduler waiter: the external JS callback or callback poll could takefuncsMuon a system Fiber with no G, then try to park when the lock was contended. Bounded mode keeps its poll hook installed, so it now skips the single-worker-only active-callback count on those system-Fiber paths. A new waiter guard reports any future attempt to create a parkable waiter without a G. The failing test passed 300 repeated J64 and 100 J32 runs after the fix; the single-worker callback group passed 50 repetitions.Review follow-up (
083562916):llgo runnow preserves the guest's exit code for native and emulator runners; a real exit-code-7 run and focused regression tests passed.LLGO_WASM_WORKERS=1and theGoIndependentclosure restriction are documented. The GC-root and timer build tags now partition consistently, and the shared mutex records waiters so an uncontended unlock does not notify and a contended unlock wakes one waiter. Its bounded 1 ms wait remains necessary for stop-the-world polling when a worker is parked. The complete Node/Chrome J32/J64 worker acceptance passed on this head, as did the focused Go tests. Full PR CI is running.