Skip to content

runtime: run syscall/js finalizers on wasm without a manual GC#5545

Open
felipegenef wants to merge 3 commits into
tinygo-org:devfrom
felipegenef:fix-syscall-js-finalizeref-pressure-gc
Open

runtime: run syscall/js finalizers on wasm without a manual GC#5545
felipegenef wants to merge 3 commits into
tinygo-org:devfrom
felipegenef:fix-syscall-js-finalizeref-pressure-gc

Conversation

@felipegenef

Copy link
Copy Markdown
Contributor

Runtime: run syscall/js finalizers on wasm without a manual GC

Follow-up to #5521, which implemented runtime.SetFinalizer so syscall/js can
auto-release bridge-table slots. While stress-testing a syscall/js-heavy wasm
project of my own on top of that change, I found the finalizers almost never run
on their own, so the bridge tables keep growing under load. Reclaiming a slot
still needs an explicit runtime.GC().

Why the slots still leak

The finalizer frees the JS slot, so reclamation depends on the GC running, and
the block GC only runs when the Go heap is exhausted. A GOOS=js program creates
lots of small, short-lived js.Values. Each one pins a JS object and a bridge
slot but costs only a few bytes of Go heap, so the heap barely grows and the GC
never fires. The finalizers are correct, they just never get a turn.

Standard Go covers this case with a 2-minute forced GC, but that runs from
sysmon, and haveSysmon = GOARCH != "wasm". There is no sysmon on wasm, and
the 4 MB heapMinimum keeps the heap-growth trigger from firing for a small live
set. So on wasm there is no trigger at all for an idle, allocation-light workload.

The fix

Two small changes.

1. Collect on finalizer-registration pressure, from the scheduler's idle
point.
Once finalizerGCThreshold (32) finalizers have been registered since
the last GC, the cooperative scheduler collects when its run queue drains (top
level only, no goroutine mid-run). A registered finalizer is a good proxy for the
external pressure the heap size can't see.

Running this from the idle point rather than from alloc is the important part.
An alloc-based trigger scales GC frequency with allocation churn: a goroutine
that boxes hundreds of short-lived js.Values would force dozens of collections
mid-run, almost all of them wasted on values that are still live. The idle point
collects a finished run's now-dead values in one pass instead.

2. Zero a finished goroutine's stack (asyncify scheduler). Asyncify goroutine
stacks are heap buffers scanned conservatively, so a returned event handler's
stale frame pointers keep its js.Values reachable until the buffer itself is
collected. The scheduler now zeroes a finished goroutine's stack.

The idle hook is installed lazily by the first SetFinalizer, next to the
existing runner spawn, so a program that never registers a finalizer links none
of it. A microbit binary with no SetFinalizer is byte-identical before and
after (code 2788), so TestBinarySize is unaffected. Non-block GCs and the
cores/threads schedulers get a nil hook; the tasks scheduler gets a no-op
stack-zero.

Scope

Cooperative schedulers (asyncify, tasks) with the block GC only. The
finalizer semantics, wasm_exec.js, and the public API are untouched. The
threshold is a policy constant, like Go's forcegcperiod.

Testing

New testdata/finalizeridle.go golden test, wasm only (same determinism
rationale and skip as finalizer.go). It registers a batch of finalizers over
the threshold, then only parks the goroutine with time.Sleep and never calls
runtime.GC(), and checks that every finalizer ran. A second case registers
finalizers on goroutine-stack-local objects, lets the goroutines finish, and
checks the objects are still collected.

finalizer.go and gc.go still pass on host and wasm. The change was built
across every scheduler and GC it touches (asyncify, tasks, none, cores,
threads, with block, boehm, and leaking) on wasm, wasip1, wasip2,
microbit, pico, and host. make fmt-check is clean.

Beyond the golden test, this fixes real breakage. On top of #5521's finalizers
but with nothing to trigger a collection, a syscall/js-heavy wasm project of
mine grew the bridge table without bound under load and failed its leak checks;
with this change the same runs hold net growth near zero and pass. Bridge-table
(_values) growth measured over a run, no manual runtime.GC():

