perf: class accessors validated by shape facts, called directly (#10498)

[proggeramlug]

Closes #10498. Class accessor sites are validated by shape facts only. The global-counter checks are gone: class_lookup_surface_generation, proto_validity, vtable_generation and accessor_link_still_current.

Instructions per op

Row	main	this PR	Target	Node
getter_read2	1,540	219	≤ 400	25.7
setter_write2	1,207	313	≤ 700	20.6
setter_ctor	3.6× field_ctor	1.52×	≤ 2×	–
field rows	418 / 58 / 80	425 / 58 / 80	–	–

How the site validates

1. The receiver's shape matches the cached one, which pins the holder prototype.
2. The holder's shape matches, so the key is still an accessor there.
3. The holder's slot still holds the cached getter/setter pair, which the site keeps rooted.
4. Then it calls the compiled getter or setter directly.

A class's link to its declared prototype is now written once. Replacing it, or a generic-origin redirect, gives the displaced prototype a fresh ShapeId. The per-hit pair decode and the handle scopes are removed. The emitted read and store paths gain an inline accessor arm on 64-bit targets. The inline setter arm takes Number values only; anything else goes to the runtime miss path, which keeps the value rooted.

js_register_class_generic_origin is reclassified from Leaf to Reenters. The Linux gc_call_effects table is regenerated, and the Windows row is edited to match; macOS was already Reenters.

Tests

• test_gap_class_accessor_shape_facts.ts covers two-level inheritance, subclass override, runtime defineProperty, delete, shadowing, setPrototypeOf, static and object-literal accessors, getter-only/setter-only, throwing accessors, and heap values. It matches node on main, on this PR, and on this PR under forced evacuation with the verifier.
• New codegen check verify_accessor_arm. 11 codegen tests that pinned the old block structure are updated.
• Sabotage: skipping the receiver-shape check in either the emitted getter or setter arm turns the gap test red; disabling the prototype retire turns 3 runtime tests red.

Verification (on ebc858c; this branch is the same commit replayed onto f6c873d, a CI-only change)

• runtime 4812/0; codegen 1912 passed, 1 failed (manifest_consistency, which fails on main since perf: batch corked sockets and streamline interpreter scope access #11757).
• Gap subset (208 tests): identical results to main.
• tsc: instructions +0.04%, full collections 82/82, output matches node.
• Zod: instructions −0.11%, full collections 0/0, output matches node.
• fmt, file size, call funnel and gc_call_effects --check pass.

Follow-ups, not in this PR

• Accessors inherited through 2+ levels, and object-literal accessors, still take the generic path. They are correct.
• ObjectMeta.accessor_key_bits stays, pending the ObjectMeta rework in perf(gc): reduce retained arena RSS and redundant GC work #11781.

Summary by CodeRabbit

• Performance

  • Improved repeated reads and writes of class accessors by using cached access paths when the receiver and accessor remain valid.

• Bug Fixes

  • Kept cached accessor results correct when prototypes or accessor definitions change.
  • Added regression scenarios covering inherited, overridden, and one-sided accessors, as well as changing properties, receiver binding, and throwing accessors.

[coderabbitai]

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: dbad76d2-1a7d-4631-a77c-5c905cb90363

📥 Commits

Reviewing files that changed from the base of the PR and between f6c873d and 7819523.

⛔ Files ignored due to path filters (2)

• crates/perry-codegen/src/gc_effects/linux-x86_64.tsv is excluded by !**/*.tsv
• crates/perry-codegen/src/gc_effects/windows-x86_64.tsv is excluded by !**/*.tsv

📒 Files selected for processing (20)

• changelog.d/11784-accessor-shape-facts.md
• crates/perry-abi/src/lib.rs
• crates/perry-codegen/src/expr/body_call.rs
• crates/perry-codegen/src/expr/property_get.rs
• crates/perry-codegen/src/expr/property_get/accessor_arm.rs
• crates/perry-codegen/src/expr/property_get/front_contract_tests.rs
• crates/perry-codegen/src/expr/property_get/generic_dispatch.rs
• crates/perry-codegen/src/expr/property_get/tests.rs
• crates/perry-codegen/src/expr/put_value_store_ic.rs
• crates/perry-codegen/src/expr/put_value_store_ic/setter_arm.rs
• crates/perry-runtime/src/object/accessor_pair.rs
• crates/perry-runtime/src/object/class_meta_registry.rs
• crates/perry-runtime/src/object/class_registry.rs
• crates/perry-runtime/src/object/class_registry/state.rs
• crates/perry-runtime/src/object/field_get_set/ic_miss/ic_slow.rs
• crates/perry-runtime/src/object/method_site/read_holder.rs
• crates/perry-runtime/src/object/method_site/read_holder/class_read.rs
• crates/perry-runtime/src/proxy/put_value/packed_set.rs
• crates/perry-runtime/src/proxy/put_value/setter_site.rs
• test-files/test_gap_class_accessor_shape_facts.ts

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

---

📝 Walkthrough

Walkthrough

Class accessor read and store sites now use receiver and holder shape facts and cached accessor pairs to call compiled getters and setters. Prototype replacement retires the displaced prototype’s ShapeId. Runtime and code-generation tests and a regression scenario file cover the changed paths.

Changes

Class accessor paths

Layer / File(s)	Summary
Cache layout and prototype-shape retirement crates/perry-abi/src/lib.rs, crates/perry-runtime/src/object/class_meta_registry.rs, crates/perry-runtime/src/object/class_registry/...	The ABI defines cache-entry fields for accessor reads and compiled-setter sites. Class prototype replacement and generic-origin redirects retire the displaced prototype’s shape semantics.
Runtime getter cache and read integration crates/perry-runtime/src/object/accessor_pair.rs, crates/perry-runtime/src/object/method_site/read_holder.rs, crates/perry-runtime/src/object/method_site/read_holder/class_read.rs, crates/perry-runtime/src/object/field_get_set/ic_miss/ic_slow.rs	Getter entries retain the accessor pair and compiled getter. Hits check receiver and holder shapes and the current accessor lane. The slow path tries the cached accessor unless a native this alias is active.
Generated getter path and CFG validation crates/perry-codegen/src/expr/body_call.rs, crates/perry-codegen/src/expr/property_get*	On eligible 64-bit targets, a ShapeId miss can enter a guarded accessor arm before the existing miss front. The arm calls the compiled getter when its checks pass. CFG tests verify the guard chain and fallback edges.
Runtime setter cache and packed-store integration crates/perry-runtime/src/proxy/put_value/packed_set.rs, crates/perry-runtime/src/proxy/put_value/setter_site.rs	Setter entries retain receiver and holder shapes, the accessor pair, and the compiled setter. The packed-store miss path tries a cached entry before other miss work; GC root scanning also visits the cached pair.
Generated setter path and accessor regression scenarios crates/perry-codegen/src/expr/body_call.rs, crates/perry-codegen/src/expr/put_value_store_ic*, test-files/test_gap_class_accessor_shape_facts.ts, changelog.d/11784-accessor-shape-facts.md	Eligible store sites can emit a guarded compiled-setter call. The regression scenarios log results for accessor reads and writes across mutations and value types. The changelog reports implementation details and performance measurements.

Priority: ➖ Normal

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

Change: Refactor · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant PropertyReadSite
  participant AccessorCache
  participant HolderObject
  participant CompiledGetter
  PropertyReadSite->>AccessorCache: Load cached entry
  AccessorCache-->>PropertyReadSite: Return shape facts, pair, and getter
  PropertyReadSite->>HolderObject: Check holder shape and accessor lane
  HolderObject-->>PropertyReadSite: Return shape and lane value
  PropertyReadSite->>CompiledGetter: Call getter when guards pass

Loading

sequenceDiagram
  participant StoreSite
  participant SetterCache
  participant HolderObject
  participant CompiledSetter
  StoreSite->>SetterCache: Load cached entry
  SetterCache-->>StoreSite: Return shape facts, pair, and setter
  StoreSite->>HolderObject: Check holder shape and accessor lane
  HolderObject-->>StoreSite: Return shape and lane value
  StoreSite->>CompiledSetter: Call setter when guards pass

Loading

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 78195

No actionable merge-blocking risk was established for the class accessor changes; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🟠 High · up to 78195

The new accessor fast paths read cached heap objects before checking whether workers have started. Cached getter references stop being maintained after worker startup, so the late check may allow stale native-memory reads. Ordinary prototype invalidation and accessor eligibility have supporting safeguards, but they do not resolve this lifecycle risk.

Retained concerns

• High · security · inferred: The generated accessor arms check worker isolation only after reading cached holder memory. Getter cache roots stop being traced and rewritten once workers start, without clearing the cache words. A subsequent matching getter site can therefore dereference an obsolete holder address before declining the fast path. The setter arm has the same late-check ordering, leaving its cached-heap ownership boundary unenforced during those reads.

Security review details

Security Blast Radius

• inferred — The supported exposure is native execution of affected accessor sites within the containing process, including its primary/worker heap boundary. Script-controlled property access can exercise these sites. A stale-memory failure could affect that process; remote reachability, tenant exposure, privilege escalation, and code execution are not established.

Security Findings and Attack Paths

• inferred — A plausible failure sequence is to prime a getter cache, start a worker, allow the cached holder to move or become obsolete after cache-root rewriting stops, and revisit the site with a matching receiver shape. The generated path reads the cached holder address before consulting the sticky worker flag. This can produce an invalid native-memory read even though the eventual accessor call is declined. The sequence was not executed, and collection behavior after worker startup remains incompletely traced.

Trust Boundaries and Controls

• observed — The existing runtime getter hit checks worker state before dereferencing the holder. In contrast, both newly generated arms load holder memory before the worker check. Matching shapes and pair identity authenticate an accessor selection but do not independently establish that an unmaintained address is still safe to read.

Resilience and Maintainability Implications

• observed — The Number-only inline setter avoids carrying movable assigned values through that arm. Non-number values use runtime invocation, which roots the assigned value across the accessor call and returns its rewritten value.

Hardening Proposals

• proposed — Make the worker-isolation branch dominate every cached holder dereference in both generated arms. Validate the lifecycle with a primed site followed by worker startup and collection, and assert guard ordering in generated control-flow contract tests.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	For #10498, the PR adds shape-guarded class accessor paths and reports getter_read2 at 219 instructions, setter_write2 at 313, and a setter_ctor/field_ctor ratio of 1.52×. These meet the state…	Update get_accessor_descriptor to perform the lookup without allocating a String per call, and add or update an automated test for that behavior.
Docstring Coverage	⚠️ Warning	Docstring coverage is 67.24% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 58 functions across 19 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The codegen arms, runtime cache validation, prototype ShapeId retirement, GC-effect classification, ABI constants, tests, and changelog entry all support the #10498 class-accessor fast path or its cor…
Title check	✅ Passed	The title clearly summarizes the main change: class accessor sites use shape facts for validation and call accessors directly.
Description check	✅ Passed	The description explains the change, links issue #10498, and gives detailed tests and verification results. It omits the template’s Summary, Changes, and Test plan headings and checklist, but the requ…

Full details: Linked Issues check

Explanation

For #10498, the PR adds shape-guarded class accessor paths and reports getter_read2 at 219 instructions, setter_write2 at 313, and a setter_ctor/field_ctor ratio of 1.52×. These meet the stated performance targets. The PR summary does not show a change to get_accessor_descriptor that removes its per-lookup String allocation. Avoiding that allocation is a separate requirement in #10498; the new fast paths do not establish that other descriptor lookups avoid it.

Full details: Docstring Coverage

Explanation

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

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

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

---

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