fix(gc): report actual restore coverage inputs and slot work

changelog.d/10237-restore-coverage-metrics.md

@@ -0,0 +1,5 @@
+Correct the `[gc-restore-coverage]` diagnostic to report the actual old-page input (`dirty_old_pages`) separately from raw external entries and the covered-object skip set. The former `dirty_pages` field included external pages that the old-arena walk did not traverse.
+
+Add admitted parent visits, enumerated slots, and strong slots whose children still require tracking. Slot productivity counts edges even when their page was already dirty; it is separate from `pages_added`. These counters compile out of the diagnostics-off walk, and collection/remembered-set behavior is unchanged.
+
+A subprocess regression exercises diagnostics on and off, unequal old/external page inputs, duplicate stale external owners, skipped parents, mixed primitive/old/young slots, and repeated repair of an already-dirty page.

crates/perry-runtime/src/gc/tests/mod.rs

@@ -50,6 +50,7 @@ mod oldgen;
 mod os_tag;
 mod promote_in_place;
 mod proxy_registry;
+mod restore_coverage;
 mod retention_9628_9629;
 mod root_words;
 mod rooted_container_values;

crates/perry-runtime/src/gc/tests/restore_coverage.rs

@@ -0,0 +1,115 @@
+//! #9877: the diagnostic must describe the actual repair walk and its slots.
+use super::super::*;
+use super::support::*;
+
+fn exercise_restore() {
+    let _isolation = copying_nursery_isolation_lock();
+    let _trigger = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
+    reset_remembered_set();
+
+    unsafe {
+        // Two walked parents have four slots, but only two young edges.
+        // The third parent's primitive slot is already covered by the scan.
+        let (one, one_slot) = alloc_old_test_object(1);
+        let (three, three_slots) = alloc_old_test_object(3);
+        let (skipped, skipped_slot) = alloc_old_test_object(1);
+        let (young, _) = alloc_nursery_test_object(0);
+        assert!(!crate::arena::pointer_in_old_gen(young as usize));
+        let young_bits = POINTER_TAG | (young as u64 & POINTER_MASK);
+        *one_slot = young_bits;
+        *three_slots = young_bits;
+        *three_slots.add(1) = POINTER_TAG | (one as u64 & POINTER_MASK);
+        *three_slots.add(2) = 42.0f64.to_bits();
+        *skipped_slot = 7.0f64.to_bits();
+
+        let page = crate::arena::generation_page_for_addr(one as usize);
+        assert_eq!(page, crate::arena::generation_page_for_addr(three as usize));
+        assert_eq!(
+            page,
+            crate::arena::generation_page_for_addr(skipped as usize)
+        );
+        let snapshot = RememberedDirtySnapshot {
+            dirty_old_pages: [page].into_iter().collect(),
+            // A repeated stale external owner is deduplicated, then rejected
+            // without dereferencing it. These pages are not old walk inputs.
+            external_dirty_entries: vec![(page + 1, 0), (page + 2, 0)],
+            dirty_pages: [page, page + 1, page + 2].into_iter().collect(),
+            fallback_headers: Vec::new(),
+        };
+        let covered = [header_from_user_ptr(skipped as *const u8) as usize]
+            .into_iter()
+            .collect();
+        remembered_set_clear();
+        restore_surviving_dirty_coverage(&snapshot, &covered, "first");
+        assert!(
+            barrier::DIRTY_OLD_PAGES.with(|s| s.borrow().contains(&page)),
+            "repair must restore the page containing the young edges"
+        );
+        // Productivity counts edges, even when their page is already dirty.
+        restore_surviving_dirty_coverage(&snapshot, &covered, "repeat");
+        assert_eq!(barrier::DIRTY_OLD_PAGES.with(|s| s.borrow().len()), 1);
+    }
+    remembered_set_clear();
+    println!("restore coverage witness completed");
+}
+
+fn field(line: &str, name: &str) -> usize {
+    let prefix = format!("{name}=");
+    line.split_whitespace()
+        .find_map(|word| word.strip_prefix(&prefix))
+        .and_then(|value| value.parse().ok())
+        .unwrap_or_else(|| panic!("missing numeric {name} in {line}"))
+}
+
+#[test]
+fn restore_coverage_diagnostic_matches_walk_and_off_arm() {
+    const CHILD: &str = "PERRY_TEST_RESTORE_COVERAGE_CHILD";
+    let thread = std::thread::current();
+    let name = thread.name().expect("libtest thread name");
+    if std::env::var(CHILD).ok().as_deref() == Some(name) {
+        exercise_restore();
+        return;
+    }
+    for enabled in ["0", "1"] {
+        let output = std::process::Command::new(std::env::current_exe().unwrap())
+            .args(["--exact", name, "--nocapture", "--test-threads=1"])
+            .env(CHILD, name)
+            .env("PERRY_GC_DIAG", enabled)
+            .output()
+            .expect("run isolated diagnostic witness");
+        let stdout = String::from_utf8_lossy(&output.stdout);
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        assert!(
+            output.status.success() && stdout.contains("restore coverage witness completed"),
+            "witness failed (diag={enabled}): {}\n{stdout}\n{stderr}",
+            output.status
+        );
+        let lines: Vec<_> = stderr
+            .lines()
+            .filter(|line| line.starts_with("[gc-restore-coverage] "))
+            .collect();
+        if enabled == "0" {
+            assert!(
+                lines.is_empty(),
+                "diagnostics off must stay silent: {stderr}"
+            );
+            continue;
+        }
+        assert_eq!(lines.len(), 2, "one diagnostic per repair: {stderr}");
+        for (line, added) in lines.iter().zip([1, 0]) {
+            assert_eq!(field(line, "dirty_old_pages"), 1);
+            assert_eq!(field(line, "external_entries"), 2);
+            assert_eq!(field(line, "covered"), 1);
+            assert_eq!(field(line, "objects_walked"), 3);
+            assert_eq!(field(line, "objects_skipped"), 1);
+            assert_eq!(field(line, "parents_visited"), 2);
+            assert_eq!(field(line, "slots_visited"), 4);
+            assert_eq!(field(line, "slots_tracking"), 2);
+            assert_eq!(field(line, "pages_added"), added);
+            assert!(
+                !line.contains(" dirty_pages="),
+                "do not mislabel the walk input"
+            );
+        }
+    }
+}

