Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/perry-runtime/src/arena/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ pub(crate) use page_meta::{
pub(crate) use page_meta::{
deferred_old_page_registrations_len, generation_page_base,
old_arena_page_index_clear_for_tests, old_page_meta_for_tests,
old_page_meta_snapshot_calls_for_tests, pending_promoted_page_runs,
old_page_meta_snapshot_calls_for_tests, pending_promoted_page_runs, register_promoted_page_run,
reset_old_page_meta_snapshot_calls_for_tests, DEFERRED_OLD_PAGE_REGISTRATION_CAP,
GENERATION_CLASS_SHIFT, GENERATION_PAGE_SIZE,
};
134 changes: 134 additions & 0 deletions crates/perry-runtime/src/gc/tests/promote_in_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,3 +413,137 @@ fn a_low_survival_cycle_still_evacuates_and_moves_the_object() {
);
assert!(crate::arena::pointer_in_nursery(after));
}

// ---------------------------------------------------------------------------
// #7913 interaction: old-page relocation vs a still-DESCRIBED promoted run
// ---------------------------------------------------------------------------

/// #7914 describes a promoted page's object list by its `first_header` address
/// and expands it lazily; #7913 relocates old pages BY DEFAULT. If a page
/// carrying an unexpanded run could be relocated, the deferred expansion would
/// later parse stale addresses — the silent-corruption shape.
///
/// It cannot, and this pins the mechanism that makes it so:
/// `evacuate_selected_old_pages_collecting` snapshots its source-block
/// occupants through `old_arena_walk_objects_on_pages`, which expands every
/// pending run on every page of the source blocks BEFORE a single object is
/// forwarded. #7913 widened that enumeration from the selected pages to the
/// whole containing source blocks, so it covers strictly more than before.
///
/// The test is written so it cannot pass by accident: the objects are removed
/// from the eager index first, so a run that is NOT expanded leaves
/// `source_headers` empty, the all-or-nothing guard returns early, and
/// `old_page_moved_objects` reads 0 instead of 3. Deleting the
/// `materialize_promoted_page_runs` call in `old_arena_walk_objects_on_pages`
/// turns this red.
#[test]
fn old_page_relocation_expands_a_described_run_before_it_moves_anything() {
let _isolation = copying_nursery_isolation_lock();
reset_remembered_set();
clear_marks();
clear_mark_seeds();
CONS_PINNED.with(|s| s.borrow_mut().clear());

// Three real old-gen objects, contiguous and linearly parseable — the
// shape a promoted block hands to OLD_ARENA.
let users: Vec<usize> = (0..3)
.map(|_| crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_OBJECT) as usize)
.collect();
let headers: Vec<(*mut GcHeader, usize)> =
users.iter().map(|&u| old_test_header_and_size(u)).collect();
let first = headers[0].0 as usize;
let last = headers[2].0 as usize;
let total: usize = headers.iter().map(|&(_, t)| t).sum();

let mut selected_pages = crate::fast_hash::new_ptr_hash_set();
for &(header, size) in &headers {
for (page, _) in crate::arena::old_object_page_overlaps(header as usize, size) {
selected_pages.insert(page);
}
}
// All three must share one page for the run to describe them as one span;
// 64-byte objects out of a fresh block do, but assert rather than assume.
assert_eq!(
selected_pages.len(),
1,
"test setup expects one page; a multi-page split would need one run per page"
);
let page = *selected_pages.iter().next().unwrap();

// Drop the eager index built at birth, then DESCRIBE the same objects.
// From here the run is the only record that this page has occupants.
crate::arena::old_arena_page_index_clear_for_tests();
crate::arena::register_promoted_page_run(page, first, last, headers.len(), total);
Comment on lines +449 to +476

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Cover the unselected page in the widened source block.

This setup registers the described run on the selected page. An implementation that walks only selected_pages would still expand the run and pass.

Create two pages in one source block. Select one page. Put the described run only on the other page. Then assert that relocation expands and moves both pages’ occupants.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/gc/tests/promote_in_place.rs` around lines 449 -
476, Update the promote-in-place test setup around register_promoted_page_run to
allocate a source block spanning two pages, select only one page, and register
the described run exclusively on the other page. Assert that relocation expands
the widened source block and moves occupants from both pages, covering pages
absent from selected_pages.

assert_eq!(
crate::arena::pending_promoted_page_runs(),
1,
"the page must be DESCRIBED going in, or this test proves nothing"
);

for &(header, _) in &headers {
unsafe {
(*header).gc_flags |= GC_FLAG_MARKED;
}
}

let mut new_headers = Vec::new();
let mut original_headers = Vec::new();
let moved = evacuate_selected_old_pages_collecting(
&selected_pages,
&mut new_headers,
&mut original_headers,
);

assert_eq!(
moved.old_page_moved_objects,
headers.len(),
"relocation must see every occupant of a DESCRIBED page. Seeing fewer \
means it relocated a block while some occupants were still only \
described — their memory would be released with live objects in it, \
and the deferred expansion would parse recycled addresses"
);
assert_eq!(
crate::arena::pending_promoted_page_runs(),
0,
"the run must have been consumed by the relocation's own enumeration"
);
for &(header, _) in &headers {
unsafe {
assert_ne!(
(*header).gc_flags & GC_FLAG_FORWARDED,
0,
"every described occupant must be forwarded, not just indexed"
);
}
}

release_evacuated_original_forwarding_stubs(&original_headers);
clear_marks();
CONS_PINNED.with(|s| s.borrow_mut().clear());
}

/// The second, independent line of defence, stated as a test rather than a
/// comment: a page that has only just been promoted is not defrag-eligible at
/// all, because `register_promoted_page_run` records live bytes and never dead
/// ones, and `old_page_defrag_eligible` requires `dead_bytes > 0`. Dead bytes
/// come only from the old-gen sweep, and the sweep's cycle constructor
/// (`GcCycleState::new_full`) expands every pending run before it starts.
#[test]
fn a_freshly_described_page_is_not_defrag_eligible() {
let _isolation = copying_nursery_isolation_lock();
let user = crate::arena::arena_alloc_gc_old(64, 8, GC_TYPE_OBJECT) as usize;
let (header, size) = old_test_header_and_size(user);
let page = crate::arena::old_object_page_overlaps(header as usize, size)[0].0;
crate::arena::old_arena_page_index_clear_for_tests();
crate::arena::register_promoted_page_run(page, header as usize, header as usize, 1, size);

let meta = crate::arena::old_page_meta_for_tests(page).expect("promotion records page meta");
assert_eq!(
meta.dead_bytes, 0,
"a promotion records live bytes only; dead bytes are the sweep's to record"
);
assert!(
!super::super::oldgen_defrag::old_page_defrag_eligible(meta),
"a page whose run is still described must not be a defrag candidate"
);
}
Loading