Skip to content

fix(gc): release the prototype-registry latch when a prune drains it (#7737) - #7740

Merged
proggeramlug merged 3 commits into
mainfrom
gc/7737-prototype-latch-reset
Aug 10, 2026
Merged

fix(gc): release the prototype-registry latch when a prune drains it (#7737)#7740
proggeramlug merged 3 commits into
mainfrom
gc/7737-prototype-latch-reset

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixed

  • The prototype registry's fast-path latch was one-way, so a single Object.setPrototypeOf disabled it for the life of the process (GC follow-ups from #7733: one-way prototype latch (#7510 shape), missing positive-direction pacing test, unmeasured grow-then-churn transition, advisory-only ratchet #7737 item 1). OBJECT_PROTOTYPES_NONEMPTY is set when a non-meta-capable owner records a prototype and was never cleared — including by prune_dead_object_prototype_owners when its retain() drains the map back to empty.

    That became load-bearing with perf(gc): stop re-marking a heap where nothing dies — yield-adaptive major pacing + the evacuation move-hook mutex #7733, which added a third reader: the evacuation move hook object_static_prototype_owner_moved consults the latch once per moved object to skip a process-global Mutex<HashMap> and a SipHash lookup (2.5 M of each on retain.ts). So one incidental re-prototyping early in a run — of a RegExp, say — silently forfeited that win for every subsequent evacuation, with no signal that it had happened.

    This is perf(gc): layout side tables are 34% of object construction — the construction/death half of #5094 (allocation is 7.7%) #7510's finding recurring: "one immortal side-table entry nullified every is_empty() fast path", now with a third victim.

    The clear could not simply be added, and the reason is the interesting part. The latch was stored outside the mutex, deliberately before the insert, so that a reader observing it never misses a committed entry. Clearing under the lock against a set outside it loses entries:

    1. writer stores true;
    2. pruner takes the lock, retains to empty, clears the latch;
    3. writer takes the lock and inserts.

    — leaving a non-empty map with the latch false, which every reader skips. The set therefore moves under the same mutex, still before the insert, which serialises (1) and (3) against (2) and makes the interleaving impossible. The publish property is unchanged: a reader that sees true takes the lock and so sees whatever the writer committed. No extra cost — that path acquired the lock on the next line anyway.

    The regression test's load-bearing assertion is the last one, that the latch comes back down; everything before it passes with the bug present. Verified by removing the clear: "the registry is empty but the latch is still armed, so every evacuated object keeps paying the mutex + SipHash lookup for the rest of the process".

Summary by CodeRabbit

  • Bug Fixes

    • Improved runtime reliability by preventing race conditions during prototype cleanup.
    • Ensured stale registry state is cleared correctly after unused prototypes are removed.
    • Added regression coverage to verify cleanup completes as expected.
  • Documentation

    • Documented the prototype cleanup fix and its performance considerations.
  • Chores

    • Updated the application version to 0.5.1432.

Ralph Küpper added 2 commits August 10, 2026 04:00
…7737)

The latch was one-way, so a single Object.setPrototypeOf anywhere in a
process permanently disabled #7733's per-evacuated-object fast path. The
set moves under the mutex so the clear cannot race an in-flight insert.

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

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: 9470da1f-b808-4603-97f6-c994e1aac68a

📥 Commits

Reviewing files that changed from the base of the PR and between 53662e9 and c6c8ee3.

📒 Files selected for processing (1)
  • crates/perry-runtime/src/object/prototype_chain.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/perry-runtime/src/object/prototype_chain.rs

📝 Walkthrough

Walkthrough

The runtime now updates the prototype registry latch under its mutex and clears it after dead-owner pruning drains the registry. A regression test verifies latch release. Project version references and the changelog were updated.

Changes

Prototype registry latch

Layer / File(s) Summary
Latch synchronization and regression coverage
crates/perry-runtime/src/object/prototype_chain.rs
Latch publication now occurs under the registry mutex before insertion. Pruning clears the latch when the registry becomes empty. A test-only accessor and regression test verify this behavior.
Release metadata
changelog.d/7739-prototype-latch-drain.md, CLAUDE.md, Cargo.toml
The changelog documents the fix. Project version references advance from 0.5.1431 to 0.5.1432.

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

Possibly related PRs

Suggested labels: bug

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the fix in detail but omits the required Summary, Changes, Related issue, Test plan, Screenshots/output, and Checklist sections. Rewrite the description using the repository template and include the required sections, test commands, related issue status, and checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main fix: releasing the prototype-registry latch after pruning drains it.
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 gc/7737-prototype-latch-reset

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.

Actionable comments posted: 1

🤖 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 `@Cargo.toml`:
- Line 318: Revert both release metadata changes: restore
[workspace.package].version in Cargo.toml at lines 318-318 to 0.5.1431, and
restore CLAUDE.md’s Current Version at lines 11-11 to 0.5.1431. Leave the
changelog fragment as the source for this contributor PR’s runtime fix.
🪄 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: bddc510c-8cec-404e-a1b3-25ce4d73df23

📥 Commits

Reviewing files that changed from the base of the PR and between ed58ec6 and 53662e9.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • CLAUDE.md
  • Cargo.toml
  • changelog.d/7739-prototype-latch-drain.md
  • crates/perry-runtime/src/object/prototype_chain.rs

Comment thread Cargo.toml

[workspace.package]
version = "0.5.1431"
version = "0.5.1432"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep release version metadata out of this contributor PR.

The PR-keyed changelog fragment is the correct source for this runtime fix. Revert both version metadata edits; maintainers apply them during merge or release.

  • Cargo.toml#L318-L318: restore [workspace.package].version to 0.5.1431.
  • CLAUDE.md#L11-L11: restore Current Version to 0.5.1431.

As per coding guidelines, external contributor PRs must not change [workspace.package].version. Based on learnings, contributors must not update release metadata when a changelog.d/ fragment is present.

📍 Affects 2 files
  • Cargo.toml#L318-L318 (this comment)
  • CLAUDE.md#L11-L11
🤖 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 `@Cargo.toml` at line 318, Revert both release metadata changes: restore
[workspace.package].version in Cargo.toml at lines 318-318 to 0.5.1431, and
restore CLAUDE.md’s Current Version at lines 11-11 to 0.5.1431. Leave the
changelog fragment as the source for this contributor PR’s runtime fix.

Sources: Coding guidelines, Learnings

@proggeramlug
proggeramlug merged commit caaab6b into main Aug 10, 2026
15 of 16 checks passed
@proggeramlug
proggeramlug deleted the gc/7737-prototype-latch-reset branch August 10, 2026 02:08
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