runtime: drain BDWGC finalizers during explicit GC - #2283
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
The fix is well-targeted: explicitly draining BDWGC's ready queue via GC_invoke_finalizers makes runtime.GC() finalizer delivery deterministic instead of depending on a later allocation, and extracting collectAndRunFinalizers() cleanly removes the prior duplication. The new on-demand test is a good regression guard, and the //go:linkname signatures match the BDWGC C ABI (GC_invoke_finalizers → int(void), GC_get/set_finalize_on_demand). No performance regression: the two added GC_invoke_finalizers calls are effectively O(1) when the ready queue is empty; the dominant cost (two full GC_gcollect cycles) is unchanged.
Findings below are all minor / maintainability — nothing blocking. See inline comments for concrete locations.
Additional note without a reliable single diff line:
- Unlocked
finalizerState.head/tailaccess —runtime/internal/lib/runtime/mfinal.go:132-138(writersetFinalizerCallback) andmfinal.go:154-161(readerrunFinalizers) mutate/read the finalizer linked list without the mutex, by design (the comment atmfinal.go:128-129notes callbacks fire during collection). This PR doesn't weaken that assumption — callingInvokeFinalizers()immediately beforerunFinalizers()on the same goroutine actually makes timing more deterministic. The whole scheme still relies on BDWGC only ever running these callbacks single-threaded (world stopped). Not a defect introduced here; flagging so the invariant is intentional. A one-line comment stating "head/tail are only touched from the collecting/GC goroutine" would prevent a future footgun.
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
1c68f52 to
7e4bcc6
Compare
|
Regarding the review note about |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…e-20260804 runtime: drain BDWGC finalizers during explicit GC
2 participants
Summary
GC_invoke_finalizersGC_gcollectinruntime.GCfinalize_on_demand, removing later-allocation timing from the assertionRoot cause
GC_gcollectdiscovers unreachable finalizable objects, while BDWGC documents thatGC_invoke_finalizersis normally called implicitly only during some later allocations. LLGo calledrunFinalizersimmediately after collection without first moving BDWGC ready callbacks into the Go finalizer queue. The test therefore depended on unrelated allocation and scheduling timing, producing both 12/32 and 0/32 CI failures before an unchanged rerun passed.The regression enables BDWGC on-demand finalization temporarily. Before this fix, eight explicit
runtime.GCcalls deterministically execute 0/32 finalizers. With the fix, the test passes without extending a wall-clock deadline.This is separate from conservative dead stack/register roots tracked by #2036; it does not broaden that compiler/runtime liveness scope.
Fixes #2275
Validation
test/gopassestest/std/weakGC test passes for 100 repetitionsinternal/clite/bdwgcandinternal/lib/runtimepackages build with host Gotest/gofinalizer selection passes; LLGo-only regression is correctly excludedgit diff --checkpasses