wasm: switch threaded WASI runner to Wasmer - #2725
Merged
cpunion merged 6 commits intoOct 4, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: Migrate WASI runner from WAMR/iwasm to Wasmer
A clean, well-tested migration. Flag strings are consistent across the many call sites (crosscompile.go, wasi.json, dev/*.sh, dev/*.py, test/goroot/*.go, test/buildcache/test.sh, llgo.yml), the host contract tests were updated to match the new Wasmer argv (volume mapping, --env whitelist, -- separator, Windows :/work naming), and no dangling iwasm/build_iwasm.sh references remain in code. dev/install_wasmer.sh pins per-platform SHA-256 digests, verifies before chmod/exec, and fails closed on an unsupported platform.
The notes below are mostly non-blocking; the two worth a reviewer decision are the --disable-cache performance tradeoff and the dropped --max-threads=128.
Behavioral changes worth confirming (not inline)
recovernow runs under wasip1. This PR deletestest/buildcache/recover_wasip1.go(which stubbedverifyRecoverCacheto a no-op because iwasm lacked the setjmp/longjmp imports) and relaxesrecover.go/dep1/recover.goto//go:build llgo, so cross-packagedefer/recovernow executes under wasip1 on every WASM cache build. This is the single highest-risk behavioral change and only correct if Wasmer's standard-EH path fully supports LLGo's defer/recover lowering. Worth explicitly confirming the buildcache WASM run was observed to pass (not skipped for a missingwasmer).
Stale WAMR comments outside the diff (migration missed these)
These files were not touched by the PR but now carry comments that contradict the Wasmer runner. Consider a follow-up sweep:
dev/wasmstdlib/full.go:138,166-167,171,174-175,179— timeout justifications cite "WAMR classic Release" / "interpreter run" and WAMR-era second counts (e.g. 217s, 50/75s TLS). Under--craneliftWasmer is a compiler, not an interpreter, so these attributions and figures are stale.test/std/crypto/x509/x509_test.go:254,test/std/os/os_test.go:1632,test/std/os/go125_symbols_test.go:27,test/std/os/exec/exec_wasm_test.go:75— rationale/behavior still attributed to "WAMR". The guarded code paths now run under Wasmer; reword or re-verify that Wasmer exhibits the same WASI preview1 rights //dev/nullbehavior.
Minor
dev/install_wasmer.sh:45-50— if neithersha256sumnorshasumis present, the comparison still runs and reports a misleading "checksum mismatch" rather than "no SHA-256 tool available". Low impact.internal/build/run.gowasiHostDirectoriesignores the passedtempDiron non-Windows (hardcodes/tmp), so a customTMPDIRmaps the guest to host/tmp. Matches prior behavior; thetempDirparameter is only honored on Windows — minor API inconsistency.
| // before execution and also grants Go's default /tmp directory. | ||
| // The 64-client select stress needs more than 64 concurrent pthreads. | ||
| WASIThreadedEmulator = `iwasm --max-threads=128 --stack-size=1048576 --heap-size=0 --dir=. --dir=/tmp "{}"` | ||
| WASIThreadedEmulator = `wasmer run --cranelift --disable-cache --enable-exceptions --enable-simd --stack-size=1048576 --volume=. --volume=/tmp "{}"` |
Contributor
There was a problem hiding this comment.
--disable-cache + --cranelift is likely the worst-case pairing for these short-lived, repeatedly-run test binaries. --disable-cache forces a full Cranelift (optimizing, slow-to-compile) recompile on every invocation, so the compile cost is paid in full while the faster execution is never amortized. This bites wherever the same module runs more than once: dev/test_wasm_wasi_threads.py runs test/go 3x and the deferred-goexit module 10x; test/buildcache/test.sh runs buildcache.wasm ~6x. If disabling the cache is intentional (clean state per run), a one-line rationale comment here would help, since this template is duplicated verbatim across ~8 call sites and the decision is otherwise hard to revisit. Otherwise consider singlepass and/or dropping --disable-cache for the repeated-run paths.
| // before execution and also grants Go's default /tmp directory. | ||
| // The 64-client select stress needs more than 64 concurrent pthreads. | ||
| WASIThreadedEmulator = `iwasm --max-threads=128 --stack-size=1048576 --heap-size=0 --dir=. --dir=/tmp "{}"` | ||
| WASIThreadedEmulator = `wasmer run --cranelift --disable-cache --enable-exceptions --enable-simd --stack-size=1048576 --volume=. --volume=/tmp "{}"` |
Contributor
There was a problem hiding this comment.
The old WAMR template carried --max-threads=128 with the rationale "The 64-client select stress needs more than 64 concurrent pthreads." The new Wasmer template drops any thread-count flag, and dev/test_wasm_wasi_threads.py still runs TestConcurrentSelectProposeReplyStress. This now relies on Wasmer 7.5.0's default thread ceiling being >= what that stress needs (>64). Please confirm the default is sufficient, or re-add an explicit cap for parity with the previous guarantee — otherwise that test could newly deadlock/fail.
| # The unoptimized testing framework exceeds Wasmtime's locals limit. | ||
| "$RUNNER_TEMP/llgo-bin/llgo" test -O2 -target wasi -emulator \ | ||
| -v -count=1 -timeout=2m ./test/simd/... | ||
| for opt in 0 2; do |
Contributor
There was a problem hiding this comment.
This now runs the full ./test/simd/... emulator suite twice (-O0 and -O2) where it previously ran once, roughly doubling this step's Wasmer time; combined with --disable-cache (recompile per module run) the cost compounds. The step has no dedicated timeout and inherits the job budget — confirm it still fits. If -O0 coverage was the goal, intentional; just flagging the wall-clock impact.
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
reviewed
Oct 4, 2026
cpunion
left a comment
cpunion
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed f8663a224. The migration direction looks reasonable, but I found two P2 gaps in runner configuration, detailed inline: inherited WASI targets bypass the host-contract adaptation, and the independent GOROOT runner still selects Cranelift on Windows.
Validation: the focused build/crosscompile/target/wasmstdlib/GOROOT unit tests passed, including the existing GC mutex-reacquisition regression. The latest 98 checks had no failed or unfinished entries. I verified the inherited-target argument construction on this PR and reproduced the CLI argument rejection with locally installed Wasmer 7.3.0; I also checked the relevant 7.5.0 CLI parsing and Windows build configuration in upstream sources. I did not rerun the full pinned Wasmer 7.5.0 matrix or execute the Windows GOROOT path locally.
Separate existing issue: #2727 records cumulative thread-resource exhaustion after repeated runtime.Goexit(). It reproduces on unchanged main with WAMR as well as this PR with local Wasmer 7.3.0, so it is not attributed to this PR. Both configurations failed in 3/3 repeated runs after completed 225 and before completed 250, while normal-return controls completed all 400 goroutines.
| "wasm-profile": "w32", | ||
| "wasm-provider": "wasi", | ||
| "emulator": "iwasm --max-threads=128 --stack-size=1048576 --heap-size=0 --dir=. --dir=/tmp \"{}\"" | ||
| "emulator": "wasmer run --cranelift --disable-cache --enable-exceptions --enable-simd --stack-size=1048576 --volume=. --volume=/tmp \"{}\"" |
Collaborator
There was a problem hiding this comment.
[P2] Keep the inherited runner template in sync with WASIThreadedEmulator
This JSON template still includes --disable-cache, while crosscompile.WASIThreadedEmulator no longer does. runEmuCmdTo applies the host-contract adaptation only when the entire template equals that constant. The built-in wasi and wasip1 names hide the mismatch because UseWithGOARMAndToolchain explicitly replaces their emulator, but a downstream target inheriting this file does not get that replacement:
{
"inherits": ["wasi"]
}I verified that such a target retains --volume=. and gets no guest PWD/PATH, no -- argument separator, and no Windows V8/volume adaptation. Passing a guest flag such as -test.v then fails at the Wasmer CLI instead of running the program:
error: unexpected argument '-t' found
Please synchronize this template with the constant and add a regression exercising an inherited target through the runner, not only the two built-in target names.
| args := []string{"--max-threads=128", "--stack-size=1048576", "--heap-size=0", "--dir=" + dir, "--dir=/tmp", artifact} | ||
| return "iwasm", append(args, programArgs...), gorootRuntimeEnv(env), nil | ||
| if p.runner == "wasmer" { | ||
| args := []string{"run", "--cranelift", "--disable-cache", "--enable-exceptions", "--enable-simd", "--stack-size=1048576", "--volume=" + dir, "--volume=/tmp", artifact, "--"} |
Collaborator
There was a problem hiding this comment.
[P2] Apply the Windows runner contract to GOROOT execution too
This independent execution path always selects --cranelift, but the official Windows Wasmer 7.5.0 archive installed by this PR only supplies V8. Unlike the public run/test path, gorootArtifactCommand does not go through runEmuCmdTo, so Windows -wasm-profile=W32-WASI fails before either the Go baseline or LLGo artifact can run. The current unit tests assert this same hard-coded command on every host, so they do not detect the missing adaptation.
Please apply the same Windows backend selection (--v8) and explicit host-to-guest volume mappings used by the public runner, and make the regression expectations platform-aware.
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.
Stock WAMR's classic interpreter cannot execute fixed-width SIMD, while LLGo's threaded WASI programs also require shared memory and SjLj exception handling. Switch W32 execution to the unmodified Wasmer 7.5.0 CLI and emit standard Wasm EH directly from LLVM. This is an alternative to maintaining the interpreter patch in #2723.
-wasm-use-legacy-eh=falsefor compilation and LTO linking. Adaptpthread_exittowasix_32v1.thread_exit; wasi-libc continues to provide thread startup and TLS.--. Synchronize the inheritedwasi.jsontemplate with the public runner, and apply Windows directory mapping to the independent GOROOT runner for both Go and LLGo artifacts.RUST_LOG=offto prevent engine tracing from contaminating guest-output assertions; ordinary runs retain the caller's logging settings. Guest output is never filtered.Validation of the review fixes on macOS arm64, Go 1.27.0, LLVM/LLD 22.1.8 and official Wasmer 7.5.0, without an explicit backend:
-test.vthrough the runner; previously Wasmer rejected it as a host option.dev/test_wasm_wasi_threads.pyacceptance driver passed, including cold/warm cache reuse and guest stdout/stderr/exit status, GC/nogc exceptions, startup, threaded GC, arena boundaries, filesystems, finalizers, reflection/GC races, select stress, standard-library packages and the GOROOT sentinel.test/simd/...passed at O0 and O2. A separateW32-WASIGOROOThelloworld.gocomparison passed with both Go and LLGo artifacts.Earlier validation includes W32 Go/C++ DWARF and execution at O0/O2 with embedded/external debug artifacts, cold/warm build-cache fixtures, Linux GC allocator-handoff stress, and Windows MSVC/MinGW build-cache execution. All 98 checks on the previous head
f8663a224completed without failures; CI for the updated branch is pending.Scope notes: this is not a full standard-library compatibility audit. Wasmer 7.5.0 publishes no macOS Intel CLI archive, so that host requires a source-built CLI. The original Thin/Full LTO flag-only runs did not exercise LTO because WASI omitted
-flto; #2726 fixes that separately and verifies genuine LTO artifacts. The cumulative Goexit resource-exhaustion report in #2727 also reproduces on unchanged main and remains a separate issue.