feat(wasm): make WAMR threads the default W32 profile - #2697
Merged
Merged
Conversation
cpunion
force-pushed
the
codex/wasi-default-threads
branch
from
September 29, 2026 02:53
50bab17 to
8652066
Compare
cpunion
marked this pull request as ready for review
September 29, 2026 03:21
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: WASI threads as default for the WASM GC runtime
This is a large, carefully-constructed change. The concurrency design (epoch-based stop-the-world, root publishing before blocking C calls, cooperative safepoints) is coherent, and the build-tag matrix across the many _default/_wasi_threads file pairs is consistent and mutually exclusive. Memory-safety review of the C _wrap coordination layer found no defects, and the new crypto PEM files are legitimate test fixtures (not leaked secrets).
Findings below, ordered by impact. Inline comments carry the specific ones; a few cross-cutting items are summarized here.
Performance (main concern): segmentForBlock/segmentForAddress (segments.go) linearly scan up to heapSegmentCount segments on every call, and they now back the per-block state helpers (gcStateOf, gcSetState, gcMarkFree, ...) used in the allocator, mark, and sweep hot loops. gcStateOf scans twice and gcSetState three times per block. The mark/sweep loops already iterate segment-by-segment yet re-resolve the segment for each block, turning O(blocks) into ~O(blocks × segments). Since the segmented allocator grows by doubling arenas, GC pauses grow with arena count. Consider threading the known *heapSegment into the state helpers within the segment loops, fast-pathing heapSegmentCount == 1, and collapsing the redundant re-scans in gcStateOf/gcSetState. (See inline note on sweep.)
Polling cadence: condWait (inline) and the timer loop (time_heap_llgo.go / timerGCWaitQuantum) both fall back to a fixed 20ms wakeup so threads reach GC safepoints. The C layer already counts genuinely-blocked threads via world_blocked in llgo_wasi_gc_stop, so a thread parked in pthread_cond_timedwait is already counted toward quorum — a longer quantum (or on-demand wake) would cut idle-program CPU churn that scales with the number of blocked goroutines. At minimum, name the 20*1e6 literal and share it with timerGCWaitQuantum.
Maintainability: configureHeap (gc_tinygo.go) still writes heapSegments[0].end/.metadata/.last even though initGC now uses addHeapSegment; the two functions duplicate the same metadata-layout formula. Worth confirming configureHeap still has a live caller and, if so, having it delegate to addHeapSegment.
Docs: test/goroot/README.md:76 still says LLGO_WASI_THREADS=1 selects WAMR and mentions a "Wasmtime adapter" for W32 — but the harness now hardcodes runner: "iwasm" for W32-WASI unconditionally (no env gate). doc/wasm-wasi-threads.md's "official Go reference tests may still use Wasmtime" is likewise stale for the goroot comparison (only the separate wasmstdlib reference profile still uses wasmtime).
Findings without inline locations
internal/build/run.go:333: This directLLGO_WASM_RUNTIME=iwasmrun path still emits the old--stack-size=819200000 --heap-size=800000000with no--max-threads, while the standardizedWASIThreadedEmulatorinvocation (used by tests, targets/wasi.json, the workflow, goroot, and wasmstdlib) is nowiwasm --max-threads=128 --stack-size=1048576 --heap-size=0 --dir=. --dir=/tmp. Since threads/shared memory are now the default module shape, this path may fail to spawn workers (no--max-threads). Align it withWASIThreadedEmulatoror document the exemption.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
cpunion
force-pushed
the
codex/wasi-default-threads
branch
from
September 29, 2026 04:53
9255796 to
a7639a8
Compare
Collaborator
Author
|
Addressed the review and CI configuration findings. Rebased through #2695 onto current main and kept this PR ready for review. The inherited collector update removes periodic condition/timer polling, reuses segment metadata and names the uncooperative-C timeout. configureHeap remains live for contiguous browser/embedded heap growth: it shares the layout helper, but cannot call addHeapSegment because that would erase live allocation metadata. The direct LLGO_WASM_RUNTIME=iwasm path now uses the standard threaded runner, including thread/stack/heap limits, preopens and PWD/PATH handling; a command-level regression verifies it. GOROOT docs now consistently describe unconditional WAMR execution for both W32 artifacts. Fixed the obsolete Wasmtime assertion and the missing threads source tag in the reflection bridge test. DWARF validation now resolves each unit's actual filename index and prints the relevant line table if resolution fails; parser tests retain rejection of missing source rows. All six J32/J64/W32 O0/O2 debug/runtime combinations pass locally with the build cache enabled, as do the rebased target/SSA/runner/profile tests. The earlier Linux O2 failure has not reproduced locally, so fresh Linux CI is still needed to confirm that check. No debug validation or coverage gate was disabled. |
cpunion
force-pushed
the
codex/wasi-default-threads
branch
from
September 29, 2026 05:39
e2c3c3a to
49c8721
Compare
Collaborator
Author
|
The W32 O2 DWARF failure is now reproduced and fixed. Clang implicitly runs wasm-opt when it finds it on PATH, without preserving debug lines. This only showed up in the CI environment because Binaryen was on PATH; setting WASMOPT alone did not reproduce it locally. Disable the implicit pass for direct Wasm Clang links, including the new default WASI-thread profile. The explicit Asyncify and Emscripten pipelines keep their existing behavior. Verified the old failure and fixed behavior on both macOS and Linux amd64 with Binaryen on PATH. All six J32/J64/W32 O0/O2 final-DWARF/source-line/runtime checks pass, and linker argument regressions cover both Asyncify and non-Asyncify Clang links. Rebased onto #2695 at fb2a438 to include its GC test-budget separation and per-worker callback bridge fix. Current head: 49c8721. Fresh CI is queued; all existing review threads are resolved and this PR remains ready for review. |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
cpunion
force-pushed
the
codex/wasi-default-threads
branch
from
September 29, 2026 09:29
49c8721 to
924370c
Compare
cpunion
force-pushed
the
codex/wasi-default-threads
branch
2 times, most recently
from
September 29, 2026 11:34
8dc7714 to
a46e23d
Compare
visualfc
approved these changes
Sep 30, 2026
cpunion
force-pushed
the
codex/wasi-default-threads
branch
from
September 30, 2026 11:14
a46e23d to
79fd653
Compare
5 of 11 tasks
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.
W32 now uses the WAMR pthread runtime by default.
llgo run -target W32and rawGOOS=wasip1 GOARCH=wasmbuilds use the same shared-memory/threaded profile, without an opt-in environment variable. Explicitly disablingLLGO_WASI_THREADSreports a migration error. The obsolete single-threaded WASI scheduler, context switching and GC implementation are removed.The target definitions, package test runners, DWARF checks, CI and execution documentation follow that default. Browser single-worker scheduling and native/embedded pthread behavior are unchanged.
Rebased onto main at 4d51d8e, including merged #2669, #2695, and #2696. There are no outstanding PR dependencies. This contribution contains only the W32 default, build, runner, DWARF, and test changes; all six contribution patches are preserved unchanged.
Validation completed locally:
LLGO_WASI_THREADSunset.The GC/allocator fixes from #2695 now come from main. Its complete WAMR
test/gorun passes all 242 top-level tests, including the previously timing-out concurrent function-info lookup. Named/raw WASI public commands and W32 O0/O2 DWARF/runtime checks were re-run after integrating that fix and pass. EH validation is consolidated in #2695.The full standard-library matrix has not been re-audited on every host at this head; see #2695 for the exact audit slices and results. Cross-platform CI remains the merge gate.
Review/CI follow-up: inherited the segment/safepoint/condition-wait fixes from #2695; updated stale Wasmtime assertions and reflect bridge source tags; routed direct
LLGO_WASM_RUNTIME=iwasmruns through the same threaded host contract; corrected GOROOT/reference-runner documentation. DWARF source validation now uses each unit's actual filename index and prints the relevant table on failure. Added parser regressions and retained strict source-line resolution checks. After rebase, focused Go configuration/SSA/runner tests and all J32/J64/W32 O0/O2 DWARF/runtime combinations with the build cache enabled pass locally.Final local verification at a7639a8: complete WAMR
test/gopasses all 242 top-level tests using the runner's absolute preopens/PWD contract; all 10 public CLI checks pass. Linux amd64 LLVM 22 also passes W32 O0/O2 final DWARF checks with the build cache enabled. The CI-only DWARF failure is now reproduced on both macOS and Linux amd64: when wasm-opt is present on PATH, Clang implicitly runs an unrequested Binaryen pass without debug preservation for W32 O2. The fix disables implicit passes for direct Wasm Clang links, including WASI threads. All six J32/J64/W32 O0/O2 checks pass with Binaryen on PATH after the fix, as does the Linux amd64 W32 O2 regression. The WAMR GC acceptance suite now runs both 20-goroutine startup shapes with separate 300-second execution budgets, preserving every test and repetition; Linux amd64 validation passes. The per-worker callback bridge fix is supplied by main via #2700, with Memory32/Memory64 two-worker output/filesystem regressions passing.Earlier validation at main 9853dc8: host configuration/runner/SSA tests and Memory32/Memory64 callback checks with one and two workers pass. The previous CI failure in
TestStdinReadDoesNotFreezeSchedulerclassified a 162 ms timer as blocking even though the read took 442 ms. The stdin/fsync checks now assert timer-before-completion ordering and count calls to the forbidden Sync API explicitly. Both checks pass five repetitions in each of the four configurations (40 executions).Current rebase validation at main 4d51d8e: cross-compilation, target definitions, command runners, wasmstdlib, browser proxy, focused Wasm/WASI build and SSA tests, DWARF parser regressions, and shell syntax checks pass. With LLGo Binaryen llgo-v132.3 on PATH, W32 O0/O2 final DWARF verification, Go/C++ source resolution, and runtime execution pass. Named WASI and raw GOOS=wasip1 GOARCH=wasm both run the threaded-GC fixture successfully with LLGO_WASI_THREADS unset.