You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
gc: policy gate docs state "default OFF" while the code defaults ON (gc_incremental_enabled, gc_safepoint_moving_minor) — this caused #6972 to dismiss a real concern #6987
Two GC policy gates document themselves as default OFF while their implementations default ON.
This is not a cosmetic docs nit: it caused a real correctness concern to be dismissed on a merged PR,
and it was independently tripped over by two separate investigations within hours of each other.
1. gc_incremental_enabled — gc/policy.rs:291
The doc header says:
/// Phase 4 of the moving-GC project: gate the INCREMENTAL old-gen collector (the/// budgeted stepper). **EXPERIMENTAL — default OFF.** ...pub(crate)fn gc_incremental_enabled() -> bool{
...// DEFAULT ON (#6180 flip): ordinary allocation pressure is collected
The header and the body comment eight lines later state opposite defaults. The body is correct — the #6180 flip made it ON, and the header was never updated.
2. gc_safepoint_moving_minor — gc/policy.rs:1396
/// gated by `gc_moving_safepoint_enabled` (default off).
gc_moving_safepoint_enabled (policy.rs:279) is !matches!(env, "0"|"off"|"false") — default ON,
and its own doc correctly calls it "Perry's default GC". Only this reference at :1396 is stale.
(policy.rs:320, the PERRY_GC_MOVING_LOOP_POLLS line, appears accurate — that one really is opt-in.)
Why this is worth an issue rather than a drive-by edit
It changed a merge decision. PR #6972 dismissed the production-reachability of #6970 on the grounds
that gc_check_trigger "forces the conservative scan on both automatic arms". That reasoning depended
on incremental mode being off. With gc_incremental_enabled actually default ON, the guarded
direct-minor arm is largely bypassed and budgeted cycles skip the conservative subphase structurally,
independent of any guard (gc/cycle.rs:568-573, classifier mode).
Measured with PERRY_GC_TRACE=1 and no environment variables set at all: 3 completed cycles, 2 of them with conservative_root_count = 0. Production genuinely completes precise-roots-only
cycles today. The conclusion in #6972 was reached honestly from the documented behaviour and was wrong
because the documentation was wrong.
For the record, the follow-up investigation could not make #6970's reproducer fail in that
configuration (80 sequential iterations × 3 runs, byte-exact), so the issue stayed latent. What
holds it latent is BlockPersistence (gc/cycle.rs:1208-1247) retaining the recent-block window
precisely when the scan is skipped, plus the low cycle count at default heap size — a retention
heuristic and timing, not a rooting invariant. That is a cushion, not a guarantee, and it is not the
thing the docs claimed was protecting us.
Two independent agents also separately flagged the :1396 mismatch, which is a reasonable signal that
anyone reading this code is being misled by it.
Suggested fix
Correct both doc comments to state the actual defaults and name the kill switches.
Summary
Two GC policy gates document themselves as default OFF while their implementations default ON.
This is not a cosmetic docs nit: it caused a real correctness concern to be dismissed on a merged PR,
and it was independently tripped over by two separate investigations within hours of each other.
1.
gc_incremental_enabled—gc/policy.rs:291The doc header says:
The header and the body comment eight lines later state opposite defaults. The body is correct — the
#6180flip made it ON, and the header was never updated.2.
gc_safepoint_moving_minor—gc/policy.rs:1396/// gated by `gc_moving_safepoint_enabled` (default off).gc_moving_safepoint_enabled(policy.rs:279) is!matches!(env, "0"|"off"|"false")— default ON,and its own doc correctly calls it "Perry's default GC". Only this reference at :1396 is stale.
(
policy.rs:320, thePERRY_GC_MOVING_LOOP_POLLSline, appears accurate — that one really is opt-in.)Why this is worth an issue rather than a drive-by edit
It changed a merge decision. PR #6972 dismissed the production-reachability of #6970 on the grounds
that
gc_check_trigger"forces the conservative scan on both automatic arms". That reasoning dependedon incremental mode being off. With
gc_incremental_enabledactually default ON, the guardeddirect-minor arm is largely bypassed and budgeted cycles skip the conservative subphase structurally,
independent of any guard (
gc/cycle.rs:568-573, classifier mode).Measured with
PERRY_GC_TRACE=1and no environment variables set at all: 3 completed cycles,2 of them with
conservative_root_count = 0. Production genuinely completes precise-roots-onlycycles today. The conclusion in #6972 was reached honestly from the documented behaviour and was wrong
because the documentation was wrong.
For the record, the follow-up investigation could not make #6970's reproducer fail in that
configuration (80 sequential iterations × 3 runs, byte-exact), so the issue stayed latent. What
holds it latent is
BlockPersistence(gc/cycle.rs:1208-1247) retaining the recent-block windowprecisely when the scan is skipped, plus the low cycle count at default heap size — a retention
heuristic and timing, not a rooting invariant. That is a cushion, not a guarantee, and it is not the
thing the docs claimed was protecting us.
Two independent agents also separately flagged the
:1396mismatch, which is a reasonable signal thatanyone reading this code is being misled by it.
Suggested fix
these gates decide whether the conservative stack scan runs, which is load-bearing for every
precise-rooting argument in the gc: console.log argument temporaries are not precise roots — a precise-roots-only collection drops string-literal args (minimal repro, no evacuation needed) #6951 family (gc: scalar-replaced object/array locals holding heap values are not precise roots #6968, gc: constructor arguments (new C(a, b)) are not precise roots across the instance allocation #6969, gc: native-method-call arguments (map.set(a, b)) are not precise roots -- aborts under precise-roots-only #6970, gc: string-method receiver and arguments are not precise roots across an allocating argument #6971, gc: a relocating minor with PRECISE roots breaks 14 of 20 representation-corpus files (5 crashes, 9 mismatches) — the conservative stack scan is load-bearing for correctness #6981, gc: constructor arguments on lower_new's non-class branches (Readline / imported ctor / new Function) and builtin.rs are still not precise roots #6986).