crates/perry-runtime/src/gc/verify.rs

@@ -232,23 +232,27 @@ pub(super) unsafe fn remember_evacuated_old_to_young_slot(
     sticky: &mut StickyRememberedSet,
     parent_header: *mut GcHeader,
     slot: *mut u64,
-) {
+) -> bool {
     if slot.is_null() {
-        return;
+        return false;
     }
     let child_addr = decode_heap_addr(*slot);
     // Nursery AND malloc-GC children both need their pages kept dirty:
     // minors sweep the malloc registry too, and old parents are black
     // leaves — dropping an old→malloc page here would free the malloc
     // child on the next minor (see remembered_child_needs_tracking).
     if child_addr == 0 || !crate::gc::barrier::remembered_child_needs_tracking(child_addr) {
-        return;
+        return false;
     }
     sticky.remember_slot(
         parent_header,
         slot,
         slot_is_external_to(parent_header, slot),
     );
+    // Report the child's tracking requirement, not whether its page was new.
+    // The repair diagnostic can count productive slots without decoding and
+    // classifying the same child a second time.
+    true
 }
 
 /// Is `slot` outside `parent_header`'s own allocation, or on a page the
@@ -336,10 +340,27 @@ pub(super) fn restore_surviving_dirty_coverage(
     snapshot: &RememberedDirtySnapshot,
     covered: &crate::fast_hash::PtrHashSet<usize>,
     cycle_label: &str,
+) {
+    // Keep slot accounting out of the normal GC walk. Both instantiations
+    // perform exactly the same repair; only the diagnostic one counts it.
+    if crate::gc::gc_diag_enabled() {
+        restore_surviving_dirty_coverage_impl::<true>(snapshot, covered, cycle_label);
+    } else {
+        restore_surviving_dirty_coverage_impl::<false>(snapshot, covered, cycle_label);
+    }
+}
+
+fn restore_surviving_dirty_coverage_impl<const DIAGNOSTICS: bool>(
+    snapshot: &RememberedDirtySnapshot,
+    covered: &crate::fast_hash::PtrHashSet<usize>,
+    cycle_label: &str,
 ) {
     let mut sticky = StickyRememberedSet::default();
     let mut walked = 0usize;
     let mut skipped = 0usize;
+    let mut parents_visited = 0usize;
+    let mut slots_visited = 0usize;
+    let mut slots_tracking = 0usize;
     #[cfg(debug_assertions)]
     let mut skipped_sticky = StickyRememberedSet::default();
     // Mirror scan_remembered_dirty_slots_copying's scan_header guards: the
@@ -367,12 +388,23 @@ pub(super) fn restore_surviving_dirty_coverage(
         {
             return;
         }
+        if DIAGNOSTICS {
+            parents_visited += 1;
+        }
         visit_gc_rewrite_slots(header, |slot| {
+            if DIAGNOSTICS {
+                // Count all enumerated slots, including unproductive weak
+                // targets and primitive values. This measures traversal work.
+                slots_visited += 1;
+            }
             if crate::weakref::is_weak_target_trace_slot(header, slot.slot) {
                 return;
             }
             slot.record_layout_read();
-            remember_evacuated_old_to_young_slot(&mut sticky, header, slot.slot);
+            let tracking = remember_evacuated_old_to_young_slot(&mut sticky, header, slot.slot);
+            if DIAGNOSTICS && tracking {
+                slots_tracking += 1;
+            }
         });
     };
     if !snapshot.dirty_old_pages.is_empty() {
@@ -429,10 +461,19 @@ pub(super) fn restore_surviving_dirty_coverage(
              object `scan_dirty_object_slots` reported complete"
         );
     }
-    if crate::gc::gc_diag_enabled() {
+    if DIAGNOSTICS {
+        // These are the two actual snapshot inputs and the skip-set size.
+        // `dirty_pages` also includes external pages and is NOT the set the
+        // old-arena walk iterates. Candidate counts precede validity guards;
+        // parent visits and slot counts describe the admitted traversal.
         eprintln!(
-            "[gc-restore-coverage] {cycle_label} dirty_pages={} objects_walked={walked} objects_skipped={skipped} pages_added={added}",
-            snapshot.dirty_pages.len()
+            "[gc-restore-coverage] {cycle_label} dirty_old_pages={} external_entries={} \
+             covered={} objects_walked={walked} objects_skipped={skipped} \
+             parents_visited={parents_visited} slots_visited={slots_visited} \
+             slots_tracking={slots_tracking} pages_added={added}",
+            snapshot.dirty_old_pages.len(),
+            snapshot.external_dirty_entries.len(),
+            covered.len(),
         );
     }
 }