Skip to content

Commit 36dbcd0

Browse files
committed
Re-enable might_reference_actor optimization where safe
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.
1 parent a48b84e commit 36dbcd0

File tree

9 files changed

+150
-20
lines changed

9 files changed

+150
-20
lines changed

.release-notes/fix-iso-to-val-trait-memory-leak.md

Lines changed: 0 additions & 17 deletions
This file was deleted.

.release-notes/next-release.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,25 @@ Lines inside triple-quoted strings (docstrings) and lines containing `"""` delim
100100

101101
Previously, writing `return error` in a function body would crash the compiler with an assertion failure instead of producing a diagnostic error. The compiler now correctly reports that a return value cannot be a control statement.
102102

103+
## Fix memory leak when a behavior parameter has a different capability than the trait it implements
104+
105+
When a behavior was called through a trait reference and the concrete actor's parameter had a different trace-significant capability (e.g., the trait declared `iso` but the actor declared `val`), the ORCA garbage collector's reference counting was broken. The sender traced the parameter with one trace kind and the receiver traced with another, causing field reference counts to never reach zero. Objects reachable from the parameter were leaked.
106+
107+
```pony
108+
trait tag Receiver
109+
be receive(b: SomeClass iso)
110+
111+
actor MyActor is Receiver
112+
be receive(b: SomeClass val) =>
113+
// b's fields were leaked when called through a Receiver reference
114+
None
115+
```
116+
117+
The sender and receiver now use consistent tracing for each parameter, regardless of capability differences between the trait and concrete method.
118+
119+
## Re-enable immutable-send GC optimization
120+
121+
The runtime's ORCA garbage collector has an optimization that skips tracing the field graph of immutable objects sent between actors — if an object is already immutable, its fields can't change, so the GC doesn't need to recurse into them on every send. This optimization was disabled in 0.39.0 (PR #4256) because it was unsafe for immutable objects that transitively contain actor references: skipping the trace meant the actor's reference count wasn't updated, leading to premature collection and segfaults.
122+
123+
The compiler now computes at compile time whether each type could transitively contain an actor reference. Types that can't — classes, structs, and tuples whose fields only contain other non-actor types — get the optimization. Types that can contain actors are traced fully, preserving correctness. This restores the performance benefit for the common case (most immutable objects don't reference actors) while keeping the safety fix for the uncommon case.
124+

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ All notable changes to the Pony compiler and standard library will be documented
66

77
### Fixed
88

