fix(gc): subtract live from-space bytes, not the block high-water, in a copied minor (#7901)

changelog.d/7943-live-from-space-census.md

@@ -0,0 +1,49 @@
+Fixed a cross-mode derivation error in the exact live-byte census (#7879/#7886's
+replacement metric): a copied minor subtracted a **block high-water** from an **exact
+object census**, charging the same dead bytes twice.
+
+A full or non-moving sweep publishes `sweep.arena_live_bytes`, an object-walk census that
+excludes the dead objects left as holes beside a survivor. It cannot reset those
+survivors' blocks, so the holes stay inside the block offsets that
+`copying_from_space_in_use_bytes()` sums. The next copied minor computed
+
+    pre_collection_live_bytes − from_space_high_water + copied + promoted
+
+which removes those holes a second time. `saturating_sub` then hides the arithmetic
+failure: when the young high-water exceeds unrelated old live bytes it erases
+old-generation occupancy from `heapUsed` and from major-GC pacing altogether, and the
+error persists into the next census.
+
+**Fix.** The sweep now publishes the *live* from-space share alongside the total
+(`SweepTraceStats::arena_live_from_space_bytes`, accumulated in
+`ArenaSweepObjectsState::keep_live_object` — the one funnel the census-publishing sweep
+routes live objects through, gated on a block-index test against Eden plus the active
+survivor). `record_arena_live_census` stores that share together with the from-space
+high-water at the same instant, and `arena::arena_live_from_space_bytes()` derives the
+current from-space contribution as `census.from_space_live + (high_water_now −
+census.high_water)`, clamped to the current high-water. The copied minor subtracts that.
+
+A copied minor publishes `None`, meaning "from-space is compacted by construction" — after
+the flip Eden is empty and the new active survivor holds only the copies, so live equals
+high-water there and the old and new derivations agree. That is why
+`copying_minors_preserve_prior_promotions_in_the_live_census` is unchanged.
+
+Two `debug_assert`s guard the subtraction site: live from-space bytes cannot exceed the
+from-space high-water, and cannot exceed the whole live census. A high-water subtraction
+can no longer silently consume unrelated generations.
+
+**Regression test**
+(`heap_accounting::copied_minor_after_a_non_moving_sweep_does_not_subtract_dead_holes_twice`)
+seeds an old live cohort, fills Eden with garbage plus one rooted survivor, runs a
+non-moving full sweep, then a copied minor. It asserts the setup is non-degenerate — the
+from-space high-water must exceed the live from-space bytes by at least half a block, or
+the two formulas coincide and a green run would prove nothing — that the old high-water
+derivation would have produced a strictly smaller figure on this exact state, that the old
+cohort is still present in `heapUsed`, and that major-GC pacing reads the same corrected
+number.
+
+Two comments were reclaimed to stay under the 2000-line-per-file gate: an
+allocate-black paragraph duplicated verbatim in `GcCycleState::new_full` and
+`new_minor_fallback`, and an 18-line description in the legacy `sweep_arena_objects` of a
+"two-phase probe-then-track" strategy the code no longer implements — the very next line
+already contradicted it.

crates/perry-runtime/src/arena/mod.rs

@@ -92,10 +92,11 @@ pub(crate) use walk::{
 
 // reset.rs
 pub(crate) use reset::{
-    active_survivor_block_index_range, copying_active_survivor_in_use_bytes,
-    copying_from_space_in_use_bytes, copying_prepare_to_space, copying_reset_from_spaces_and_flip,
-    old_arena_reclaim_dead_blocks, old_arena_reclaim_selected_dead_blocks,
-    survivor_arena_reclaim_dead_blocks, ArenaResetEmptyBlocksState, OldArenaReclaimDeadBlocksState,
+    active_survivor_block_index_range, block_in_copying_from_space,
+    copying_active_survivor_in_use_bytes, copying_from_space_in_use_bytes,
+    copying_prepare_to_space, copying_reset_from_spaces_and_flip, old_arena_reclaim_dead_blocks,
+    old_arena_reclaim_selected_dead_blocks, survivor_arena_reclaim_dead_blocks,
+    ArenaResetEmptyBlocksState, OldArenaReclaimDeadBlocksState,
     SurvivorArenaReclaimDeadBlocksState,
 };
 pub use reset::{arena_reset_all_blocks_to_zero, arena_reset_empty_blocks};
@@ -118,12 +119,12 @@ pub(crate) use quarantine::{
 pub use quarantine::{quarantine_stats, QuarantineStats};
 
 // stats.rs
-pub(crate) use stats::record_arena_live_census;
 pub(crate) use stats::{active_survivor_space, inactive_survivor_space};
 pub use stats::{
     arena_live_allocated_bytes, js_arena_stats, longlived_in_use_bytes, old_gen_in_use_bytes,
     pointer_in_nursery, pointer_in_old_gen,
 };
+pub(crate) use stats::{arena_live_from_space_bytes, record_arena_live_census};
 #[cfg(test)]
 pub(crate) use stats::{old_gen_in_use_bytes_recomputed, old_gen_in_use_bytes_resync};
 

crates/perry-runtime/src/arena/reset.rs

@@ -88,6 +88,18 @@ pub(crate) fn copying_from_space_in_use_bytes() -> usize {
     eden + survivor
 }
 
+/// #7901: is `block_idx` inside the copying collector's from-space (Eden plus
+/// the ACTIVE survivor semispace)? The inactive semispace is to-space and is
+/// empty at sweep time, so it deliberately does not count.
+#[inline]
+pub(crate) fn block_in_copying_from_space(
+    block_idx: usize,
+    general_n: usize,
+    active_survivor: &std::ops::Range<usize>,
+) -> bool {
+    block_idx < general_n || active_survivor.contains(&block_idx)
+}
+
 pub(crate) fn active_survivor_block_index_range() -> std::ops::Range<usize> {
     let general_n = ARENA.with(|a| unsafe { (*a.get()).blocks.len() });
     let survivor0_n = SURVIVOR_ARENA_0.with(|a| unsafe { (*a.get()).blocks.len() });

crates/perry-runtime/src/arena/stats.rs

@@ -11,6 +11,13 @@ struct ArenaLiveCensus {
     high_water_bytes: usize,
     arena_free_bytes: usize,
     old_free_bytes: usize,
+    /// #7901: the from-space (Eden + active survivor) share of `live_bytes`,
+    /// and the from-space high-water at the same instant. A copied minor
+    /// replaces from-space wholesale, so it must remove the LIVE share — the
+    /// high-water also covers dead holes this census already excluded, and
+    /// subtracting those a second time silently eats unrelated generations.
+    from_space_live_bytes: usize,
+    from_space_high_water_bytes: usize,
     valid: bool,
 }
 
@@ -21,6 +28,8 @@ crate::perry_thread_local! {
             high_water_bytes: 0,
             arena_free_bytes: 0,
             old_free_bytes: 0,
+            from_space_live_bytes: 0,
+            from_space_high_water_bytes: 0,
             valid: false,
         }) };
 }
@@ -33,19 +42,65 @@ fn arena_free_bytes() -> usize {
 /// Record the exact header-inclusive live bytes found by a completed GC walk.
 /// This is called after block reset/reclaim, so its high-water and hole
 /// snapshots describe the same instant as `live_bytes`.
-pub(crate) fn record_arena_live_census(live_bytes: usize) {
+///
+/// `from_space_live_bytes` is the from-space (Eden + active survivor) share of
+/// `live_bytes` (#7901). `None` means "from-space is compacted — every byte
+/// below its bump pointers is live", which is exactly true after a copying
+/// minor has flipped: Eden is empty and the new active survivor holds only the
+/// copies. A non-moving sweep must pass `Some(..)`: it leaves dead holes
+/// beside surviving objects, and those holes are inside the from-space
+/// high-water but NOT inside `live_bytes`.
+pub(crate) fn record_arena_live_census(live_bytes: usize, from_space_live_bytes: Option<usize>) {
     let high_water_bytes = arena_in_use_bytes();
+    let from_space_high_water_bytes = copying_from_space_in_use_bytes();
+    // Live can never exceed the high-water it sits in; clamping keeps one
+    // miscounted walk from making the derived non-from-space figure negative
+    // (and, through `saturating_sub`, silently zero).
+    let from_space_live_bytes = from_space_live_bytes
+        .unwrap_or(from_space_high_water_bytes)
+        .min(from_space_high_water_bytes);
     ARENA_LIVE_CENSUS.with(|census| {
         census.set(ArenaLiveCensus {
             live_bytes,
             high_water_bytes,
             arena_free_bytes: arena_free_bytes(),
             old_free_bytes: crate::gc::old_free_bytes(),
+            from_space_live_bytes,
+            from_space_high_water_bytes,
             valid: true,
         });
     });
 }
 
+/// #7901: how much of what [`arena_live_allocated_bytes`] currently reports
+/// lives in the copying collector's from-space (Eden + the active survivor).
+///
+/// This is the quantity a copied minor must subtract before adding back its
+/// copied/promoted survivors — NOT `copying_from_space_in_use_bytes()`, which
+/// is a block high-water. Two components, matching how the running census is
+/// itself derived:
+///
+/// * `census.from_space_live_bytes` — the exact live share the last collection
+///   measured, holes already excluded;
+/// * the from-space high-water growth since that census — bump allocations,
+///   which the running census also treats as live.
+///
+/// With no valid census the running figure is itself high-water-derived, so
+/// the high-water is the consistent answer.
+pub(crate) fn arena_live_from_space_bytes() -> usize {
+    let high_water_now = copying_from_space_in_use_bytes();
+    ARENA_LIVE_CENSUS.with(|census_cell| {
+        let census = census_cell.get();
+        if !census.valid {
+            return high_water_now;
+        }
+        census
+            .from_space_live_bytes
+            .saturating_add(high_water_now.saturating_sub(census.from_space_high_water_bytes))
+            .min(high_water_now)
+    })
+}
+
 /// Header-inclusive bytes occupied by live arena objects.
 ///
 /// A GC publishes an exact object census. Until the next collection, new bump

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

@@ -1368,6 +1368,9 @@ pub(super) fn gc_collect_minor_copying_fast_path_with_eligibility(
     let phase_start = trace_phase_start(trace);
     let from_space_bytes = crate::arena::copying_from_space_in_use_bytes();
     let pre_collection_live_bytes = crate::arena::arena_live_allocated_bytes();
+    // #7901: the LIVE share of from-space inside `pre_collection_live_bytes`.
+    // Captured here, before anything moves or resets.
+    let pre_from_space_live_bytes = crate::arena::arena_live_from_space_bytes();
     // #7742: decide BEFORE anything classifies, then retag the young blocks so
     // every classification for the rest of this cycle already reads the
     // generation those objects will have when it ends. The eligibility
@@ -1783,7 +1786,10 @@ pub(super) fn gc_collect_minor_copying_fast_path_with_eligibility(
             // the collections that run NO copying minor.
             eden_live_bytes: 0,
             eden_dead_bytes: 0,
+            // The copied minor publishes its census directly (below), not
+            // through these sweep fields.
             arena_live_bytes: 0,
+            arena_live_from_space_bytes: 0,
         };
         trace.pause_us = start.elapsed().as_micros() as u64;
         trace.capture_layout_scans();
@@ -1804,19 +1810,36 @@ pub(super) fn gc_collect_minor_copying_fast_path_with_eligibility(
     // nothing (see `credit_promoted_bytes_to_old_baseline`).
     credit_promoted_bytes_to_old_baseline(collector.stats.promoted_bytes);
     // Everything outside from-space retains its pre-minor accounting. Remove
-    // the entire Eden/active-survivor high-water, then add back exactly the
+    // the from-space share of that accounting, then add back exactly the
     // objects that survived by copy or promotion. This also preserves objects
     // promoted by an EARLIER minor: old-page cycle summaries do not retain a
     // complete allocated-byte census across later cycles (#7879 A/B caught
     // `12_large_live_set` dropping ~38 MiB of prior promotions from heapUsed).
-    // Whole-block promotion is covered too: subtracting the full from-space
-    // high-water excludes its dead bytes, while `promoted_bytes` adds back only
-    // marked objects. No second object walk is needed.
+    //
+    // #7901: subtract the LIVE from-space share, not `from_space_bytes` (the
+    // block high-water captured above for fragmentation telemetry). After a
+    // non-moving sweep the high-water still covers dead holes beside surviving
+    // objects — holes the exact census already excluded — so subtracting it
+    // charges the same garbage twice, and `saturating_sub` then quietly eats
+    // unrelated old-gen occupancy out of `heapUsed` and major-GC pacing.
+    debug_assert!(
+        pre_from_space_live_bytes <= from_space_bytes,
+        "live from-space bytes ({pre_from_space_live_bytes}) exceeded the from-space \
+         high-water ({from_space_bytes}) — the census split is inconsistent"
+    );
+    debug_assert!(
+        pre_from_space_live_bytes <= pre_collection_live_bytes,
+        "a from-space subtraction ({pre_from_space_live_bytes}) larger than the whole \
+         live census ({pre_collection_live_bytes}) would consume unrelated generations"
+    );
     let arena_live_bytes = pre_collection_live_bytes
-        .saturating_sub(from_space_bytes)
+        .saturating_sub(pre_from_space_live_bytes)
         .saturating_add(collector.stats.copied_bytes)
         .saturating_add(collector.stats.promoted_bytes);
-    crate::arena::record_arena_live_census(arena_live_bytes);
+    // `None`: to-space is compacted by construction — Eden is empty after the
+    // flip and the new active survivor holds only the copies, so from-space
+    // live == from-space high-water.
+    crate::arena::record_arena_live_census(arena_live_bytes, None);
     note_collection_finished_arena_occupancy();
     // The same argument one trigger over: a young generation that did not die
     // is a heap growing by LIVE data, so arena-growth pacing must not read that

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

@@ -987,16 +987,9 @@ impl GcCycleState {
     ) -> Self {
         let malloc_sweep_due = copied_minor_malloc_sweep_due(trigger.kind);
         let trigger_kind = trigger.kind;
-        // Allocate-black for the WHOLE cycle, from the first build slice on:
-        // the mark barrier only engages at the END of BuildValidPointerSet
-        // (the longest phase), so an object born during a build slice and
-        // installed via a runtime-internal raw store would be swept live
-        // (measured: identical 2,890-node loss with barrier-window-only
-        // birth flags). Cleared when the barrier disables at sweep entry
-        // (post-snapshot births cannot be reached by the in-flight sweep,
-        // and a mark they carried would leak into the next cycle as
-        // "already traced"). Every black birth is also pushed as a mark
-        // seed — see `gc_note_black_birth`.
+        // Allocate-black for the WHOLE cycle — see `new_full` above for why the
+        // barrier window alone is not enough, and `gc_note_black_birth` for why
+        // every black birth is also seeded.
         super::barrier::GC_BIRTH_EXTRA_FLAGS.with(|cell| cell.set(GC_FLAG_MARKED));
         Self {
             collection_kind: GcCollectionKind::Minor,
@@ -1948,11 +1941,17 @@ impl GcCycleState {
             trace.pause_us = elapsed_us;
             trace.capture_layout_scans();
         }
-        let arena_live_bytes = self
-            .sweep
-            .map(|sweep| sweep.arena_live_bytes as usize)
-            .unwrap_or_else(crate::arena::arena_live_allocated_bytes);
-        crate::arena::record_arena_live_census(arena_live_bytes);
+        // #7901: a non-moving sweep leaves dead holes beside survivors, so it
+        // must publish the LIVE from-space share alongside the total; a
+        // following copied minor subtracts that instead of the high-water.
+        let (arena_live_bytes, from_space_live) = match self.sweep {
+            Some(sweep) => (
+                sweep.arena_live_bytes as usize,
+                Some(sweep.arena_live_from_space_bytes as usize),
+            ),
+            None => (crate::arena::arena_live_allocated_bytes(), None),
+        };
+        crate::arena::record_arena_live_census(arena_live_bytes, from_space_live);
         // #7865: arena-growth pacing tests a POST-collection occupancy, which
         // is the same kind of quantity as its post-full baseline. Recorded here
         // rather than per-kind because this is the one site both kinds reach.