diff --git a/changelog.d/7182-fromspace-scan-forwarded-owners.md b/changelog.d/7182-fromspace-scan-forwarded-owners.md new file mode 100644 index 0000000000..2bab5a0568 --- /dev/null +++ b/changelog.d/7182-fromspace-scan-forwarded-owners.md @@ -0,0 +1,7 @@ +**`PERRY_GC_FROMSPACE_SCAN` no longer reports dead relocation stubs as offenders.** The scan skips owners that are themselves in from-space — they are about to be reclaimed, so they legitimately still point at their dead peers — but it did not skip owners carrying `GC_FLAG_FORWARDED`. A forwarded object is dead for exactly the same reason, one step further on, and unlike a from-space object it can sit in old-gen where the space check never reaches it. Old-gen defrag and array growth both leave such stubs behind, so every one of them was reported. + +On a Perry-compiled zod workload these were the single largest population in the residue after #7179 — all at `2^k - 1` element indices of repeatedly-grown arrays (the last element written before each capacity doubling), i.e. pure noise sitting on top of whatever genuine holders remain. #7154's triage has been reading these counts. + +The skip is **counted** (`fwd_owners_skipped=` in the report line), not silent: a filter that shrinks the offender count without saying so reads exactly like progress, which is the failure mode this instrument exists to prevent (#6942, #7024). `fromspace_scan_skips_but_counts_forwarded_owners_7154` asserts both halves — that the identical planted reference is reported while the holder is live and stops being reported once the holder is forwarded, and that the counter moves — and is red on the pre-fix predicate. + +Instrument only; no collector behaviour changes. diff --git a/crates/perry-runtime/src/gc/fromspace_scan.rs b/crates/perry-runtime/src/gc/fromspace_scan.rs index 49b218d8de..2604141fc8 100644 --- a/crates/perry-runtime/src/gc/fromspace_scan.rs +++ b/crates/perry-runtime/src/gc/fromspace_scan.rs @@ -22,7 +22,11 @@ //! through `decode_root_word`, the same decoder the mark and rewrite paths //! share (#6910). Objects that are themselves in from-space are skipped: they //! are about to be reclaimed, and a dead object legitimately still points at -//! its dead peers. +//! its dead peers. So are owners carrying `GC_FLAG_FORWARDED` — a relocation +//! stub is dead for the same reason, one step further on, and unlike a +//! from-space object it can sit in old-gen where the space check does not reach +//! it. Both skips are counted (`fwd_owners_skipped=`) so a suppressed +//! population can never read as a fixed one. //! //! A word whose target carries `GC_FLAG_FORWARDED` is an unambiguous **missing //! rewrite**: the object moved this cycle and this reference was not updated. @@ -75,6 +79,9 @@ pub(crate) struct FromSpaceRef { pub(crate) struct FromSpaceScanReport { pub(crate) objects_scanned: usize, pub(crate) words_scanned: usize, + /// Owners skipped because they are themselves FORWARDED (dead relocation + /// stubs). Reported so the filter can never be mistaken for a fix. + pub(crate) forwarded_owners_skipped: usize, pub(crate) missing_rewrites: usize, pub(crate) dangling: usize, /// Offending slots whose page was NEVER dirtied -> a store path skipped the @@ -146,6 +153,23 @@ unsafe fn scan_object(header: *mut GcHeader, report: &mut FromSpaceScanReport) { if is_from_space(owner_space) { return; } + // ...and a FORWARDED owner is the same thing one step further on: it has + // been superseded by its relocated copy, so it is dead by definition, and + // its payload legitimately still names pre-move addresses. Old-gen defrag + // and array growth both leave such stubs OUTSIDE from-space, where the + // check above does not reach them, so every one of them was reported as an + // offender. Measured on a Perry-compiled zod workload they were the single + // largest population in the residue — all at `2^k - 1` element indices of + // repeatedly-grown arrays (the last element written before each capacity + // doubling), i.e. pure noise that buried the genuine holders underneath. + // + // COUNTED, not silently dropped: a filter that shrinks the offender count + // without saying so reads exactly like progress, which is the failure mode + // this instrument exists to prevent (#6942 / #7024). + if (*header).gc_flags & GC_FLAG_FORWARDED != 0 { + report.forwarded_owners_skipped += 1; + return; + } report.objects_scanned += 1; let payload_words = (total - GC_HEADER_SIZE) / 8; @@ -288,10 +312,11 @@ fn report_and_abort(report: &FromSpaceScanReport) -> ! { pub(super) fn emit_report(report: &FromSpaceScanReport, phase: &str) { eprintln!( - "[gc-fromspace-scan {}] objects={} words={} missing_rewrites={} dangling={} owners={} | never_dirty={} lost_dirty={} dirty_but_missed={}", + "[gc-fromspace-scan {}] objects={} words={} fwd_owners_skipped={} missing_rewrites={} dangling={} owners={} | never_dirty={} lost_dirty={} dirty_but_missed={}", phase, report.objects_scanned, report.words_scanned, + report.forwarded_owners_skipped, report.missing_rewrites, report.dangling, report.distinct_owners.len(), diff --git a/crates/perry-runtime/src/gc/tests/fromspace_scan.rs b/crates/perry-runtime/src/gc/tests/fromspace_scan.rs index 4785999419..03ba36d0bc 100644 --- a/crates/perry-runtime/src/gc/tests/fromspace_scan.rs +++ b/crates/perry-runtime/src/gc/tests/fromspace_scan.rs @@ -143,3 +143,67 @@ fn fromspace_scan_ignores_references_held_by_from_space_objects() { (*young_header).gc_flags &= !GC_FLAG_FORWARDED; } } + +/// #7154: a FORWARDED owner is a dead relocation stub. Its payload legitimately +/// still names pre-move addresses, so reporting it is a false positive — and +/// unlike a from-space owner it can sit in old-gen, where the space check does +/// not reach it. On a Perry-compiled zod workload this was the single largest +/// population in the residue, which is what made it worth a predicate. +/// +/// The skip must also be COUNTED. A filter that shrinks the offender count +/// without saying so reads exactly like progress, which is the failure mode +/// this whole instrument exists to prevent. +#[test] +fn fromspace_scan_skips_but_counts_forwarded_owners_7154() { + let holder = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_OBJECT); + let young = crate::arena::arena_alloc_gc(64, 8, GC_TYPE_OBJECT); + unsafe { + std::ptr::write_bytes(holder, 0, 64); + } + + let baseline = scan_heap_for_fromspace_refs(); + + // Same planted offender as the positive test above... + unsafe { + plant_reference(holder, young); + let young_header = header_from_user_ptr(young) as *mut GcHeader; + (*young_header).gc_flags |= GC_FLAG_FORWARDED; + } + let reported = scan_heap_for_fromspace_refs(); + assert!( + reported.missing_rewrites > baseline.missing_rewrites, + "test premise: the planted reference must be reported while the holder \ + is live (baseline={}, reported={})", + baseline.missing_rewrites, + reported.missing_rewrites + ); + + // ...but once the HOLDER itself is forwarded it is dead, and the identical + // planted reference must stop being an offender. + unsafe { + let holder_header = header_from_user_ptr(holder) as *mut GcHeader; + (*holder_header).gc_flags |= GC_FLAG_FORWARDED; + } + let after = scan_heap_for_fromspace_refs(); + assert_eq!( + after.missing_rewrites, baseline.missing_rewrites, + "#7154: a FORWARDED owner is a dead relocation stub — its stale payload \ + must not be reported as a missing rewrite" + ); + assert!( + after.forwarded_owners_skipped > baseline.forwarded_owners_skipped, + "#7154: the skip must be COUNTED, not silent — a suppressed population \ + that does not show up in the report reads as a fixed one \ + (baseline={}, after={})", + baseline.forwarded_owners_skipped, + after.forwarded_owners_skipped + ); + + unsafe { + clear_reference(holder); + let holder_header = header_from_user_ptr(holder) as *mut GcHeader; + (*holder_header).gc_flags &= !GC_FLAG_FORWARDED; + let young_header = header_from_user_ptr(young) as *mut GcHeader; + (*young_header).gc_flags &= !GC_FLAG_FORWARDED; + } +}