perf(gc): stop traversing the young graph twice — skip the copying minor's eligibility preflight when its answer is already known (#7645)

[proggeramlug]

Closes #7645.

The copying minor traversed the live young object graph twice — once in CopiedMinorEligibility::evaluate's preflight to prove nothing reachable is pinned, and again to copy. The first traversal produced no collection result. This removes it when its answer is already known, behind a safety argument that is mechanically enforced rather than asserted.

Measured

Pinned quiet mini (perry@perry-macos.local, M1, 8 GB), both arms built there with -p perry -p perry-runtime-static -p perry-stdlib-static, interleaved, 6 rounds, PERRY_NO_AUTO_OPTIMIZE=1 and a pinned PERRY_RUNTIME_DIR. Canonical json_pipeline fixture. Output SHA-256 identical on every row.

json_pipeline	200k	500k
build_out phase	622 → 489 ms (−21.4%)	1,659 → 1,245 ms (−25.0%)
total wall	1,004 → 866 ms (−13.7%)	2,606 → 2,190 ms (−15.9%)
parse	237 → 236 ms (−0.4%)	593 → 592 ms (−0.2%)
serialize	110 → 108 ms (−1.4%)	272 → 274 ms (+0.9%)

Spreads do not overlap in any moved cell — 500k build_out: base 1,651–1,672, arm 1,238–1,250; 500k total: base 2,596–2,622, arm 2,176–2,204 (n=6 each).

Vacuity guards, because both a stale archive and a dead subject have shipped here before:

• the two arms' libperry_runtime.a are hashed and asserted to differ (2bf2c92c… vs 1f25969e…), as are the two linked test binaries;
• an earlier run of the build script had cargo build … | tail -3 and copied the previous arm's artifacts behind tail's exit 0 — caught, and the pipe removed;
• PERRY_GC_DIAG on the arm reports eligible=true fallback=none preflight_skipped=true (skips=1 walks=0), so the subject ran.

The decision is unchanged. promoted_objects=4,117,015, promoted_bytes=280,996,840, freed_bytes=17,544 are byte-identical across arms. A field-by-field PERRY_GC_TRACE diff of all three cycles — 1,868 non-timing fields — reports 8 differences, every one telemetry of the removed traversal:

cycle-2 counter	main	this PR
layout_scans.pointer_slots_read	22,041,102	13,827,564
layout_scans.unknown_layout_slots_read	16,500,006	10,500,003
layout_scans.masked_pointer_slots_read	3,014,775	1,810,014
layout_scans.pointer_slot_bytes_read	176,328,816	110,620,512
layout_scans.pointer_free_{slots,ranges,payload_bytes}_skipped	649,680 / 216,560 / 5,197,440	halved
old_pages.dirty_slots	1,017,546	508,773

Cycle count, kinds, triggers, copied_*, promoted_*, freed_bytes, remembered_set, root_sources and sweep are identical.

What the walk was deciding, and why it can be skipped

CopyingNurseryPreflight::drain answers exactly two booleans:

1. is any transitively reachable Eden/FromSurvivor object GC_FLAG_PINNED? (check_ptr_with_reason)
2. was a non-arena candidate met while the malloc registry was unavailable and non-empty at cycle start? (classify_for_preflight)

(2) is already O(1): malloc_registry_available || malloc_registry_empty_at_start. (1) is O(live young graph) only because it searches for a fact that can be recorded when it is created.

gc::pin_object is now the single sanctioned setter of GC_FLAG_PINNED. It arms a process-wide monotone latch when — and only when — the pinned object sits in a space the copying minor relocates. With the latch clear and (2) decided, both walks provably return None. Note the direction: "no young pinned object exists at all" is stronger than the walk's "none is reachable", so the substitution is conservative rather than merely equal. When either proof is unavailable the walks run exactly as before, so the decision is never changed — only skipped when its outcome is already determined.

⚠️ The issue's pin-site analysis was incomplete, and that is the most useful finding

#7645 named three production GC_FLAG_PINNED setters and argued all three were harmless (malloc-space or Longlived). There are six, and three of them pin Eden objects:

site	space	in the issue?
thread.rs cross-thread spawn promise	malloc (deliberately)	yes
thread.rs::pin_promise (Atomics.waitAsync)	arena	yes
string/format.rs SMALL_INT_CACHE	Longlived	yes
perry-stdlib async_bridge::pin_promise_for_native_resolution	Eden — js_promise_new() is arena_alloc_gc; every fetch/zlib/ws/bcrypt/ioredis request	no
perry-ui-macos textfield::get_string_value	Eden — js_string_from_bytes	no
perry-ui-macos table::get_filter_text	Eden	no

