runtime/wasm: add single-worker Asyncify scheduler - #2192
Conversation
c241c73 to
e32655c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Review: single-worker Asyncify WASM scheduler
Solid, well-structured PR. The G/M/P backend split (pthread / wasm-fiber / baremetal) is cleanly partitioned by mutually-exclusive build tags, the intrusive runqueue is allocation-free and O(1), the emscripten_fiber_t layout is guarded by a _Static_assert, and retired-G reaping is correctly deferred to the next resumed G on every switch path. Test coverage (Node-executed scheduler test, G/M/P ownership tests) is good.
Findings below. The most important is #1 — the new scheduler's fail-fast guards do not actually fail fast, because the compiled fatal is a non-aborting stub. Inline comments carry the concrete locations.
1. (High) fatal() returns instead of aborting — every new scheduler guard falls through
The compiled fatal is the stub at runtime/internal/lib/runtime/.../stubs.go:119, which only prints and returns (the real aborting fatal in panic.go is inside a /* ... */ block spanning lines 260–1412, so it is not built). The new scheduler writes its invariant guards as fatal(...); return and assumes fatal never returns. Because it does return, illegal states continue executing:
resumeDeadWasmG(proc_wasm.go:222) unwinds a dead fiber normally after the trailingfatal.casgstatus(proc_atomic.go:39-43) does not apply the CAS on an illegal transition, then every caller proceeds as if the new status took effect.goschedBackend/gopark/goexitBackendreturnafter an empty-runqfatal, leaving a_Gdead/_GwaitingG as the running context.
Recommend either making the compiled fatal actually abort the process (e.g. abort/c.Exit) or replacing these fatal(...); return sites with an explicit abort. This is pre-existing at the stub, but this PR is where the assumption becomes load-bearing.
2. (Low) goexitBackend reports a false deadlock when a worker exits while main is parked
goexitBackend (proc_wasm.go:198) pops the runq before marking gp dead; if a normal goroutine returns while main is parked (empty runq), it hits the "no goroutines (main called runtime.Goexit) - deadlock!" path even though this is not main and not Goexit. Combined with #1 (the return is reached), the finished goroutine's fiber then unwinds with gp still _Grunning and unreaped. Please confirm the intended "worker exits while main waits" handling.
3. (Low) Inconsistent ID generation: nextGoid/nextMid add +1, nextPid does not
See inline comment on proc_atomic.go. Not currently a functional bug (tests only assert non-zero/unique), but the undocumented asymmetry is a latent hazard — make the three uniform or comment the intent.
4. (Low) Stale/misleading doc on the shared g type
See inline comment on runtime2.go:38. runtime2.go has no build tag, so this g is shared by all backends, yet the comment describes only the pthread per-G M/P model — the exact model the new wasm backend (single shared wasmSched.m/.p) is built to violate. The "when those facilities are added" sentence is also stale: this PR adds suspend/resume.
5. (Info) Gosched doc overstates behavior on pthread
Gosched (proc.go:104) is documented as yielding to another goroutine, but goschedBackend is a no-op on the pthread backend (proc_pthread.go:105). Consider noting that yielding is backend-dependent.
Non-blocking follow-ups
- No stack/context pool: each
goon wasm does 2×AllocRoot(~128KB: 64KB G stack + ≥64KB asyncify stack) + 2× free per goroutine, never pooled. A free-list keyed on the default size would remove steady-state churn under high goroutine turnover. resumeDeadWasmGis a near-duplicate ofresumeWasmGbut omits the nil-guards onold/next/next.context; consider factoring the shared switch sequence.
|
Review follow-up pushed in
Local validation passed on macOS arm64 with Go 1.26.5 and an independent Go 1.24.11 GOROOT, Emscripten 4.0.21, and Node 25.2.1. The exact Go 1.24.2 artifact path remains covered by CI because its locally downloaded GOROOT lives under |
8882b7c to
0e631e5
Compare
|
Disposition of the two non-blocking review follow-ups:
|
…orker-asyncify-scheduler # Conflicts: # internal/build/source_patch_test.go
…orker-asyncify-scheduler # Conflicts: # runtime/internal/runtime/z_default.go
Part of #2152. #2166 is merged and provides the runtime-owned G/M/P boundary used here.
Problem
#2166 moves
gostatement creation behindruntime.NewProc, but its only execution backend is one pthread per G.js/wasmhas no scheduler that can suspend multiple G contexts on one Worker, so a runnable WebAssembly goroutine path is still missing.The two supported js/wasm entry points also need distinct, unambiguous data models:
GOOS=js GOARCH=wasm llgo buildkeeps Go's 64-bit word model and uses Emscripten Memory64;llgo build -target wasmimpliesGOOS=js GOARCH=wasmbut selects the configured wasm32 model.They must not require redundant GOOS/GOARCH flags or share incompatible package-cache entries.
Change
js/wasmbackend with one M, one P, an allocation-free FIFO run queue, and Emscripten Fiber contexts backed by Asyncify;runtime.Gosched, park/ready test points, normal return, panic/defer/recover, andruntime.Goexitthrough that scheduler;-target wasmthrough its target configuration before platform-dependent output and package loading;tinygo.wasmtarget tag and target name for wasm32, without adding another environment variable or build tag;GOOS=js GOARCH=wasmaswasm64-unknown-emscriptenwithMEMORY64=1, while the named target remainswasm32-unknown-emscripten;longfor Memory64 and 32-bit Clongfor the configured target;ssize_tfollow pointer width, avoiding an i32/i64writesignature mismatch under Memory64;.js/.mjsoutput and include Node in the existing Emscripten environment list;nogcruntime for wasm targets, because BDWGC is unavailable there;Addreturns the previous value.The default per-G reservation is one 64 KiB C stack plus one 64 KiB Asyncify area. The main G allocates only its Asyncify area on first switch. No pthread or Worker is created per G.
The Emscripten Fiber API is used only as the raw unwind/rewind context adapter. The scheduler does not use Promise-owned Asyncify flow. #2079 currently maps TLS and GLS to one physical owner; separating GLS per logical G on a multiplexed worker requires a later locality-layout change and is not partially implemented here.
Execution flow
NewProcallocates G state and two stack areas, initializes a Fiber, and appends the G to the local FIFO.Goschedrequeues the current G; park leaves it waiting; exit marks it dead.Scope
This is the single-Worker execution slice, not complete
GOOS=jssupport. It does not add channels, select, timers, host async I/O, safe-point preemption, GC roots, browser Workers, or a WASI Asyncify scheduler. The existing WASI path is covered only as a build regression.Memory64 is currently limited to the Emscripten js host. The pinned WASI SDK 25 provides only wasm32 sysroots and libraries, so raw
wasip1/wasmretains its existing wasm32 compatibility path.The independent diff over the merged #2166/main base is
+1258/-160across 38 files. 469 added lines are tests and CI fixtures; the pthread portion is primarily a mechanical move out of the common lifecycle file.Validation
llgo build -target wasmproduces wasm32 and executes the scheduler fixture;GOOS=js GOARCH=wasm llgo buildproduces Memory64 and executes the same fixture;uintptrand Clongsizes (4 or 8 bytes);test/llgoext, rawjs/wasm, and rawwasip1/wasmbuilds pass.--memory=15g --cpus=2: focused build/crosscompile/SSA tests and runtime module tests pass.wasmtarget.test/stdpackages but was terminated by the 30-minute job cap; this PR raises only that safety cap to 40 minutes. The rerun completes successfully in 37m46s; currentmainis about 25 minutes.runtime/internal/runqueuehas 100% statement coverage; target resolution, type-model selection, target triples, output naming, and both fixture model assertions have focused coverage.No test or previously runnable path is skipped or ignored.