9+
- Fix memory leak when a behavior parameter has a different capability than the trait it implements ([PR #4944](https://github.com/ponylang/ponyc/pull/4944))
910
- Fix pool_memalign crash due to insufficient alignment for AVX instructions ([PR #4909](https://github.com/ponylang/ponyc/pull/4909))
1011
- Fix `assignment-indent` to require RHS indented relative to assignment ([PR #4913](https://github.com/ponylang/ponyc/pull/4913))
1112
- Fix tool install with `use` flags ([PR #4915](https://github.com/ponylang/ponyc/pull/4915))
@@ -20,6 +21,7 @@ All notable changes to the Pony compiler and standard library will be documented
2021

2122
### Changed
2223

24+
- Re-enable immutable-send GC optimization ([PR #4944](https://github.com/ponylang/ponyc/pull/4944))
2325
- Update to LLVM 21.1.8 ([PR #4876](https://github.com/ponylang/ponyc/pull/4876))
2426
- Exempt unsplittable string literals from line length rule ([PR #4923](https://github.com/ponylang/ponyc/pull/4923))
2527

src/libponyc/codegen/gendesc.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,11 @@ static LLVMValueRef make_field_offset(compile_t* c, reach_type_t* t)
254254
LLVMOffsetOfElement(c->target_data, c_t->structure, index), false);
255255
}
256256

257+
// Pre-computed by reach_compute_might_reference_actor(), which must be
258+
// called before gendesc_init().
257259
static LLVMValueRef make_might_reference_actor(compile_t* c, reach_type_t* t)
258260
{
259-
(void)t;
260-
261-
return LLVMConstInt(c->i1, 1, false);
261+
return LLVMConstInt(c->i1, t->might_reference_actor ? 1 : 0, false);
262262
}
263263

264264
static LLVMValueRef make_field_list(compile_t* c, reach_type_t* t)

src/libponyc/codegen/gentype.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "gentype.h"
22
#include "genbox.h"
33
#include "gendesc.h"
4+
#include "../reach/reach.h"
45
#include "genexpr.h"
56
#include "genfun.h"
67
#include "genident.h"
@@ -843,6 +844,9 @@ bool gentypes(compile_t* c)
843844
if(c->opt->verbosity >= VERBOSITY_INFO)
844845
fprintf(stderr, " Descriptors\n");
845846

847+
// Compute might_reference_actor before gendesc_init reads it.
848+
reach_compute_might_reference_actor(c->reach);
849+
846850
i = HASHMAP_BEGIN;
847851

848852
while((t = reach_types_next(&c->reach->types, &i)) != NULL)

src/libponyc/reach/reach.c

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,74 @@ uint32_t reach_max_type_id(reach_t* r)
15951595
return len;
15961596
}
15971597

1598+
// Check whether a single field's reach_type_t might reference an actor.
1599+
// For trait-like types, checks all concrete subtypes.
1600+
static bool field_might_reference_actor(reach_type_t* field_type)
1601+
{
1602+
if(!field_type->is_trait)
1603+
return field_type->might_reference_actor;
1604+
1605+
size_t j = HASHMAP_BEGIN;
1606+
reach_type_t* sub;
1607+
1608+
while((sub = reach_type_cache_next(&field_type->subtypes, &j)) != NULL)
1609+
{
1610+
if(!sub->is_trait && sub->might_reference_actor)
1611+
return true;
1612+
}
1613+
1614+
return false;
1615+
}
1616+
1617+
void reach_compute_might_reference_actor(reach_t* r)
1618+
{
1619+
size_t i;
1620+
reach_type_t* t;
1621+
1622+
// First pass: actors always might reference an actor (they ARE actors).
1623+
i = HASHMAP_BEGIN;
1624+
while((t = reach_types_next(&r->types, &i)) != NULL)
1625+
{
1626+
if(t->underlying == TK_ACTOR)
1627+
t->might_reference_actor = true;
1628+
}
1629+
1630+
// Fixpoint: propagate through fields until stable.
1631+
// Values only move false->true (monotone), so this converges.
1632+
bool changed = true;
1633+
while(changed)
1634+
{
1635+
changed = false;
1636+
i = HASHMAP_BEGIN;
1637+
1638+
while((t = reach_types_next(&r->types, &i)) != NULL)
1639+
{
1640+
if(t->might_reference_actor)
1641+
continue;
1642+
1643+
switch(t->underlying)
1644+
{
1645+
case TK_CLASS:
1646+
case TK_STRUCT:
1647+
case TK_TUPLETYPE:
1648+
break;
1649+
default:
1650+
continue;
1651+
}
1652+
1653+
for(uint32_t f = 0; f < t->field_count; f++)
1654+
{
1655+
if(field_might_reference_actor(t->fields[f].type))
1656+
{
1657+
t->might_reference_actor = true;
1658+
changed = true;
1659+
break;
1660+
}
1661+
}
1662+
}
1663+
}
1664+
}
1665+
15981666
void reach_dump(reach_t* r)
15991667
{
16001668
printf("REACH\n");
@@ -1610,6 +1678,8 @@ void reach_dump(reach_t* r)
16101678

16111679
printf(" serialise_id: %" PRIu64 "\n", t->serialise_id);
16121680
printf(" is_trait: %s\n", (t->is_trait)?"true":"false");
1681+
printf(" might_reference_actor: %s\n",
1682+
(t->might_reference_actor)?"true":"false");
16131683
printf(" can_be_boxed: %s\n", (t->can_be_boxed)?"true":"false");
16141684

16151685
printf(" vtable: %d\n", t->vtable_size);
@@ -2022,6 +2092,7 @@ static void reach_type_serialise(pony_ctx_t* ctx, void* object, void* buf,
20222092
dst->vtable_size = t->vtable_size;
20232093
dst->can_be_boxed = t->can_be_boxed;
20242094
dst->is_trait = t->is_trait;
2095+
dst->might_reference_actor = t->might_reference_actor;
20252096
dst->serialise_id = t->serialise_id;
20262097

20272098
dst->field_count = t->field_count;

src/libponyc/reach/reach.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ struct reach_type_t
102102
uint64_t serialise_id;
103103
bool can_be_boxed;
104104
bool is_trait;
105+
bool might_reference_actor;
105106

106107
uint32_t field_count;
107108
reach_field_t* fields;
@@ -148,6 +149,8 @@ uint32_t reach_vtable_index(reach_type_t* t, const char* name);
148149

149150
uint32_t reach_max_type_id(reach_t* r);
150151

152+
void reach_compute_might_reference_actor(reach_t* r);
153+
151154
void reach_dump(reach_t* r);
152155

153156
pony_type_t* reach_method_pony_type();
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
use "lib:codegen-trace-additional"
2+
3+
use @gc_local[Pointer[None]](target: Main)
4+
use @gc_local_snapshot[Pointer[None]](target: Main)
5+
use @gc_local_snapshot_destroy[None](obj_map: Pointer[None])
6+
use @objectmap_has_object[Bool](obj_map: Pointer[None], obj: Any tag)
7+
use @objectmap_has_object_rc[Bool](obj_map: Pointer[None], obj: Any tag, rc: USize)
8+
use @pony_exitcode[None](code: I32)
9+
10+
actor Dummy
11+
12+
class A
13+
14+
class B
15+
let a: A = A
16+
let d: Dummy
17+
18+
new create(d': Dummy) => d = d'
19+
20+
trait tag Receiver
21+
be receive(b: B val)
22+
23+
actor Main is Receiver
24+
var map_before: Pointer[None] = Pointer[None]
25+
26+
new create(env: Env) =>
27+
// Call through a trait-typed variable, same as the val-to-val test.
28+
// B has an actor field (Dummy), so might_reference_actor is true.
29+
// Immutable tracing DOES recurse into fields despite val capability.
30+
let r: Receiver = this
31+
r.receive(recover B(Dummy) end)
32+
map_before = @gc_local_snapshot(this)
33+
34+
be receive(b: B val) =>
35+
let map_after = @gc_local(this)
36+
// Both sender and receiver trace b as IMMUTABLE (val). Since B has an
37+
// actor field (might_reference_actor is true), immutable tracing recurses
38+
// into fields. So b.a DOES appear in the object map with rc bookkeeping.
39+
let ok = @objectmap_has_object_rc(map_before, b, USize(1)) and
40+
@objectmap_has_object(map_before, b.a) and
41+
@objectmap_has_object_rc(map_after, b, USize(0)) and
42+
@objectmap_has_object(map_after, b.a)
43+
@gc_local_snapshot_destroy(map_before)
44+
@pony_exitcode(I32(if ok then 1 else 0 end))

0 commit comments

Comments
 (0)