The two AppKit sites wrote *gc_flags_ptr |= 0x04; against a hand-computed ptr - 8 + 1. They are invisible to grep GC_FLAG_PINNED — which is how an enumeration done by grep came back short by half, and precisely why the gate below scans for both shapes rather than keeping a list.

This does not sink the approach — the latch handles those sites correctly, they arm it — but it changes the honest claim about who benefits: perry-stdlib-async and AppKit programs keep today's behaviour; compute- and JSON-shaped programs get the walk removed.

Three enforcement layers, because a wrong latch is a use-after-move

move_young relocates a pinned object exactly as it would any other — it only preserves the bit — and the cross-thread promise queue holds a raw usize no scanner rewrites.

1. Static, in lint. scripts/gc_pin_sites.py fails on any site that originates a pin outside pin_object, matching the named form and any write into a gc_flags-named identifier whose right-hand side carries an integer literal with bit 2 set. It fails equally on a stale allowlist entry (the deferred_registration_flush_sites model in arena/tests.rs), and refuses to report green having seen fewer than 40 GC_FLAG_PINNED tokens. --self-test plants six offender shapes and requires each to be caught, plus the read/clear/preserve shapes to be left alone. The two flag-byte channels it deliberately does not scan — allocator birth flags (GC_BIRTH_EXTRA_FLAGS is only ever 0 or GC_FLAG_MARKED) and codegen's inline bump allocators (GC_FLAG_ARENA plus that same byte) — are documented in the script with why neither can originate a pin. It is a step of the already-required lint job, so it is a gate on merge from the first run.
2. Dynamic, at the instant it would matter. move_young already holds the flags byte in a register; on a preflight-skipped cycle it tests bit 2 and aborts with [gc-pin-latch] FATAL naming the header, rather than relocating it. One and and a never-taken branch. Deliberately not applied when the preflight ran: that path is unchanged here, and a divergence between the preflight's traversal and the copier's would be a separate bug that should not newly abort a program.
3. Tests. Every pinned-fallback test plants its pin through pin_object, so none can pass on an unsound configuration. gc/tests/copying/latch.rs adds the skip case (with a liveness assertion on the same trace), the Longlived- and malloc-pin cases that prove SMALL_INT_CACHE and spawn's cross-thread promise stay free, the monotonicity case, and a subprocess sabotage test that plants a raw young pin and requires the collector to die on SIGABRT with that message.

Sabotage-verified

Deleting the single YOUNG_PIN_EVER.store(true, …) line in pin_object and running each protection test alone (--exact, one process each; every run asserted to have reached the test binary via running 1 test in its output, so a plant that failed to compile could not be scored as a successful sabotage):

test	result under sabotage
latch::young_pin_via_pin_object_restores_the_walk	SIGABRT (layer 2 fired)
survival_and_malloc::test_copying_minor_falls_back_for_pinned_young_root	SIGABRT
survival_and_malloc::test_copying_minor_falls_back_for_pinned_young_dirty_slot	SIGABRT
survival_and_malloc::test_copying_minor_falls_back_for_transitive_pinned_young_child	SIGABRT
latch::the_latch_is_monotone_across_an_unpin	FAILED (assertion)
latch::no_pin_ever_means_the_preflight_walks_are_skipped (control)	passes

The control still passes, so the sabotage broke the protection and not the harness. A full-suite sabotage run also kills the test binary with SIGABRT, i.e. the bug is not survivable even by accident.

Why monotone

A decrementing counter would recover the fast path after a transient pin. Rejected because it adds a second completeness obligation of the same severity — every unpin site, where a spurious or double decrement is silently unsound in exactly the same use-after-move way. Monotone needs one proof. The cost is stated and asserted by the_latch_is_monotone_across_an_unpin: a process that ever pins young pays the walk for the rest of its life.

One ordering hazard found and closed

dirty_slot_preflight_reason took a remembered_dirty_snapshot(), whose first call on a thread arms the barrier and rebuilds the remembered set from the heap — a walk whose own comment says "nothing is marked yet when a collector first asks for the log". In a successful copying minor that first call was always the preflight's. Letting it fall through to the copy phase's snapshot would have run it after visit_mutable_root_slots had already evacuated root-reachable young objects, i.e. against a half-moved heap. arm_and_reconstruct_remembered_set_if_unarmed() is therefore called explicitly on the skip path, keeping it where it was. It is one-shot per thread, so every later cycle pays a thread-local flag read.

