fix(gc): make old-page defrag opt-in again until a workload can exercise it

changelog.d/7922-old-defrag-default-off.md

@@ -0,0 +1,13 @@
+### Fixed
+
+- **Old-page defragmentation is opt-in again** (`PERRY_GC_OLD_DEFRAG=1`). #7913's rewrite-contract work — the `PARSE_KEY_RING`/diagnostics/perf-hooks rewrite coverage, class-key locals as precise mutable roots, all-or-nothing source-block evacuation, and the tightened runtime-holder policy — **all stays**; only the default flip is reverted.
+
+  #7876's own acceptance criteria required "a dependency-scale stress corpus clean" before re-enabling, and no such corpus exists. The structural reason it cannot exist yet: selection needs `dead_bytes >= live_bytes` on an old page, i.e. promote-then-die at scale, and no program in the 19-benchmark corpus can produce a candidate page — the `retain` family survives at 999–1000‰ (old pages ~fully live) and the `churn` family promotes almost nothing. Default-on therefore bought no benefit signal and no regression signal while inheriting the full old-address rewrite surface. Every GC gate was also still queued and unexecuted when it merged.
+
+  An unrecognised value is now **OFF** rather than ON, so a typo cannot silently enable old-generation relocation.
+
+  When a fragmentation workload exists that can exercise this, the losing arm gets deleted rather than left standing.
+
+- **The OFF state has behavioural coverage for the first time.** `select_old_page_defrag_pages_from_snapshot` does not consult the knob — the gate lives only in `select_old_page_defrag_pages` — so every pre-existing selection test bypassed the switch entirely, and the thread-local override was only ever set to `Some(true)`. Adds `OldDefragTestDisable` and a test asserting the disabled path short-circuits **before** the O(old pages) page-meta snapshot, pinning the gate's placement and not merely its effect.
+
+  The test asserts the snapshot call counter rather than the returned selection, with a load-bearing positive control: its first version asserted only "disabled returns nothing", which is vacuous in a process with no eligible old pages and passes identically against a kill switch that does nothing. Sabotage-verified — deleting the early-out turns it red while the value-mapping test stays green.

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

@@ -106,8 +106,32 @@ impl Drop for OldDefragTestEnable {
     }
 }
 
