Fix memory leak and re-enable immutable-send GC optimization#4944
Open
SeanTAllen wants to merge 4 commits intomainfrom
Open
Fix memory leak and re-enable immutable-send GC optimization#4944SeanTAllen wants to merge 4 commits intomainfrom
SeanTAllen wants to merge 4 commits intomainfrom
Conversation
SeanTAllen
added
the
changelog - fixed
ponylang-main
added
the
discuss during sync
SeanTAllen
changed the title
SeanTAllen
force-pushed
the
fix-selector-painting-4102
branch
2 times, most recently
from
4f5c8ec to
daab566
Compare
SeanTAllen
added
do not merge
SeanTAllen
changed the title
Member
Author
|
I tried to do these individually but there are so intertwined that I decided they needed to come in together. |
SeanTAllen
force-pushed
the
fix-selector-painting-4102
branch
3 times, most recently
from
36dbcd0 to
358471e
Compare
…than the trait it implements When a behavior is called through a trait and the concrete actor's parameter has a different trace-significant capability (e.g., trait has iso, concrete has val), the sender traces with one trace kind and the receiver traces with another. This ORCA GC mismatch causes field reference counts to never reach zero, leaking objects reachable from the parameter. The fix includes trace-kind characters in mangled names for behaviors and constructors, so methods whose parameters differ in trace-significant ways get distinct vtable indices. When a forwarding method is created due to a cap mismatch, genfun_forward now adds a dispatch case that traces with the forwarding method's params (the trait's capabilities) but calls the concrete method's handler. A two-pass ordering in genfun_method_bodies ensures concrete handlers are generated before forwarding dispatch cases that reference them. The leak is not currently active because make_might_reference_actor forces full tracing of immutable objects. Without this fix, re-enabling that optimization would expose the leak. Design: #4943 Closes #4102
…actor make_might_reference_actor currently always returns true, so immutable tracing recurses into fields. The test incorrectly assumed it would skip field recursion for non-actor fields.
…ference_actor" This reverts commit 2fb4b6d.
The might_reference_actor optimization was disabled in PR #4256 as a safety net — hardcoded to true for all types, forcing full tracing of every immutable object. This caused a ~1/3 performance drop in message-heavy programs. The compiler now computes might_reference_actor at compile time using a monotone fixpoint: actors are always true, classes/structs/tuples propagate from their fields (including through trait subtypes), and primitives are always false. The result is baked into the type descriptor so the runtime reads a constant — no runtime computation needed. The existing runtime code in gc.c is already correct — it checks might_reference_actor at all 11 call sites. Only the compiler needed to change: emit the computed value instead of hardcoded true. Adds a new full-program test (val-to-val-with-actor-through-trait) that verifies immutable tracing does recurse when B has an actor field. Counterfactual-verified: hardcoded true breaks the val-to-val test, hardcoded false causes regression-1118 to segfault. Release notes and CHANGELOG updated directly (multi-type PR: Fixed + Changed). Individual release notes file removed per convention.
SeanTAllen
force-pushed
the
fix-selector-painting-4102
branch
from
358471e to
bcedd41
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.
When a behavior is called through a trait and the concrete actor's parameter has a different trace-significant capability (e.g., trait has
iso, concrete hasval), the sender traces with one trace kind and the receiver traces with another. This ORCA GC mismatch causes field reference counts to never reach zero, leaking objects reachable from the parameter.The fix includes trace-kind characters in mangled names for behaviors and constructors, so methods whose parameters differ in trace-significant ways get distinct vtable indices. When a forwarding method is created due to a cap mismatch,
genfun_forwardadds a dispatch case that traces with the forwarding method's params (the trait's capabilities) but calls the concrete method's handler. A two-pass ordering ingenfun_method_bodiesensures concrete handlers are generated before forwarding dispatch cases that reference them.With the leak fixed, the
might_reference_actoroptimization can safely be re-enabled. The compiler now computes at compile time whether each type could transitively contain an actor reference using a monotone fixpoint, and emits the result in the type descriptor instead of the hardcodedtruethat was set as a safety net in PR #4256. The runtime code in gc.c is already correct — it just needed a truthful value.This is a multi-type PR (Fixed + Changed). CHANGELOG and release notes are updated directly; no changelog label should be applied.
Design: #4943
Closes #4102