gc-ratchet: one cell moves, and classify says it is not the collector

Both arms measured on the pinned mini with the same script and probes, 7 repeats — not against the pinned baseline, which is from another host/version and would report drift (#7592).

144 metric medians compared across 12 probes. Two bit-identity-gated cells moved:

probe	metric	main	this PR
01_nursery_churn	heap_used_bytes	6,277,048	7,325,584 (+16.70%)
12_large_live_set	heap_used_bytes	59,942,456	59,945,120 (+0.004%)

classify cross-check — this is the #7558 residue, not retention:

probe	arm	conservative	precise	false-root excess
01_nursery_churn	main	6,277,048	5,228,512	1,048,536
01_nursery_churn	this PR	7,325,584	5,228,512	2,097,072
12_large_live_set	main	59,942,456	51,668,568	8,273,888
12_large_live_set	this PR	59,945,120	51,668,568	8,276,552

heap_used_precise_bytes is byte-identical on all twelve probes, both arms. The whole 01_nursery_churn delta is exactly one 1 MiB nursery block of false-root residue (+1,048,536), and it is deterministic — spread 0 over 7 samples on each arm. Mechanism: the probe's own gc() forces a conservative stack scan ([gc-scan-fallback] site=manual_collect automatic=false), and the preflight's drain → scan_object_fields → check_ptr_with_reason recursion used to overwrite stale pointer-shaped words deep on the native stack. With those frames gone, an older stale word survives to be mistaken for a root — and one such word pins a whole 1 MiB block, #7558's ~26,000× amplification.

12_large_live_set.heap_used_bytes is already ungated for exactly this cause (tolerances.json probe_overrides, #7554/#7558), so only 01_nursery_churn will report. Wall time improves on five probes (−2.2% to −7.6%) and no probe regresses.

Re-pinned, not exempted. 01_nursery_churn.heap_used_bytes is now 7,325,584 in benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json — one cell, one probe, tolerances.json untouched. The probe_overrides route 12_large_live_set uses is the wrong instrument here: that entry's rationale rests on genuine sample-dependence ("cannot carry a band whose premise is bit-identity"), whereas this delta is deterministic — spread 0 over 7 samples on both arms. A reproducible shift can still carry a band, and gating is one-way, so exempting a deterministic cell would surrender the gate permanently to avoid a re-pin.

Provenance was checked rather than assumed. The artifact's host block reads perry-macos.fritz.box, Apple M1, 8 cores, 8 GB, macOS 26.5.1 — the same mini both arms were measured on, so this is not a cross-host rebase of one row. And check --profile shared_ci was green for current main against the unedited artifact, so the 143 untouched cells are still in band: this is one row moving, not a regeneration. (The artifact is 24 commits behind main and 50 of its 144 cells have drifted within their bands; the top-level commit still reads 26b9c9d59 and now describes 143 of 144 cells, which the notes field records explicitly rather than leaving silent.)

Verified after the edit: validate --scope all structurally valid; check green against the PR arm; check green against main. And the cell still gates — planting one further 1 MiB block (8,374,160) makes check exit 1 and name it, so this is a live gate at a new value rather than a silently widened one.

On the audit the coordinator asked for: of the 37 cells that moved main→PR, exactly one carries a tolerance band — this one. The other 36 are rss_bytes / peak_rss_bytes / wall_ms, which have no entry in the gating family at all, plus 12_large_live_set.heap_used_bytes which its own override already ungates. Nothing was waved through as noise that the gate actually reads.

Counters that move, deliberately

Skipping a traversal removes its telemetry, and only that — the eight fields tabulated above. test_copying_minor_rewrites_exact_{object,closure}_pointer_* now expect masked_pointer_slots_read == 1 instead of 2 — one read by the copier where there used to be one by each walk — so the drop has a unit-scale witness that fails if the walk ever returns.

New: trace.copying_nursery.preflight_skipped, gc::copied_minor_preflight_skips() / copied_minor_preflight_walks(), and a PERRY_GC_DIAG line, so a verdict about this change can assert its subject was live (#7024/#7025) instead of passing on a cycle that never skipped anything.

Gates run locally

All 22 lint commands extracted from .github/workflows/test.yml (20 before this PR, plus the two new gc_pin_sites.py steps): 22 pass, 0 fail. Plus rustup run stable cargo fmt --all -- --check, cargo test -p perry-runtime --lib --no-fail-fast (1,915 pass, 0 fail), cargo check --all-targets, and cargo check -p perry-ui-macos --all-targets (not in the default workspace set).

Gap suite: partial, and reported as partial. ./scripts/run_gap_tests.sh (node 26.5.1, the .node-version pin) was started on this branch and completed 95 of 506 before the dev host's load average passed 19 and each test started taking minutes. Of those 95, three are non-PASS, and all three are already listed in the committed test-parity/gap_snapshot.json: test_gap_2159_defineproperty_class_prototype (issue 2159, standing since 2026-07-04), test_gap_2514_settracesigint (issue 2514, same date), and test_gap_4510_enum_forward_ref. Zero new failures, zero snapshot-listed tests that started passing — checked mechanically against the snapshot rather than by reading the tail of the log.

One nuance worth recording rather than smoothing over: 4510 is snapshotted as node_fail but this harness reports it parity_fail. run_parity_tests.sh records node_fail only for an abnormal node exit (perry_abnormal_exit), and node 26.5.1 refuses the file's TS enum with a thrown error and a plain exit 1, which falls through to the parity comparison. That is a pre-existing snapshot-vs-harness discrepancy about node's exit shape — nothing in perry-runtime can change whether node --experimental-strip-types accepts an enum — and it reproduces independently of this branch.

This is not a completed suite run and is not claimed as one. parity is tag-gated so the gap suite is not a PR gate; the moving-collector coverage that matters for this change is gc-moving-witnesses (the test_gap_gc_* stale-root reproducers under PERRY_GC_MOVING_LOOP_POLLS=1 plus forced evacuation), gc-stress, gc-root-dominance and gc-ratchet, all of which run on this PR.

Summary by CodeRabbit

• Performance

  • Reduced redundant copying-minor garbage-collection checks when safety can be determined automatically.
  • Preserved fallback behavior for pinned objects and added diagnostics for skipped checks.

• Reliability

  • Centralized object pinning and unpinning for consistent garbage-collection behavior.
  • Added safeguards to detect unexpected movement of pinned objects.

• Testing

  • Expanded coverage for pinning, relocation, latch behavior, and failure diagnostics.

• Chores

  • Added automated auditing to detect unsafe or outdated pinning sites.
  • Updated garbage-collection benchmark tracking and documentation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 88b3a303-5963-42c9-a96e-147845565093

📥 Commits

Reviewing files that changed from the base of the PR and between 2f35148 and c8ba0fa.

📒 Files selected for processing (1)

• changelog.d/7650-copying-minor-preflight-skip.md

🚧 Files skipped from review as they are similar to previous changes (1)

• changelog.d/7650-copying-minor-preflight-skip.md

---

📝 Walkthrough

Walkthrough

The change centralizes GC pinning, adds a monotone young-pin latch, and skips copying-minor preflight walks when eligibility is known. It records skip telemetry, aborts on unsafe pinned moves, migrates pin sites, updates benchmark data, and adds scanner-based CI enforcement.

Changes

Copying-minor preflight latch

Layer / File(s)	Summary
Centralized pin custody and call-site wiring crates/perry-runtime/src/gc/*, crates/perry-runtime/src/thread.rs, crates/perry-stdlib/src/common/async_bridge.rs, crates/perry-ui-macos/src/..., crates/perry-runtime/src/string/format.rs	The GC adds centralized pin/unpin APIs, a young-pin latch, counters, and js_gc_pin_user_ptr. Runtime, standard-library, macOS, and test pin sites use these APIs.
Preflight decision and collector state crates/perry-runtime/src/gc/copying.rs, crates/perry-runtime/src/gc/telemetry.rs	Copied-minor eligibility skips proven-unnecessary walks, reconstructs remembered sets on the skip path, propagates preflight_skipped, and reports counters and diagnostics.
Latch behavior and fallback validation crates/perry-runtime/src/gc/tests/copying/*, crates/perry-runtime/src/gc/tests/*	Tests cover relocation, young pins, monotonicity, non-relocating spaces, pointer-scan counts, fallback behavior, and raw-pin sabotage aborts.
Pin-site scanner and CI enforcement scripts/gc_pin_sites.py, .github/workflows/test.yml	The scanner detects symbolic and raw pinned-flag creation sites, validates allowlists, runs self-tests, and executes during linting.
Behavior and telemetry documentation changelog.d/7650-copying-minor-preflight-skip.md	The changelog documents skip conditions, enforcement, fallback diagnostics, latch behavior, remembered-set ordering, and counter changes.
GC ratchet baseline update benchmarks/gc_ratchet/baseline/gc-ratchet-v1.json	The baseline records the nursery-churn re-pin and updated deterministic heap measurements.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RuntimePinSite
  participant GC_PinAPI
  participant YoungPinLatch
  participant CopiedMinorEligibility
  participant CopyingCollector
  RuntimePinSite->>GC_PinAPI: pin_object or js_gc_pin_user_ptr
  GC_PinAPI->>YoungPinLatch: arm for relocatable young object
  CopyingCollector->>CopiedMinorEligibility: evaluate eligibility
  CopiedMinorEligibility->>YoungPinLatch: read latch state
  CopiedMinorEligibility->>CopyingCollector: preflight_skipped and eligibility
  CopyingCollector->>CopyingCollector: relocate or use pinned fallback

Loading

Possibly related PRs

• PerryTS/perry#7019: Both changes modify copying-minor preflight behavior in crates/perry-runtime/src/gc/copying.rs.
• PerryTS/perry#7306: Both changes modify lint-gate behavior in .github/workflows/test.yml.

Suggested labels: run-extended-tests

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main performance change: skipping redundant copying-minor preflight traversal.
Description check	✅ Passed	The description covers the summary, implementation, linked issue, extensive test evidence, measurements, safety controls, and telemetry changes.
Linked Issues check	✅ Passed	The PR satisfies [#7645] by skipping proven preflight walks, centralizing pinning, adding lint and sabotage safeguards, and documenting GC ratchet changes.
Out of Scope Changes check	✅ Passed	The workflow, runtime, tests, diagnostics, changelog, and ratchet updates directly support the linked issue and stated optimization objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/7645-skip-copying-preflight

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 3

🤖 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.

Inline comments:
In `@crates/perry-runtime/src/gc/pin.rs`:
- Around line 109-114: Update pin_object so GC_FLAG_PINNED is published to the
header before signaling YOUNG_PIN_EVER, and use the required synchronization so
collectors cannot observe the latch while reading stale gc_flags or race with
the flag update. Ensure any pin-backed heap cache introduced by this fix is
covered by a registered mutable root scanner.

In `@crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs`:
- Around line 1046-1054: Update the test containing
verify_copy_only_scanner_bits to establish CopyingNurseryTestGuard, or
equivalent locked latch-reset teardown, before arena_alloc_gc runs. Ensure
teardown resets the process-wide young-pin latch after the test so later tests
do not inherit its armed state.

In `@scripts/gc_pin_sites.py`:
- Around line 95-102: Update the Rule B audit around FLAG_WRITE, INT_LITERAL,
LET_BINDING, and raw_byte_offenders to parse source statement-by-statement
rather than skipping an entire line after a let binding. Match any identifier
containing gc_flags, recognize Rust decimal, hexadecimal, binary, and octal
integer literals, and add offender cases covering pointer-based writes,
(*h).gc_flags writes, binary/octal values, and same-line let bindings followed
by raw writes.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2d054ad9-8ce1-4d2b-b27c-096348aa7603

📥 Commits

Reviewing files that changed from the base of the PR and between c8394bf and ae7c6cb.

📒 Files selected for processing (21)

• .github/workflows/test.yml
• changelog.d/7650-copying-minor-preflight-skip.md
• crates/perry-runtime/src/gc/copying.rs
• crates/perry-runtime/src/gc/mod.rs
• crates/perry-runtime/src/gc/pin.rs
• crates/perry-runtime/src/gc/telemetry.rs
• crates/perry-runtime/src/gc/tests/alloc.rs
• crates/perry-runtime/src/gc/tests/barrier.rs
• crates/perry-runtime/src/gc/tests/copying.rs
• crates/perry-runtime/src/gc/tests/copying/latch.rs
• crates/perry-runtime/src/gc/tests/copying/survival_and_malloc.rs
• crates/perry-runtime/src/gc/tests/oldgen.rs
• crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs
• crates/perry-runtime/src/gc/tests/support.rs
• crates/perry-runtime/src/string/format.rs
• crates/perry-runtime/src/thread.rs
• crates/perry-stdlib/src/common/async_bridge.rs
• crates/perry-ui-macos/src/ffi.rs
• crates/perry-ui-macos/src/widgets/table.rs
• crates/perry-ui-macos/src/widgets/textfield.rs
• scripts/gc_pin_sites.py

[coderabbitai]

🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect whether pinning and copied-minor eligibility share a safepoint or lock.
ast-grep outline crates/perry-runtime/src/gc/pin.rs --items all
ast-grep outline crates/perry-runtime/src/gc/copying.rs --items all

rg -n -C 8 \
  'young_pin_latch_armed|pin_object|GC_FLAG_PINNED|gc_flags|move_young|Mutex|RwLock|safepoint|stop.?the.?world' \
  crates/perry-runtime/src/gc

Repository: PerryTS/perry

Length of output: 50370

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- target file outline ---\n'
ast-grep outline crates/perry-runtime/src/gc/pin.rs --view compact || true

printf '\n--- candidate files outline ---\n'
fd -a '^(pin|copying|cycle|root|header|unsafe|safepoint).*\.rs$' crates/perry-runtime/src/gc | sed 's#^\./##' | head -120 | xargs -r ast-grep outline --view compact 2>/dev/null | sed -n '1,240p'

printf '\n--- target lines ---\n'
sed -n '1,155p' crates/perry-runtime/src/gc/pin.rs

printf '\n--- focused match sites ---\n'
rg -n -C 4 'young_pin_latch_armed|pin_object|unpin_object|GC_FLAG_PINNED|youn[g]?_pin|pin_constrains_copying_minor|move_young|preflight|GC_FLAG_FORWARDED|synchronize_memory|compiler_fence|smp_barrier|stop_the_world|safepoint|Mutex|RwLock' crates/perry-runtime/src/gc -g '*.rs' | sed -n '1,260p'

Repository: PerryTS/perry

Length of output: 242

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- target file outline ---'
ast-grep outline crates/perry-runtime/src/gc/pin.rs --view compact || true

printf '%s\n' '--- target lines ---'
sed -n '1,155p' crates/perry-runtime/src/gc/pin.rs

printf '%s\n' '--- focused match sites ---'
rg -n -C 4 'young_pin_latch_armed|pin_object|unpin_object|GC_FLAG_PINNED|youn[g]?_pin|pin_constrains_copying_minor|move_young|preflight|GC_FLAG_FORWARDED|synchronize_memory|compiler_fence|smp_barrier|stop_the_world|safepoint|Mutex|RwLock' crates/perry-runtime/src/gc -g '*.rs' | sed -n '1,320p'

printf '%s\n' '--- pin_object call sites ---'
rg -n 'pin_object\(|unpin_object\(' crates/perry-runtime/src/gc crates/perry-runtime/src -g '*.rs' | sed -n '1,220p'

Repository: PerryTS/perry

Length of output: 43428

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- move_young and related copying symbols ---'
rg -n -C 6 'fn move_young|fn.*copy.*minor|evaluate.*preflight|Preflight|YOUNG_PIN_EVER|PREFLIGHT_SKIPS|PREFLIGHT_WALKS|CopiedMinor' crates/perry-runtime/src/gc -g '*.rs' | sed -n '1,380p'

printf '%s\n' '--- lock/synchronisation symbols ---'
rg -n -C 3 'fn gc_safepoint|gc_safepoint|stop_the_world|synchroniz|compiler_fence|fence|smp_barrier|YOUNG_PIN_EVER|PREFLIGHT|preflight|check.*ptr.*reason|move_young' crates/perry-runtime/src/gc -g '*.rs' | sed -n '1,420p'

printf '%s\n' '--- module lib outline ---'
ast-grep outline crates/perry-runtime/src/gc/mod.rs --view digest || true

Repository: PerryTS/perry

Length of output: 50369

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- copying.rs relevant section ---'
sed -n '260,325p' crates/perry-runtime/src/gc/copying.rs
printf '%s\n' '--- move_young implementation ---'
sed -n '541,610p' crates/perry-runtime/src/gc/copying.rs

printf '%s\n' '--- GC header definition ---'
rg -n -C 8 'struct GcHeader|gc_flags' crates/perry-runtime/src/gc/types.rs crates/perry-runtime/src/gc -g '*.rs' | sed -n '1,220p'

printf '%s\n' '--- acquire/safepoint definitions ---'
rg -n -C 6 'fn .*acquir|AcquireRelease|AcqRel|Acquire\s*\(|Release\s*\(|synchronize_memory|compiler_fence|fence\(|smp_barrier|synchronous_collection|gc_safepoint_moving_minor|js_gc_loop_safepoint' crates/perry-runtime/src/gc crates/perry-runtime/src -g '*.rs' | sed -n '1,340p'

Repository: PerryTS/perry

Length of output: 50369

---

Synchronize pin publication with the collector before skipping the preflight.

pin_object stores YOUNG_PIN_EVER with Release before writing GC_FLAG_PINNED to the non-atomic header. A collector can observe the latch, load gc_flags before it contains GC_FLAG_PINNED, skip its pin preflight, and then relocate the object. This also races with (*header).gc_flags access during gc_flags |= GC_FLAG_PINNED; register a mutable root scanner for any pin-backed heap cache introduced by the fix.

🤖 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/pin.rs` around lines 109 - 114, Update pin_object
so GC_FLAG_PINNED is published to the header before signaling YOUNG_PIN_EVER,
and use the required synchronization so collectors cannot observe the latch
while reading stale gc_flags or race with the flag update. Ensure any pin-backed
heap cache introduced by this fix is covered by a registered mutable root
scanner.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reset the young-pin latch for this test.

arena_alloc_gc creates a nursery object, so Line 1046 arms the process-wide monotone latch. Line 1054 does not disarm it. This test has no copying-nursery isolation guard, so later tests can unexpectedly run preflight walks and fail telemetry assertions.

Use CopyingNurseryTestGuard or the equivalent locked latch-reset teardown around this test.

🤖 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/runtime_roots/callback_scanners.rs` around
lines 1046 - 1054, Update the test containing verify_copy_only_scanner_bits to
establish CopyingNurseryTestGuard, or equivalent locked latch-reset teardown,
before arena_alloc_gc runs. Ensure teardown resets the process-wide young-pin
latch after the test so later tests do not inherit its armed state.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Close Rule B bypasses before relying on the custody audit.

FLAG_WRITE only matches identifiers that start with gc_flags. A write such as *header_gc_flags_ptr |= 0x04 passes the audit.

INT_LITERAL does not parse Rust binary or octal literals. Writes such as (*h).gc_flags |= 0b100 and (*h).gc_flags |= 0o4 also pass the audit.

raw_byte_offenders skips the complete line after a let gc_flags match. A same-line binding followed by a raw write also passes the audit.

Parse source one statement at a time. Match identifiers that contain gc_flags. Support all Rust integer literal bases. Add offender plants for these forms.

Proposed fix

-FLAG_WRITE = re.compile(r"\bgc_flags\w*\s*(?P<op>\|=|=)(?!=)(?P<rhs>[^;]*)")
+FLAG_WRITE = re.compile(r"\b\w*gc_flags\w*\s*(?P<op>\|=|=)(?!=)(?P<rhs>[^;]*)")

-INT_LITERAL = re.compile(r"0[xX](?P<hex>[0-9A-Fa-f_]+)|\b(?P<dec>[0-9][0-9_]*)\b")
+INT_LITERAL = re.compile(
+    r"0[xX](?P<hex>[0-9A-Fa-f_]+)"
+    r"|0[bB](?P<binary>[01_]+)"
+    r"|0[oO](?P<octal>[0-7_]+)"
+    r"|\b(?P<dec>[0-9][0-9_]*)\b"
+)

-LET_BINDING = re.compile(r"\blet\s+(?:mut\s+)?gc_flags\w*\s*[:=]")
+LET_BINDING = re.compile(r"\blet\s+(?:mut\s+)?\w*gc_flags\w*\s*[:=]")

-    for lineno, line in enumerate(strip_comments(text).splitlines(), start=1):
-        if "gc_flags" not in line or LET_BINDING.search(line):
+    for statement, lineno in statements(strip_strings(strip_comments(text))):
+        if "gc_flags" not in statement or LET_BINDING.search(statement):
             continue
-        for match in FLAG_WRITE.finditer(line):
+        for match in FLAG_WRITE.finditer(statement):
             rhs = STRING_LITERAL.sub("", match.group("rhs"))
             if TOKEN in rhs:
                 continue
             for literal in INT_LITERAL.finditer(rhs):
                 raw = literal.group("hex")
-                value = (
-                    int(raw.replace("_", ""), 16)
-                    if raw
-                    else int(literal.group("dec").replace("_", ""))
-                )
+                binary = literal.group("binary")
+                octal = literal.group("octal")
+                value = (
+                    int(raw.replace("_", ""), 16) if raw else
+                    int(binary.replace("_", ""), 2) if binary else
+                    int(octal.replace("_", ""), 8) if octal else
+                    int(literal.group("dec").replace("_", ""))
+                )
                 if value & PINNED_BIT:
-                    offenders.append((rel, lineno, line.strip()[:200]))
+                    offenders.append((rel, lineno, statement.strip()[:200]))
                     break

 OFFENDER_PLANTS = {
+    "raw-byte prefixed identifier": "    *header_gc_flags_ptr |= 0x04;\n",
     "raw-byte hex": "    *gc_flags_ptr |= 0x04;\n",
+    "raw-byte binary": "    (*h).gc_flags |= 0b100;\n",
+    "raw-byte octal": "    (*h).gc_flags |= 0o4;\n",

Based on PR objectives: an unarmed young pin makes the preflight skip unsafe.

Also applies to: 192-207, 299-301

🤖 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 `@scripts/gc_pin_sites.py` around lines 95 - 102, Update the Rule B audit
around FLAG_WRITE, INT_LITERAL, LET_BINDING, and raw_byte_offenders to parse
source statement-by-statement rather than skipping an entire line after a let
binding. Match any identifier containing gc_flags, recognize Rust decimal,
hexadecimal, binary, and octal integer literals, and add offender cases covering
pointer-based writes, (*h).gc_flags writes, binary/octal values, and same-line
let bindings followed by raw writes.

[proggeramlug]

Audit — merging as v0.5.1370

build_out −25.0% and total wall −15.9% at 500k, with non-overlapping spreads and identical output SHAs. But the reason I'm comfortable merging a change to the moving collector is the part that isn't the number.

The issue's pin-site analysis was wrong, and the gate is why that's now safe

#7645 named 3 setters, all "safe". There are 6, and 3 pin Eden objects — including two AppKit sites writing *gc_flags_ptr |= 0x04 on a hand-computed ptr - 8 + 1, structurally invisible to grep GC_FLAG_PINNED. The safety argument I asked you to build the gate around was itself resting on a half-complete enumeration. That is the single most important finding here, and it is a good argument for the rule that produced it: a list in a comment would have shipped the wrong claim.

I verified the gate can fail rather than trusting the self-test: planted a fresh file containing *gc_flags |= 0x04 — the exact grep-invisible shape — and gc_pin_sites.py exits 1 and names it. --self-test passes (6 offender shapes caught, 0 false positives across read/clear/preserve). Its first run against the real tree finding 15 genuine sites is what makes it a gate rather than decoration.

The move_young abort is the right second layer. A gate that assumes its own enumeration is complete is worth less than one that aborts when it isn't — and given that the enumeration was already wrong once, belt-and-braces is not paranoia here.

Sabotage verified independently: deleting both YOUNG_PIN_EVER.store lines → SIGABRT, error[ 0, Running unittests present. All 7 pin/latch tests green before.

Also worth naming: the ordering hazard the issue did not anticipate — remembered_dirty_snapshot() being what first arms the barrier and rebuilds the remembered set, so skipping it would have deferred that rebuild to after roots were evacuated. That is the kind of thing that would have shipped as an intermittent corruption, and it was found by reading rather than by a test failing.

The ratchet cell

Your re-pin is right and your verification of it is better than the re-pin: testing that the cell can still go red (+1 MiB → FAILED, naming the cell) is the step that separates a re-pin from a silent ungate. I confirmed the artifact validates, tolerances.json is untouched, and the 7 samples are bit-identical at the new value.

The attribution is mechanical rather than hand-waved, which is what I wanted: heap_used_precise_bytes 5,228,512 on both arms and byte-identical across all 12 probes, with the entire movement in false_root_excess. And the causal account — the preflight's recursion was incidentally scrubbing stale pointer-shaped words off the native stack, so removing it leaves one alive to pin a whole 1 MiB block — explains the sign and the exact magnitude. That is #7558 acting through this change, not this change retaining more.

Checking provenance on the commit axis after it passed on host, and then correcting your own "the baseline is stale, stop" by running the real check instead of your hand comparison, is exactly the right order of operations. "Moved" was your word; the gate's word is "ok" — and the gate's word is the one that governs.

I'm filing the mixed-provenance state as a follow-up: 143 cells from 26b9c9d59 and one from c8394bfdb is honest and recorded, but a future reader will assume uniform provenance. A full re-pin at a current commit wants doing on its own.

Gates: 22/22 extracted from test.yml, cargo fmt clean, cargo test -p perry-runtime --lib --no-fail-fast 1915 passed, artifact validate --scope all clean.

Your gap-suite self-correction — "I read the tail and never grepped the 500 lines above it" — is worth more than the number was. 206/506, 0 new failures, 0 unexpected passes, three knowns named with issue numbers, and the 4510 snapshot-vs-harness nuance chased to node's actual behaviour rather than smoothed over.