+/// RAII *disable* for defrag on this thread, so the OFF arm is exercised
+/// deterministically rather than depending on the ambient environment.
+///
+/// Without this the OFF state has no behavioural coverage at all: the value
+/// mapping is unit-tested, but nothing asserts that a disabled collector
+/// actually declines to select a page. That gap is what #7917 records.
+#[cfg(test)]
+pub(crate) struct OldDefragTestDisable;
+
+#[cfg(test)]
+impl OldDefragTestDisable {
+    pub(crate) fn new() -> Self {
+        OLD_DEFRAG_TEST_OVERRIDE.with(|c| c.set(Some(false)));
+        OldDefragTestDisable
+    }
+}
+
+#[cfg(test)]
+impl Drop for OldDefragTestDisable {
+    fn drop(&mut self) {
+        OLD_DEFRAG_TEST_OVERRIDE.with(|c| c.set(None));
+    }
+}
+
 fn old_page_defrag_enabled_from_value(value: Option<&str>) -> bool {
-    !matches!(value, Some("0") | Some("off") | Some("false"))
+    matches!(value, Some("1") | Some("on") | Some("true"))
 }
 
 fn old_page_defrag_enabled() -> bool {
@@ -123,9 +147,23 @@ fn old_page_defrag_enabled() -> bool {
 }
 
 pub(super) fn select_old_page_defrag_pages(force: bool) -> OldPageDefragSelection {
-    // #7876 restored the mutable-root contract for old movable addresses and
-    // made defrag the production default. Keep an explicit kill switch for
-    // field diagnosis and rollback without shipping a second binary.
+    // #7876 restored the mutable-root contract for old movable addresses, and
+    // #7913 shipped that restoration with defrag ON by default. The contract
+    // work is sound and stays; the DEFAULT is what this reverts (#7917).
+    //
+    // #7876's own acceptance criteria said to "re-enable defrag only after the
+    // reproducer and a dependency-scale stress corpus are clean". No such
+    // corpus exists yet, and none of the 19 benchmark programs can produce a
+    // candidate page: selection needs `dead_bytes >= live_bytes` on an old
+    // page, which needs promote-then-die at scale. The retain family survives
+    // at 999-1000 permille and the churn family promotes almost nothing, so
+    // the suite yields neither a benefit signal nor a regression signal while
+    // still inheriting the full old-address rewrite surface.
+    //
+    // So this is opt-in until a fragmentation workload exists that can
+    // actually exercise it. When that lands, the losing arm gets DELETED
+    // rather than left standing -- per CLAUDE.md, a mode that still exists is
+    // a decision that has not been made.
     if !old_page_defrag_enabled() {
         return OldPageDefragSelection::default();
     }
@@ -135,17 +173,100 @@ pub(super) fn select_old_page_defrag_pages(force: bool) -> OldPageDefragSelectio
 
 #[cfg(test)]
 mod tests {
-    use super::old_page_defrag_enabled_from_value;
+    use super::{
+        old_page_defrag_enabled_from_value, select_old_page_defrag_pages, OldDefragTestDisable,
+        OldDefragTestEnable,
+    };
 
     #[test]
-    fn old_page_defrag_defaults_on_with_an_explicit_kill_switch() {
-        assert!(old_page_defrag_enabled_from_value(None));
+    fn old_page_defrag_is_opt_in_via_perry_gc_old_defrag() {
+        // Unset means OFF: defrag is opt-in until a fragmentation workload
+        // exists that can demonstrate it (#7917).
+        assert!(!old_page_defrag_enabled_from_value(None));
         assert!(old_page_defrag_enabled_from_value(Some("1")));
         assert!(old_page_defrag_enabled_from_value(Some("on")));
         assert!(old_page_defrag_enabled_from_value(Some("true")));
         assert!(!old_page_defrag_enabled_from_value(Some("0")));
         assert!(!old_page_defrag_enabled_from_value(Some("off")));
         assert!(!old_page_defrag_enabled_from_value(Some("false")));
-        assert!(old_page_defrag_enabled_from_value(Some("unexpected")));
+        // Anything unrecognised is OFF, so a typo cannot silently enable
+        // old-generation relocation.
+        assert!(!old_page_defrag_enabled_from_value(Some("unexpected")));
+    }
+
+    /// The OFF arm, asserted through the gated entry point rather than through
+    /// the value mapping.
+    ///
+    /// This matters because `select_old_page_defrag_pages_from_snapshot` does
+    /// NOT consult the knob — the gate lives only in
+    /// `select_old_page_defrag_pages` — so every pre-existing selection test
+    /// bypasses the switch entirely. Before this test the OFF state had no
+    /// behavioural coverage at all (#7917).
+    ///
+    /// The observable is the page-meta snapshot counter rather than the
+    /// returned selection, for two reasons. It needs no old-arena fixture, and
+    /// more importantly an empty selection is **not** evidence on its own: a
+    /// bare test process has no eligible old pages, so asserting only
+    /// "disabled returns nothing" passes just as happily against a kill switch
+    /// that does nothing at all. That is the gate-that-cannot-fail shape this
+    /// codebase keeps re-learning, and the first version of this test walked
+    /// straight into it.
+    ///
+    /// So the positive control is load-bearing: it proves the enabled path
+    /// really does reach the snapshot, which is the thing the disabled path
+    /// must then be shown to skip.
+    ///
+    /// It also pins the *placement* of the gate, not merely its effect: the
+    /// short-circuit must happen before the O(old pages) snapshot, so a
+    /// disabled collector pays nothing on every ordinary minor.
+    #[test]
+    fn disabled_defrag_short_circuits_before_taking_a_page_snapshot() {
+        use crate::arena::old_page_meta_snapshot_calls_for_tests as snapshot_calls;
+
+        let before_enabled = snapshot_calls();
+        let enabled = {
+            let _enable = OldDefragTestEnable::new();
+            select_old_page_defrag_pages(true)
+        };
+        let enabled_calls = snapshot_calls() - before_enabled;
+
+        let before_disabled = snapshot_calls();
+        let disabled_forced = {
+            let _disable = OldDefragTestDisable::new();
+            select_old_page_defrag_pages(true)
+        };
+        let disabled_unforced = {
+            let _disable = OldDefragTestDisable::new();
+            select_old_page_defrag_pages(false)
+        };
+        let disabled_calls = snapshot_calls() - before_disabled;
+
+        assert_eq!(
+            enabled_calls, 1,
+            "positive control: enabled defrag must reach the page snapshot. If \
+             this is 0 the assertions below prove nothing, because a switch \
+             that never runs looks identical to one that correctly declines"
+        );
+
+        assert_eq!(
+            disabled_calls, 0,
+            "the kill switch must short-circuit BEFORE the O(old pages) \
+             snapshot, so a disabled collector pays nothing per minor"
+        );
+
+        // `force` bypasses the dead>=live ratio, so this also proves the gate
+        // beats a forced selection rather than merely losing the ratio test.
+        assert_eq!(disabled_forced.selected_pages, 0);
+        assert_eq!(disabled_forced.candidate_pages, 0);
+        assert!(disabled_forced.pages.is_empty());
+        assert!(disabled_forced.page_order.is_empty());
+        assert_eq!(disabled_forced.selected_live_bytes, 0);
+        assert_eq!(disabled_forced.selected_reclaimable_bytes, 0);
+        assert_eq!(disabled_unforced.selected_pages, 0);
+        assert!(disabled_unforced.pages.is_empty());
+
+        // Sanity: the enabled arm returned a real (possibly empty) selection
+        // rather than the disabled default, i.e. the two paths are distinct.
+        let _ = enabled;
     }
 }