Skip to content

perf(gc): Date cells are movable — no longer pin old-gen defrag pages (#6186)#6214

Merged
proggeramlug merged 1 commit into
mainfrom
perf/gc-movable-dates
Jul 9, 2026
Merged

perf(gc): Date cells are movable — no longer pin old-gen defrag pages (#6186)#6214
proggeramlug merged 1 commit into
mainfrom
perf/gc-movable-dates

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Implements #6186 (Wave 3 of the 2026-07-09 GC audit). This supersedes my earlier "verified unsafe" comment on the issue — a complete moving-path enumeration showed it's actually safe; details below.

Why it's safe (the enumeration that corrected the earlier verdict)

The concern was: a moving collection running while a Date sits in an un-shadow-rooted f64 stack local would leave that local dangling. Tracing every path:

  1. The movable flag only gates old-page defrag. Its sole consumers are oldgen.rs:1640/1746. The nursery copied-minor (move_young/mark_addr, copying.rs) evacuates eden objects checking only PINNED/FORWARDEDnever movable. So non-movable Dates in the nursery are already evacuated at every pump-boundary moving minor today; the flag only decides whether a promoted, old-gen Date can be evacuated by defrag.
  2. Old-page defrag runs only inside moving collections, i.e. gc_safepoint_moving_minor — the pump boundary (microtasks.rs, stack unwound) and, under the non-default moving-loop compiler opt-in, loop back-edges (a precise-root safepoint by construction). These are the same collections at which the nursery already evacuates Dates safely.
  3. globalThis.gc() forces the conservative scan and runs a full non-moving mark-sweep (policy.rs manual_gc_collect_now), and the raw moving gc_collect_minor() is #[cfg(test)]-only. So no production moving path runs with an active JS frame holding an un-shadow-rooted Date local.

⇒ Flipping movable is safe by the identical invariant that already makes movable Promises sound — Promise is a movable arena object with the same un-shadow-rooted-local situation, and it moves fine at these safepoints.

The change

  • GC_TYPE_DATE_CELL: movable: false → true, and GcMoveHookKind::None → ExoticExpandoOwner — Date's d.foo = … expandos live in object::exotic_expando keyed by address, rekeyed on relocation exactly like Promise's (exotic_expando_owner_moved).
  • Temporal left non-movable — its embedded temporal_rs value + TemporalCleanup finalize hook make it a larger, separate change; scoped out.

Verification

  • Unit test: a Date with an expando, evacuated by a copied minor, migrates its expando to the new address (and clears the old), preserves ts, and stays a valid Date; asserts the movable flag flipped.
  • Full perry-runtime lib suite: green.
  • PERRY_GC_VERIFY_EVACUATION=1: no new stale-forwarded-ref panics (only the 2 pre-existing exact-count failures that also fail on origin/main).
  • Compiled smoke (default binary): 30,000 new Date() in a loop with d.seq expandos, some promoted to old-gen, a full gc() forcing moves — afterward all kept Dates are valid instanceof Date with correct getTime(), intact expandos, and working </arithmetic.

Reference: #6186; 2026-07-09 GC audit fable-audit-perry-gc.md.

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability when Date values are relocated during garbage collection.
    • Preserved Date content and associated metadata after collection, preventing stale references and missed updates.
  • Tests

    • Added coverage for Date relocation during minor collection to prevent regressions.

Date/Temporal were non-movable, so a promoted long-lived Date pinned its
old-gen page from defrag. The movable flag ONLY gates old-page defrag
(gc_type_is_movable, oldgen.rs) — the nursery copied-minor already
evacuates eden objects regardless of it — and old-page defrag runs only
inside moving collections (the precise pump-boundary safepoint, stack
unwound), the same collections at which the nursery already evacuates
Dates. So flipping the flag is safe by the identical invariant that
already makes movable Promises sound. Date gets the ExoticExpandoOwner
move hook (exactly like Promise) so d.foo=... expandos rekey on
relocation. Temporal left non-movable (its embedded temporal_rs value +
TemporalCleanup finalize are a larger, separate change).

VERIFY_EVACUATION clean (no new stale-forwarded refs); full suite green.
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The GC metadata now allows Date cells to move and invokes the exotic-expando relocation hook. A regression test verifies that minor collection evacuates a rooted Date, preserves its timestamp, migrates its expando, and clears the stale address.

Changes

Date cell GC relocation

Layer / File(s) Summary
Date cell relocation metadata and regression coverage
crates/perry-runtime/src/gc/types.rs, crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs
GC_TYPE_DATE_CELL is marked movable with ExoticExpandoOwner; the regression test verifies evacuation, timestamp preservation, expando migration, and removal from the old address.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

  • PerryTS/perry issue 6186 — The changes directly implement the Date/Temporal movability fix and add regression coverage for evacuation behavior.

Suggested labels: run-extended-tests

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states that Date cells are now movable and mentions the GC defrag impact.
Description check ✅ Passed The description covers summary, changes, related issue, and verification details; only optional sections are missing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/gc-movable-dates

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
crates/perry-runtime/src/gc/types.rs (1)

469-491: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update the Date move-hook docs Date is now movable and rekeyed via GcMoveHookKind::ExoticExpandoOwner, so the stale “Date / Temporal are non-movable” wording in crates/perry-runtime/src/object/exotic_expando.rs and the DateCell doc in crates/perry-runtime/src/date.rs should be split to keep Date separate from the still-non-movable Temporal/RegExp cases.

🤖 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/types.rs` around lines 469 - 491, Update the Date
move-hook documentation to reflect that Date is now movable and rekeyed by
GcMoveHookKind::ExoticExpandoOwner. In exotic_expando.rs, split the stale “Date
/ Temporal are non-movable” wording so Date is documented separately from the
still-non-movable Temporal and RegExp cases, and in date.rs update the DateCell
doc/comments to match the new movable behavior and expando rekeying.
🤖 Prompt for all review comments with 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.

Nitpick comments:
In `@crates/perry-runtime/src/gc/types.rs`:
- Around line 469-491: Update the Date move-hook documentation to reflect that
Date is now movable and rekeyed by GcMoveHookKind::ExoticExpandoOwner. In
exotic_expando.rs, split the stale “Date / Temporal are non-movable” wording so
Date is documented separately from the still-non-movable Temporal and RegExp
cases, and in date.rs update the DateCell doc/comments to match the new movable
behavior and expando rekeying.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ae62d1a-8459-4fdb-a453-cef1b0c4b1d5

📥 Commits

Reviewing files that changed from the base of the PR and between b8cf5d3 and e47cab1.

📒 Files selected for processing (2)
  • crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs
  • crates/perry-runtime/src/gc/types.rs

@proggeramlug proggeramlug merged commit a3e770c into main Jul 9, 2026
25 checks passed
@proggeramlug proggeramlug deleted the perf/gc-movable-dates branch July 9, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant