Replace inherited-read table with guarded sites; fix mixed getters (integrates #11713)

[proggeramlug]

Current-head additional CI attribution: moving-witness job110531942868 has exactly the same five zero-movement failures as completed main087bd job110382314630. macOS native-roots job110538314178 fails linking the stdlib provider on the same eight CoreFoundation/Objective-C symbols as completed main606e job110400249998. Raw logs and exact comparison assertions are retained in the audit. These are failed checks, not positive movement or provider-link evidence. Both mandatory root jobs still need whole-job SUCCESS; the statepoints curated check has passed and its dependency-scale corpus is running.

Inherited reads and method calls now use per-site receiver/holder shape facts, replacing the process-wide inherited-read cache and its GC roots. Class getter and setter sites use live shape/prototype checks, and worker startup sends holder-backed sites through ordinary dispatch safely. This integrates fork #11713 while preserving its contributor commits.

Mixed accessor pairs retain generic getter dispatch when a closure getter remains beside a compiled setter. The ordered-delete Map regression uses real rooted GC objects and reloads addresses after allocation. Declared-class reads follow the live prototype link after Object.setPrototypeOf, preserve the original getter receiver, and stop at null. New fixtures cover prototype relinking, class-chain refusal, and moving inherited holders at depths 1–3.

Validation at 2452746b03b77f048f36d2f7b060af2760456667:

• Locked release compiler and matching runtime/stdlib static-wrapper build passed with LLVM22.1.4; source tree clean.
• All16 compiled integration tests passed: class_proto_relink1, method_site13, read_holder_accessor1, read_holder_entry1. The moving method/accessor tests assert active roots and rewrites under forced evacuation and from-space poisoning.
• All3 new standalone fixtures passed against pinned Node26.5.1. Holder movement reported 107,580 copied objects,8 holder-root rewrites, and5 primes (limit8). Class-prototype-hop checks asserted active class entries and refusal of plain holder/absent entries.
• Current main2584525 dependency updates are incorporated. Windows Inkwell0.9/internal0.14 lock entries coexist with non-Windows0.10/0.15; compiler/runtime/stdlib release check and lock downgrade gate passed.
• Full serial runtime library tests passed:4757/0failed/5ignored. Full stdlib243passed/1failed reproduces exactly the independently verified main2584525 DOMException thread-exit assertion at symbols_tests.rs484, with no additional failures. Final script lint completed110/111 executable commands, failing only the grandfathered Public benchmark evidence freshness check. Its compile tier and2CI-only commands were explicitly skipped. Current-head CI remains pending, including both mandatory root-dominance passes.

Historical validation at9b38e3a98a: runtime4757 passed/0failed/5ignored; all8 focused accessor/read-holder units and3 serial Map regression runs passed. Full stdlib243passed/1failed reproduces the identical DOMException thread-exit assertion on independent main2584525. These historical results do not establish current-head suite results or CI exceptions. Source review retains two unrelated pre-existing followups: old-parent declared methods visible through the method table, and duplicate block-scoped class names losing prototype writes.

The owner directed expedited merging after disclosure of +0.67% Zod instructions and +7% TypeScript peakRSS (394.6→422.4MB). The author attributes RSS to main pacing tracked in #11736; measurements and that attribution have not been independently reproduced here. Original #11713 stays open until landing is verified. Current-head CI, both actual root checks, and any failure attribution still gate landing.

Current-head CI attribution: GC-ratchet job110531945861 reports73 failed numeric rows, all exactly matching completed main087bd job110380943914. Windows native-root job110538313461 reports the same09_try_catch_roots funclet-refusal tripwire as completed main606e job110400250156. These exceptions do not cover arbitrary Windows build failures. Both mandatory root-dominance jobs and the remaining current-head CI still require actual completion; native-backend job110549152517 completed1841passed/5failed/1ignored; its5firstassertionmessages exactly match completed main9e05951071 job110249849088 with the reviewed testname rename, and its test source is unchanged from the reviewed9b38 source. The exception is limited to immutable245 head and this actual job URL. Both mandatory root jobs and all other checks still gate landing.

Summary by CodeRabbit

• New Features
  • Added faster property-read and setter paths for eligible objects, including inherited properties and class accessors.
  • Property reads now reflect changes to prototype chains and inherited values, including after garbage collection.
• Bug Fixes
  • Improved correctness for inherited reads and writes when prototypes change, objects move during garbage collection, or worker threads are active.
  • Added coverage for ordered map deletions and accessor behavior across garbage collection.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 35d62513-6351-4348-842a-87615353356d

📥 Commits

Reviewing files that changed from the base of the PR and between 2452746 and cd1a4ba.

📒 Files selected for processing (3)

• crates/perry-runtime/src/hot_diag.rs
• crates/perry-runtime/src/object/field_set_by_name/fast_paths.rs
• crates/perry/tests/class_field_miss_one_path.rs

 ___________________________________________________________________
< That's not technical debt - that's technical *predatory lending*. >
 -------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f87d53f3-9c26-4f8f-a206-cfd7046f7f00

📥 Commits

Reviewing files that changed from the base of the PR and between a53418a and 2452746.

⛔ Files ignored due to path filters (5)

• Cargo.lock is excluded by !**/*.lock
• crates/perry-codegen/src/gc_effects/linux-x86_64.tsv is excluded by !**/*.tsv
• crates/perry-codegen/src/gc_effects/macos-aarch64.tsv is excluded by !**/*.tsv
• crates/perry-codegen/src/gc_effects/windows-x86_64.tsv is excluded by !**/*.tsv
• crates/perry-codegen/src/wasm32/runtime_abi.tsv is excluded by !**/*.tsv

📒 Files selected for processing (1)

• changelog.d/11738-inherited-read-one-shape.md

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

• changelog.d/11738-inherited-read-one-shape.md

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The changes remove the inherited-read cache and add holder-backed read, method, class-accessor, and setter sites. Worker startup gates holder-backed site access. GC root handling and tests cover relocation, prototype changes, and worker execution.

Changes

Property and method sites

Layer / File(s)	Summary
Holder-backed inherited method sites crates/perry-codegen/src/expr/method_site.rs, crates/perry-runtime/src/object/method_site.rs, crates/perry/tests/method_site.rs	Inherited method entries retain direct holders and shape words. Generated hits reload holder slots. Worker startup disables site use, priming, publication, and later root scanning.
Holder-backed reads and inherited-read cache removal crates/perry-runtime/src/object/method_site/read_holder*, crates/perry-runtime/src/object/field_get_set/*, crates/perry-runtime/src/object/inherited_read_cache*, crates/perry-runtime/src/object/proto_validity.rs, crates/perry-runtime/src/typed_feedback/guards.rs, tests/fixtures/one_shape_*	The inherited-read cache and its read-path hooks are removed. Read sites validate live receiver, holder, class, prototype, accessor, and absent-entry facts. Generic getters confirm entries before publication.
Compiled class setter sites crates/perry-runtime/src/proxy/put_value*, crates/perry-runtime/src/proxy.rs, tests/fixtures/one_shape_setter_site/*	Packed-set caches gain a setter memo. Eligible class setters validate receiver, holder, accessor, and prototype state. Cached keys and holders are scanned as GC roots.
Class relinking and runtime validation crates/perry-runtime/src/object/class_registry/*, crates/perry-runtime/src/object/accessor_pair*, crates/perry-runtime/src/object/class_*, crates/perry-runtime/src/object/shapes.rs, crates/perry-runtime/src/value/addr_class.rs	Class prototype relinking and class lookup generation changes support class reads. Accessor pairs now use validated raw-getter probing. Address comments retain the alignment guard in the narrower delegate.
GC, fixtures, and supporting updates crates/perry-runtime/src/gc/*, crates/perry-runtime/src/hot_diag.rs, crates/perry-runtime/src/map.rs, scripts/*, changelog.d/*, test-files/*	GC registration and diagnostics no longer reference the inherited-read cache. Tests cover holder relocation, worker gating, accessor parity, and rooted Map pointer keys.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 24527

The changelog accurately describes the inspected changes and follows repository requirements. No actionable merge-blocking issue was identified; normal CI validation remains necessary.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to a5341

Guards constrain cached method access and prevent obsolete prototype fallback. However, a new prototype-read path can resume fallback after an allocating getter returns undefined, without an established guarantee that the retained values remain valid after memory relocation.

Retained concerns

• Medium · security · inferred: The newly added relinked-prototype read can execute an allocating getter, classify its undefined result as a miss, and return to fallback code that reads the original raw key and may reuse the receiver without a visible relocation reload. Getter-side rooting protects callback execution but does not rewrite those caller locals. A stale-pointer outcome is inferred; complete incoming pinning contracts and a reproducing execution remain unverified.

Security review details

Security Blast Radius

• inferred — The supported exposure is within a process executing compiled code that can mutate a class prototype and install allocating getters. The suspected outcome concerns native heap-pointer validity during fallback. Remote reachability, cross-tenant access, privilege escalation, and exposure beyond that process are not established by the inspected paths.

Security Findings and Attack Paths

• inferred — The decision-relevant path is prototype relink, ordinary recursive getter execution, allocation-driven relocation, an undefined getter result, and resumption of outer fallback using retained raw values. The new helper supplies the additional callback-and-miss route; similar pre-existing accessor machinery alone does not establish unchanged exposure. No exploit or stale-pointer fault was reproduced.

Trust Boundaries and Controls

• observed — Proxy dispatch roots the receiver, target, handler, and key before looking up and invoking a trap, then enforces the get invariant. Ordinary accessor invocation roots its getter and effective receiver across coercion, closure rebinding, and user code. These are substantial callback-boundary controls, but they do not demonstrate relocation reloads in outer fallback frames.

Resilience and Maintainability Implications

• observed — The nearby generic prototype-read implementation explicitly roots the prototype, key, receiver, and displaced override across recursive lookup. It provides an established ownership pattern against which the new relink path and its caller continuation can be evaluated.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 170 functions across 52 files. (1 skipped…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the primary change: replacing the inherited-read table with guarded sites and fixing mixed getters. It also identifies the integrated issue.
Description check	✅ Passed	The description is detailed and directly related to the changes. It covers the summary, implementation changes, related issues, validation results, known limitations, and performance impact. It does n…

Full details: Docstring Coverage

Explanation

Docstring coverage is 55.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 170 functions across 52 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

[proggeramlug]

Owner decision on the performance trade-off, posted by Claude, which owns the A2 lane. The decisions are recorded in the coordination log as items 59–61.

• Zod +0.67% instructions: accepted for landing ("0.6% is fine too for now. we must finish A2 asap"). Recovering it is a later follow-up.
• TypeScript peak RSS +7%: not a cost of this change. Old-generation live bytes and object counts match main at every full collection, and main takes the same high mode under perturbation. The difference is the collector-dependent arena re-arm tracked in GC: arena-bytes trigger re-arm depends on which collector ran (tsc peak RSS ±28 MB knife-edge) #11736. The owner chose to land without waiting for that fix.
• The headroom-policy experiment: stays out, as in this integration.
• Measurements: quiet dedicated host, 10 paired reps in Williams four-arm order. TypeScript median +0.01% and mean +0.02%, with 0/10 runs on the ~205B slow route. The 37 other GC workloads show no regression above noise. Details are in Remove inherited-read side table with shape-guarded read and accessor sites #11713's description.

So the merge decision is resolved: this integration may land once CI is green, apart from failures that fail identically on main. The mixed-getter fix and the Map fixture repair in this PR are welcome additions. #11713 can be closed as superseded once this lands.

[proggeramlug]

HOLD: don't merge yet. A sabotage sweep on the rebased A2 head (e9850488ac) found a probable correctness regression:

class B {} class C extends B {}
B.prototype.k = 5;
const inst = new C();
console.log(inst.k);                         // primes the inherited-read site
Object.setPrototypeOf(C.prototype, { k: 7 });
console.log(inst.k);                         // read at a different site

Node 26.5.1 prints 5 then 7, and main e322e70e96 prints 5 then 7. The A2 head prints 5 then 5, a stale read. Without the first read, main is wrong too (a separate, existing issue), but the primed case regressed. The likely cause is A2 removing the marked-prototype proto_validity bumps; this isn't bisected yet. I'm root-causing it now and will push a fix plus regression test to this PR's branch, or to a follow-up on it. The two sabotage-gap fixtures (one_shape_holder_move, one_shape_class_proto_hop) will come with it. The performance acceptance above still stands.

[proggeramlug]

Hold resolved. Four commits are pushed on top of 9b38e3a98a, so the head is now a53418acf4.

Root cause. It was not A2 state going stale. The generic read for a declared-class instance (resolve_proto_chain_field_inner, class_registry/prototype_objects.rs) and lookup_prototype_method walk the parent class id recorded when the class was declared. Neither reads the prototype that C.prototype actually links to now. After Object.setPrototypeOf(C.prototype, X), the relink correctly changes C.prototype's shape, so A2's class-read entry declines and the site re-primes against its live walk. The generic fallback still answered from the old class link. On main this was already wrong for unprimed reads; the deleted INHERITED_READ_CACHE masked it for primed sites only.

Fix (576ac60d2e). When C.prototype carries the user prototype-override flag, the class walk reads C.prototype's own properties. It then follows the recorded link, with the instance as receiver for getters, handling Proxy and stopping on a null link. lookup_prototype_method stops at C in that case. There's no new table and no global invalidation; the shape change from the relink already covers the site facts. This also fixes the unprimed case that main gets wrong.

Tests.

• a53418acf4 adds tests/fixtures/one_shape_class_proto_relink, whose expected output is Node 26.5.1's, plus crates/perry/tests/class_proto_relink.rs. They cover:
  • unprimed, primed at another site, primed at a function site, and same-site relink mid-loop;
  • a depth-3 holder with a middle prototype relinked, relink to null, relink onto another class's prototype, and relink onto a getter;
  • Object.create chains and constructor-function prototypes.
• Red on 9b38e3a98a (9 wrong lines), green with the fix. Main got 4 of these lines wrong.
• 715f1ed416 and 3fa4ae8d39 add two sabotage-gap fixtures:
  • one_shape_holder_move catches an unrooted plain data holder, which no fixture or test caught before.
  • one_shape_class_proto_hop catches class prototype ids admitted to holder facts, which only a unit test caught before.

Verification at a53418acf4 (perrymaster):

Check	Result
All 7 one_shape_* fixtures	pass
method_site, read_holder_accessor, read_holder_entry, class_proto_relink	pass (13/13, 1, 1, 1)
accessor_pair and read_holder unit tests	pass (3/3, 5/5)
Serial runtime suite	4739 passed, 1 failed: a DNS-dependent turnloop_net getaddrinfo test that is flaky on that host and untouched by this change
cargo fmt --check	pass
Lint	the same 4 reds as main
Inherited matrix	identical instruction counts per access before and after, Node output matched

Still wrong, and not regressions (same on main):

• After a relink, a method declared in the old parent class is still visible through the class method table (typeof inst.m).
• Block-scoped classes with duplicate names lose prototype writes.

Both go to follow-ups.

[proggeramlug]

CI status on 2452746b03, plus one fix pushed as cd1a4baacf:

• cargo-test-perry (6/8), class_field_miss_one_path::a_by_name_overwrite_keeps_the_layout_a_method_body_reads: red on main 2584525b26 too, with the same panic at :114.
  • Main's 8a51a41b51 ("Retire object layout notes and header state after ShapeId tracing") deleted the only increment of rt_overwrite_kept_typed, so the premise check could never pass. Main's push CI skips cargo-test-perry, so nobody saw it there.
  • The layout behaviour itself is fine: the test's real assertion (fewer than 16 pre-check misses in 3000 calls) passes.
  • cd1a4baacf restores the census counter, counting a typed object as one whose ShapeId has an F64 lane. It has no behaviour change and is census-only. The test passes 2/2. A sabotage (object_store_generalize before the in-bounds store) turns it red at :109.
  • A2's integration tests and the 7 one_shape_* fixtures × 4 GC seeds pass.
• cargo-test-perry (2/8), (5/8) and (7/8): these never ran a test. Each test harness's coherent-runtime cargo build hit spawnSync ... cargo ETIMEDOUT at 600 s. They will be re-run.
• cargo-test, perry-hir issue_5833 reflected_script_var_gets_an_early_nonconfigurable_global_slot: fixed separately by Fix Script reflection test setup after #11591 #11737 ("Fix Script reflection test setup after module-level function declarations are exposed on globalThis (ESM: they must not be) #11591").
• gc-ratchet, lint (public-baseline freshness), gc-moving-witnesses, native-backend / llvm-inprocess-complete, native-roots (Windows and macOS) / gc-native-roots-complete: these fail identically on main. gc-ratchet's 73 metric deltas are byte-identical to main's.