DOM events under load growth before growth after
100 ~90 slots ~0
500 ~290 slots ~0
20s sustained load ~450 slots ~0

The other half of the win is cadence. An operation that boxes a few hundred
short-lived js.Values triggers about one collection at the idle point; the same
operation with the trigger inside alloc forced roughly ten, almost all of them
wasted on values that were still live.

@deadprogram

Copy link
Copy Markdown
Member

Here are some lightly edited generated comments:

PR #5545 Review Comments

1. finishing global vs per-task field

The var finishing bool in task_asyncify.go is a plain package-level global, justified by the cooperative scheduler guarantee that no suspension point exists between deadlock()Pause()Resume(). This is correct today, but fragile: if a preemption point is ever introduced in that path, two goroutines could clobber each other's flag silently.

Consider moving this to a field on *Task (or a bit in an existing field). That would make the intent explicit ("this specific task is finishing") and eliminate the implicit coupling to cooperative scheduling semantics. The cost is one extra byte or bit per task, which is negligible compared to the stack buffer it already owns.

2. Build tag coverage across scheduler variants

The PR splits gc_finalizer_sched.go into two files:

  • gc_finalizer_sched.go: (gc.conservative || gc.precise) && (scheduler.tasks || scheduler.asyncify) which installs the idle hook + spawns runner
  • gc_finalizer_sched_other.go: (gc.conservative || gc.precise) && !scheduler.none && !scheduler.tasks && !scheduler.asyncify which spawns runner only

The existing scheduler.none path has no spawnFinalizerRunner (it drains inline via wakeFinalizer).

Please confirm the union of these three paths is exhaustive for every scheduler variant (none, tasks, asyncify, cores, threads). In particular, verify that no future scheduler variant could fall through all three build constraints and leave spawnFinalizerRunner undefined.

3. Threshold of 32 regarding tunability and documentation

finalizerGCThreshold = 32 is described as "a policy constant, like Go's forcegcperiod." A few points worth discussing:

  • Is 32 the right default? For a bursty syscall/js workload creating hundreds of js.Values per event, 32 triggers roughly one collection per event handler dispatch (at the idle point), which seems well-calibrated. But a workload that legitimately registers many long-lived finalizers in a setup phase would pay for several unnecessary collections during init.

  • Should it be runtime-tunable? Go's GOGC is an environment variable; forcegcperiod is a named constant but not user-facing. If this stays a compile-time constant, documenting it in a comment (already done) is probably sufficient. If there's any chance users will need to tune it, exposing it via an environment variable or runtime/debug style API would be worth considering now rather than later.

  • Zero disables the trigger this is documented in the code comment, which is good. Worth calling out in any user-facing docs if they exist.

@felipegenef

Copy link
Copy Markdown
Contributor Author

Thanks @deadprogram, all fair points. Done:

1. finishing flag. Now a per-goroutine field on the asyncify state struct instead of a package global, so each finishing goroutine owns its own and the handoff to Resume no longer depends on scheduler timing. One byte per task, asyncify only.

2. Build tag coverage. There's a fourth file, gc_finalizer_sched_none.go (no-op for scheduler.none). The three constraints partition the scheduler space, so every variant, present or future, matches exactly one file (cores/threads land in the !none && !tasks && !asyncify catch-all). I documented the partition in a comment and added a test that iterates validSchedulerOptions, so a newly added scheduler is checked automatically.

3. Threshold of 32. Kept it a const. It only fires at an idle point and resets the counter, so a big setup burst costs one collection, not one per 32:

finalizer GC threshold

Happy to make it tunable via a runtime/debug setter or an env var if you'd rather; I left the API shape to you since it can be added later without breaking anything.

@deadprogram

Copy link
Copy Markdown
Member

Any further comments from @jakebailey @dgryski or anyone else also wasm-involved?

// will never be resumed, so its stack can be cleared now to drop any
// pointers its returned frames left behind (see clearStack).
t.state.finishing = false
t.clearStack()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might need t.state.args = nil?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed. Added a testFinishedGoroutineArgs case in finalizeridle.go to cover it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants