Repository navigation
[WASM W2-B] Complete single-worker runtime and aggregate lowering - #2602
Merged
xushiwei merged 33 commits intoSep 17, 2026
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 summary
Comprehensive review across code quality, performance, security/memory-safety, and documentation accuracy. This is a large, carefully engineered change (WASM W2 runtime lowering) with strong test coverage.
No blocking defects found. The highest-risk areas are implemented soundly:
- wasm32 physical-address overflow (
ssa/expr.go,checkUnsafeBuiltinBounds): the newremaining = maxAddr - addrnon-wrapping comparison correctly traps at the real 32-bit boundary even when the semanticuintptris i64. - Wide-vs-physical pointer split (
ssa/storage.go): 64-bit Go indices truncate to the 32-bit physical width before GEP; integer ABI conversions get an explicit runtime range assertion (fitLLVMValue). - GC roots for lowering-created allocations (
internal/abi/large.go,internal/abi/gcroot.go,cl/uintptr_escapes.go,cl/gcroot.go): aggregate-copy snapshots and//go:uintptrescapespointers are re-rooted, preventing use-after-free. - JS/host boundary (
runtime/internal/wasmjs/_wrap/host.c,embind/_wrap/emval.cpp): copies are length-clamped, memory views re-acquired after growth, and Emscripten control-flow re-thrown correctly.
Verified performance claims (both hold):
finalizer.gopreserveFinalizableObjectsreplaces an O(heapBlocks × finalizers) whole-heap scan with a linear pass over the finalizer list;TestCandidateTraversalScalesLinearlylocks it in.wasm_stack_addresses.golocalizeWasmStackAddressesrematerializes constant alloca-GEP addresses only for functions that callsetjmp, avoiding spilling live addresses across throwing calls.- Bonus:
ssa/gcroot.goappendGCRootPointersnow guards recursion withGCRootCount, so pointer-free aggregates emit zero per-elementextractvalueinstructions (a 128 KiB byte array drops from 131072 to 0).
The findings below are minor robustness/maintainability suggestions, not correctness bugs.
Notes without a single inline location:
runtime/internal/runtime/tinygogc/finalizer.go(markFinalizerObjectBlocked, ~line 212): the remaining per-heap-edge linear scan over the finalizer list is the one non-linear part left in the finalizer path. Not a regression (pre-existing behavior), and fine for small finalizer sets — flagging only because this PR advertises linearizing finalizer traversal. An object-keyed index built once per cycle would remove the quadratic factor for finalizer-heavy workloads.finalizer.go(preserveFinalizableObjects, ~lines 128-190): removingearlierFinalizerForObjectde-dup and queueing all eligible candidates in one pass is memory-safe (the subsequent re-mark keeps objects alive another cycle), but the ordering/dedup semantics for multiple finalizers on one object changed — worth confirming against the intended finalizer-ordering contract.ssa/target.go/internal/build/wasm_reflect.go: two same-namedusesWasmReflectBridgesmethods on different receivers (*Targetvs*wasmProgramUse) with different meanings — legal but easy to conflate; consider distinct names.doc/wasm-proposal.md(Reflection section): the detected bridge entry points also includeSeq/Seq2(seeisWasmReflectBridgeName), which the prose omits. Defensible for a scope summary.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
cpunion
force-pushed
the
codex/wasm-w2-runtime-lowering-20260913
branch
from
September 15, 2026 08:24
c8b6e7d to
291adee
Compare
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
…p object Keep the existing allocation-free registry. Restart after queue removal and clear remaining candidates for the same object so cleanup still waits until a later collection. Validate with the existing lifecycle suite on wasm32, wasm64 and WASI (five repetitions each), plus repeated JS callback and filesystem tests. No new skip or relaxed lifecycle assertion.
Exercise the production registry operations with isolated collector state. Count heap metadata reads instead of timing collection, and check interleaved finalizer/cleanup records and queue removal. Run the check in the existing Wasm runtime job alongside integration fixtures. The old heap scan fails the operation-count checks. Mutations removing same-object candidate clearing or following detached next links fail queue-order checks. The fixed implementation passes ten repetitions.
cpunion
force-pushed
the
codex/wasm-w2-runtime-lowering-20260913
branch
from
September 17, 2026 11:07
3b3a5e5 to
f28418a
Compare
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.
Tracks #2152. Depends on W2-A #2601. Replaces the closed #2580 with a clean review thread; its fixes remain in the branch.
Scope
This does not add multi-worker scheduling or parallel goroutines. Complete applicable
test/**and GOROOT inventory accounting belong to W3.Review boundary
Head
3b3a5e599051, based on W2-A1eb979a9515fand main2db247e43848. The dedicated range is1eb979a9515f..3b3a5e599051: 33 commits, 96 files, +4,392/-292. The original 28 feature commits remain patch-equivalent; follow-ups document event-stack ordering, test aggregate convergence at 1/8/32 nesting levels, correct conservative-GC assumptions in lifecycle acceptance, and use the hosted documentation-check runner proposed independently in #2604. The latest follow-up withdraws the caller-literal size optimization and adds a focused allocation-overhead regression; it changes only two files. The full diff against main includes the prerequisite profile, host, and reflection layers.Validation
Downstream full GOROOT acceptance in W3 exposed two allocation-count regressions in the previous head:
closure.goandfixedbugs/issue4667.go. Allocation stack tracing identifies two 16-byte heap allocations perRecordPanicLocationWasmcall: address-taken temporary string headers introduced by the caller-literal size optimization escape during lowering. This follow-up restoresunsafe.Stringwithout changing the six-scalar ABI ornoinlineboundary. Its regression compares wrapper calls with the same underlying operations: a separate trace confirms that the push already allocates a 72-byte frame snapshot, so the contract is no additional wrapper allocation, not zero total shadow-stack allocation. No GOROOT xfail is added.The corrected isolated GOROOT validation passes both original allocation cases in 30 fresh processes on each of four providers: all 240 execution records have exit status zero and all 240 logs have the expected empty output. Each unmodified source is staged separately, as in the formal GOROOT runner. An earlier raw GoJS probe was rejected because it picked up GOROOT's neighboring
cmplxdivide.c; its zero exit status was not used as acceptance evidence. The same run also reproduced the existinginit1.goMemory32/WASI xfail, which remains unchanged. The final public test-command gate passes: the corrected overhead regression passes on all four paths, as do completetest, compile-onlytest -c, and raw JavaScript/WASIrunchecks. The old implementation fails the same regression (38 allocations through wrappers versus 20 through underlying helpers). Current head3b3a5e599051has now completed all 71 checks successfully, with 98.24% patch coverage against a 94.38% target and no unresolved review threads. Only the conditional non-PR release publication job is skipped. W3 is rebased afterward for full-corpus acceptance.The runtime and size follow-up passes 300 lifecycle executions, the complete runtime/browser gate, and paired output/size checks. Its over-strict zero-total-allocation test was subsequently replaced by the direct-helper comparison above. Restoring correct allocation behavior adds 1,215 B to GoJS and Memory32 Emscripten
cprintf, 1,544 B to Memory64 Emscripten, and 1,260 B to WASI, relative to the withdrawn optimization.printlnadds 1,202–1,555 B andfmtprintf998–1,164 B in the same paired builds. No native implementation changes in this follow-up.The branch inherits W1's
llgo envprofile/provider fix and W2-A's explicit browser-result protocol. LLVM 22 compiler construction, the completeinternal/abisuite, scalar caller-ABI regressions, andgit diff --checkpass locally.The finalizer-focused validation passes 200 fresh lifecycle executions each for J32/Emscripten (wasm32 with the Emscripten JavaScript host), J64/Emscripten (wasm64 with the Emscripten JavaScript host), and W32/WASI (wasm32 with WASI Preview 1), followed by the complete runtime gate including panic/repanic tracebacks and both real-browser providers. All 600 retained logs have the expected completion marker and no unexpected panic/fatal output. After restoring the caller helpers, all 300 additional lifecycle logs were checked again. No diagnostic runtime instrumentation is included in the contribution or its clean validation candidates.
The earlier caller-literal validation measured approximately 1.2 KB/1.5 KB savings for Memory32/Memory64. That optimization is being withdrawn because it introduces heap allocations on the instrumentation path; passing runtime output checks did not establish its allocation behavior. Correct allocation semantics take precedence over these byte savings. A fresh paired size comparison is included in the focused follow-up validation.
Native size guardrail
Native size growth and reproducible performance regressions remain acceptance concerns. Native
cprintfshould stay unchanged where possible, and other native growth should remain small and explained. The current-head paired benchmark at3b3a5e599051measures these executable-file changes against main2db247e43848(default, non-LTO builds):All eight native configurations have completed at the current head. All 192 file/text/data/BSS measurements across the six programs per platform, including LTO, match the previous
fcce9aaf1c2fhead exactly. LTOcprintfhas the same +160 B/zero pattern. Retained Linux ELF files identify the 160 B as ten net additional 16-byte function-information index records:.rodatagrows, while executable code and the actual.bsssection do not. The existing pre-link metadata index can retain records for dead runtime helpers; this does not mean those helpers' machine code is linked.The paired native symbol audit attributes all 767 B of Linux
fmtprintftext growth: 385 B net in reflect argument/result conversion, 224 B in libffi signature/element handling, and 158 B in caller/panic frame handling. The 1,760 B.rodataincrease is predominantly function metadata: 22 additional table/index entries and 31 additional strings account for 1,728 B through the table, string pool, offsets, and index; the remaining 32 B is other read-only content/alignment. Unwind metadata adds 320 B and function-entry metadata 56 B, totaling the measured 2,136 B data growth. LTO reduces the text delta to 539 B and data delta to 1,852 B. Native file growth is therefore bounded and attributed, not evidence of a bulk WASM runtime being linked. Single-sample program run timings are not evidence of performance equivalence; the new formal benchmark round also checks earlier noisy timer/channel medians.WASM growth accounting
WASM growth is recorded and evaluated against the capabilities it adds, rather than an unconditional zero-growth requirement. Further Wasm-specific size optimization is deferred to follow-up work and will not reopen these prerequisite PRs for small byte savings. The current-head paired benchmark at
3b3a5e599051versus main2db247e43848measures these module-byte changes:Compared with the previous
fcce9aaf1c2fhead, all 20 LLGo modules increase by 1,202–1,553 B after restoring allocation-free string construction; glue and official-Go reference sizes are unchanged. The earlier focused probe uses different fixture/output paths, so its byte deltas are retained separately rather than substituted for these formal measurements.For J32/Emscripten
cprintfat the previous head, about 17.3 KB enters in W1's Go64-on-Memory32 model; W2-A is effectively unchanged, and the remaining approximately 3.6 KB enters in W2-B. Symbol inspection associates the Memory32 increase with widened Go operations and checked Go/C boundaries, while W2-B adds panic/caller tracking. These are measured stage boundaries and an attribution based on symbol inspection, not proof that every byte is unavoidable. RawGOOS=js/wasip1baselines also selected different compatibility runtime/provider paths, so their larger differences are not like-for-like ABI comparisons. W3's publishedfe7f99a301c3module-size results match the previousfcce9aaf1c2fhead exactly; its acceptance layer added no module bytes before the pending sequential rebase.The previous head
fcce9aaf1c2fcompleted all 71 checks, with no unresolved review threads. Those narrower green gates did not detect the allocation-count regression subsequently found by W3's full corpus. Historical acceptance and measurements remain in #2580; previous green runs are not reused as current-head evidence.At that previous head, the four LLGo standard-library slices, both official Go reference slices, runtime/browser integration, and public test-command checks passed. Patch coverage was 98.24% against a 94.38% target. All 52 Wasm byte metrics matched
291adee7373a. The second native benchmark round did not reproduce the earlier macOS timer/channel increases: their medians were 22.5%/24.2% lower than the paired base. Linux timer samples had wide overlapping ranges, so isolated median changes are not treated as proof of either regression or equivalence. W3's full compatibility corpus is validated separately in #2603.The earlier W3 runtime failure exposed an intermittent
typed finalizers did not complete. Isolated first-mark tracing identified an initialized static-data word,16779018, conservatively treated as an interior pointer into the pending object at16778976. The same bytes are present in the original Wasm data segment: this is conservative retention, not a damaged queue or a return-signature-specific ABI failure.The fixture now registers three independent objects per signature, requires all 12 signatures to execute, and keeps per-registration invalid/duplicate event checks using a separate channel for each copy. Noncapturing callbacks obtain their channel from the target object so late callbacks cannot encounter a cleared or reassigned global channel. A deterministic collector-registry test separately verifies that a marked object keeps its finalizer pending without blocking other candidates, and becomes eligible when its retaining root disappears. No GC safety rules, skips, or collection deadlines are changed. A separate optional size experiment remains excluded.
A subsequent W3 benchmark round exposed a reproducible sub-nanosecond MinGW
getgdifference. An isolated same-source ABBA comparison measured roughly +0.3 ns for bothgetgand a trivial direct call. After normalizing relocation addresses, disassembly is identical for both benchmark loops (17/19 instructions), the direct-call body (2), and the completegetgfunction (61), including the TLS offsets. Code placement differs. This records a layout-sensitive microbenchmark result, not performance equivalence; it does not justify adding runtime checks or globally increasing alignment and binary size. The probe and its workflow are not included in this PR.An additional ARM64 MinGW same-source comparison at the current head investigates the repeatable full-suite direct-call/atomic-write differences. Eight fresh-process samples per revision, interleaved in ABBA order, give main/head medians of 0.59010/0.59005 ns for a direct call, 0.66375/0.66395 ns for an atomic write, and 1.8055/1.774 ns for
getg. The earlier approximately 0.073/0.22 ns differences do not reproduce in this isolated fixture. Both 17-instruction loops, the two-instruction call body, and the complete four-instruction atomic-write body are identical; thegetghot path retains the same operations and TLS offset. Its cold-path addresses move. This supports tracking the full-suite result as layout-sensitive, not asserting universal native performance equivalence. The validation-only source and workflow are excluded from this PR.The decisive full-package fixture control separates the two variables using 24 interleaved fresh processes, eight per variant:
The repeated direct-call/write differences follow the test-source change, not the compiler/runtime change under fixed sources. The loops retain 17 instructions and their helpers retain 2/4 instructions, with only symbol/data relocations differing; the current fixture shifts the benchmark functions by four bytes and also relocates their callees. This does not identify a particular CPU cache mechanism or prove equivalence for every workload. Other native timing variations remain explicitly monitored: macOS interface-call/defer/timer medians changed direction across successive rounds. No benchmark or runtime code is changed merely to tune these positions, and no diagnostic code/workflow is included in the PR.
A separate 32-bit MinGW control completes the same three-variant experiment (24 fresh processes). Its main / current-with-main-fixture / current-with-current-fixture medians are 0.93645 / 1.11100 / 0.94240 ns for
getg, and 0.88815 / 1.11250 / 0.88860 ns for a direct call. Atomic writes stay at about 7.98 ns and channel handoff at about 593–604 ns. Unlike ARM64, the fixed-fixture head is slower in the two tiny calls, while the current fixture returns close to the baseline. Complete disassembly, including internal function-info/PC-site anchors, confirms 17 instructions in each loop, 3 in the direct callee, and 109 ingetgfor all variants; changes are symbol/data relocations, not additional operations or a changed TLS slot. This records a real layout-sensitive sub-nanosecond effect, not a zero-regression guarantee. It does not justify globally changing function alignment, growing native programs, or changing runtime algorithms to fit one benchmark layout. The formal 386 results and this different controlled-run result are retained separately rather than conflated.