Repository navigation
Conversation
4 of 8 tasks
Collaborator
Author
|
@fennoai please review the opt-in Wasm logical goroutine registry, caller-frame instrumentation and standalone decoder, especially GC/lifetime safety, paused-worker consistency and default-build overhead. Full compiler/browser/debugger/schema tests and the actual Chrome Memory32/64 1/2/4-worker matrix pass locally, including parked recursive stacks after GC and record removal after exit. The frontend still requires pausing other worker threads before shared-memory inspection; it does not claim automatic worker control or parked-frame locals. |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: WASM debug runtime + goroutines panel
Thorough, well-engineered PR. The lock-free registry snapshot protocol (epoch at the registry level, per-node odd/even sequence guards with begin/end double-reads, re-verified in the JS reader after the walk) is clean and well-tested (Memory32/64, 1/2/4 workers, corruption cases). Build-tag partitioning across the stub / wasm-debug / other variants fully covers with no overlap. Docs (dev/browser-debugging.md, doc/debugging.md, the READMEs) and schema_v1.json accurately match the runtime behavior and consumer fields.
Security: no actionable issues. Goroutine strings reach DevTools as structured protocol objects ({type:'string', ...}), not HTML, so no DOM-injection sink is introduced; keep them text-only downstream. Memory reads are bounded (LIMIT = 4096, cycle detection, numberAddress range checks, Chrome's bounds-checked getWasmLinearMemory).
Findings below are minor/perf — nothing blocking.
Contributor
Focused review: GC/lifetime, paused-worker consistency, default-build overheadI verified each of your three highlighted concerns directly against the code (locked range ✅ Default-build overhead — zero
✅ GC / lifetime safety
✅ Paused-worker consistencyThe seqlock protocol is correct and defensively enforced in the standalone decoder (
Net effect: if a worker is not actually paused, the decoder fails loudly with a "pause all workers" error rather than returning torn/corrupt data — which matches your stated contract that the frontend requires pausing other workers and makes no automatic-worker-control claim. Notes (non-blocking, carried over from the diff review)
No blocking issues on any of the three areas you flagged. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Collaborator
Author
|
Current doc_verify failure is the unchanged README LLDB homepage (https://lldb.llvm.org/): lychee reports a connection failure, with 95 other links passing and no excluded links. Direct access currently returns HTTP 200 and the official documentation remains valid. I have not retried CI, added link exclusions/tolerance, or changed a valid URL to bypass the failure. The full local Chrome debugger matrix and all 23 extension tests pass; the fresh hosted runtime/debugger checks are still running. |
Collaborator
Author
|
@fennoai please check the small P0 correction in eed7d78. Runtime P IDs are zero-based; debugger records now initialize last-P to -1 and the decoder reads the signed field so P0 remains valid. A new Memory32/64 unit test covers zero and the unstarted sentinel, and the real Chrome matrix now asserts the main G is P0 and both parked Gs have valid last-P values. All 24 extension tests and the complete actual 1/2/4-worker, embedded/external matrix pass. The earlier external-link failure was left intact; this head changes runtime decoding and tests, with no URL or CI-rule changes. |
Contributor
P0 correction verified ✅ — zero-based processor IDsChecked The P0 fix (
|
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 |
cpunion
force-pushed
the
codex/wasm-debug-runtime-20261006
branch
from
October 6, 2026 07:58
eed7d78 to
ede6941
Compare
cpunion
force-pushed
the
codex/wasm-debug-runtime-20261006
branch
from
October 6, 2026 09:38
ede6941 to
49441f3
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
1 participant
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.
Browser debug builds retain logical Go caller frames and a versioned runtime registry, so DevTools
$goroutinescan show live IDs, parent IDs, state, last processor and source stacks even when a fiber is parked.llgo debugenables the opt-in tag; ordinary builds retain their existing instrumentation policy. Decoding lives in a separately tested extension module, with batched memory reads, sequence guards and lazy snapshot objects that are released with their children.Validation: full compiler/browser/debugger/schema Go tests; 24 extension tests; real Chrome for Testing 152 with Memory32/64, 1/2/4 workers, embedded/external DWARF, parked recursive stacks after GC, valid P0 display, and record removal after exit. Rebased onto main
be669acd8; before this rebase the Wasm/browser/runtime and patch coverage checks passed with host checks pending. Current rebase CI is running; all 24 extension tests pass on this head. A local integration with #2738/#2743 passes the same ten Chrome sessions. The same browser acceptance runs in PR CI.Users still pause all worker threads in DevTools before inspecting shared state. Logical frames expose function/file/line, not parked-frame locals or automatic worker control. Advances the browser runtime views in #2164; WASI guest-debug transport and physical-board qualification remain separate.