perf(codegen,gc): declare a class's typed-shape layout at allocation, not after its constructor (#7510 item 1, #7512)

[proggeramlug]

#7510 item 1 (construction), which absorbed the residual half of #7512. Item 2 (death) shipped in #7525; item 3 (slot notes) is untouched.

The defect

js_gc_init_typed_shape_layout was emitted after the constructor call, so no raw-f64 class-field store inside a constructor could pass its GC_OBJ_TYPED_LAYOUT_INTACT guard — neither the inline and i16 %r, 4096 test nor class_field_fast_contract's layout_typed_raw_f64_slot_for_user. Every one fell back to js_put_value_set.

Declaring the fields number was what made the class slower. More type information selected a representation whose guard the construction path had made unsatisfiable.

The one-line reorder does not work, and that is the whole design problem: init_typed_shape_layout validates that each raw-f64 slot already holds a plain double, and a fresh slot holds TAG_UNDEFINED, whose 0x7FFC tag sits inside layout_raw_f64_bits' reject range. An early call downgrades every instance it touches. (test_validating_install_refuses_a_freshly_allocated_instance pins this as executable documentation — if that predicate ever accepts undefined, the test fails and the whole split becomes removable.)

The change

A second runtime entry point, js_gc_declare_typed_shape_layout, skips slot validation. That moves the burden of proof to codegen, where typed_shape::class_layout_declarable_at_allocation discharges it with two conditions:

1. Every raw-f64 field is assigned by the constructor prologue from a plain parameter — perf(codegen): elide provably-dead per-store bookkeeping on class-field stores #7486's ctor_prologue_param_assigned_fields, non-empty only for a class with no heritage, no field initializers or computed keys, no decorators, plain parameters, and no setter shadowing an assigned field. A LocalGet of a plain parameter cannot throw, allocate, or observe this, so nothing can read a raw-f64 slot between the declaration and its first write. Every, not some — one field assigned later would still be exposed.
2. The pointer mask is empty, so the declared state is POINTER_FREE — byte-identical to what layout_init_pointer_free already sets on every fresh instance. The only delta emitted is the intact bit and the shape-shared descriptor install; the collector's view at birth is unchanged. Classes with pointer fields would install SIDE_MASK over the allocator's fill: sound on the pre-filling allocation path, but it would rest on that pre-fill, so they are out of scope.

One predicate drives both emitters — the declaration is emitted iff the post-constructor install is suppressed — so they cannot drift into double-installing or into leaving an instance with no descriptor.

Nothing rests on the values actually being numbers. A constructor that stores a string into a number-declared field is rejected by the store guard (is_plain_number_bits, and the inline path's finite-exponent test), falls back to the boxed setter, and downgrades the descriptor through layout_note_slot — the same path any post-install contradiction has always taken.

Measurements

Pinned quiet host (load 2.0, wall == user), interleaved, best-of-7:

bench	wall	speedup
push_cls — new Node(v, w) ×20M	2.360 → 1.520 s	1.553×
push_cls_read — same, plus reading both fields back	2.410 → 1.550 s	1.555×

The collector result is strictly better, not merely equal. push_cls: promotion 210,488 → 64 bytes, copied 0.0042 → 0.0036 GB, peak RSS 29.6 → 24.1 MB, cycles unchanged at 105. Those are the numbers the equivalent object literal already had (churn: 0.0036 GB, 64 B, 24.2 MB) — #7512's anomaly closed on the memory axis as well as the time one.

churn, churn_alloc, push_num, tree, deeplist, churn_read compile to byte-identical generated objects across the two arms, so their ±2% is host noise by construction. (Compared as object bytes — the object cache key hashes codegen env and compiler identity per #6394, so filenames always differ and cannot answer this.)

Object literals do not qualify — and the reason is a finding

HIR rewrites a closed-shape literal to new __AnonShape_<hash>(…), whose synthesized constructor is a qualifying prologue. But the minted class's field types come out as Any, not Number. Any is pointer-bearing, so the gate refuses — and there was no raw-f64 store path to unlock anyway.

The corollary is worth its own ticket: {v: number, w: number} is currently declared to the collector as two pointer slots. The literal path's remaining construction cost is a type-propagation gap, not an ordering one, and it is a different lever from this ticket's.

Testing

• cargo test -p perry-runtime — 1767 pass, 0 fail. cargo test -p perry-codegen — failure set identical to origin/main (22 pre-existing; the two ext_registry entries that appeared once did not reproduce in four further full-suite runs — a shared-registry test-order flake).
• New perry-codegen/tests/typed_shape_declared_at_allocation.rs (9 tests): the declaration dominates the constructor call, replaces the post-constructor install, and carries a raw-f64 mask with a null pointer mask — plus six negatives (non-prologue number field, pointer field, untyped field, all-boolean class, no constructor).
• New perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs (5 tests): the validating install still refuses a fresh instance, the declaring one accepts it, both still reject a slot-count mismatch and overlapping masks, and a contradicting store both evicts the descriptor and leaves the object conservatively scanned so its string child is still traced.
• Adversarial end-to-end: 20,000 instances constructed with heap strings in a number-declared field, collected hard — all 20,000 readable, typeof string, matching node exactly, under default and under PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_VERIFY_EVACUATION=1. A read-before-write probe confirms the gate refuses both shapes that would expose a pre-write slot (the declare symbol is not even linked) and matches node.
• 25 class/object-heavy test-files/ programs: byte-identical output across both arms, and identical again under GC zeal + from-space protection.
• cargo fmt --all -- --check clean; scripts/check_file_size.sh clean — new.rs was 19 lines under the 2000-line cap and this change would have pushed it to 2042, hence the lower_call/typed_shape_init split.

What's left on #7510

Item 3 (layout_note_slot compiling away on a store that still matches the canonical mask, ~6% of churn_alloc) is untouched, and the ticket's headline acceptance (≥1.5× on churn_alloc, gc::layout under 8%) still is not met — churn_alloc is the literal path, which as above needs the type-propagation fix, not this one. #7510 stays open.

Summary by CodeRabbit

• Performance

  • Improved handling of numeric fields in newly created objects, enabling more efficient storage and potentially reducing garbage-collection overhead.
  • Benchmark results show improvements in applicable scenarios, with no generated-object changes for other tested workloads.

• Reliability

  • Added safeguards to preserve conservative storage and tracing when field values or layouts do not match expectations.
  • Expanded coverage for valid, invalid, contradictory, overlapping, and mismatched layout cases.

• Documentation

  • Added changelog documentation describing the new typed-layout behavior and its limitations.

[coderabbitai]

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 25 seconds

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c28521b-7c88-4bd2-9a42-031cf16d9a06

📥 Commits

Reviewing files that changed from the base of the PR and between a0a0b78 and 29b42ed.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (15)

• CLAUDE.md
• Cargo.toml
• changelog.d/7532-typed-shape-declared-at-allocation.md
• crates/perry-codegen/src/gc_call_effects.rs
• crates/perry-codegen/src/lower_call/field_init.rs
• crates/perry-codegen/src/lower_call/mod.rs
• crates/perry-codegen/src/lower_call/new.rs
• crates/perry-codegen/src/lower_call/typed_shape_init.rs
• crates/perry-codegen/src/root_reload.rs
• crates/perry-codegen/src/runtime_decls/arrays.rs
• crates/perry-codegen/src/typed_shape.rs
• crates/perry-codegen/tests/typed_shape_declared_at_allocation.rs
• crates/perry-runtime/src/gc/layout.rs
• crates/perry-runtime/src/gc/tests/layout_trace.rs
• crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs

📝 Walkthrough

Walkthrough

Typed-shape layouts can now be declared immediately after class-instance allocation when constructor analysis proves safe raw-f64 initialization. Runtime layout installation distinguishes fresh allocations from existing objects, and codegen and GC tests cover valid and rejected cases.

Changes

Typed-shape allocation declaration

Layer / File(s)	Summary
Codegen eligibility and declaration crates/perry-codegen/src/typed_shape.rs, crates/perry-codegen/src/lower_call/..., crates/perry-codegen/src/runtime_decls/arrays.rs, crates/perry-codegen/src/gc_call_effects.rs, crates/perry-codegen/src/root_reload.rs	Codegen identifies eligible classes, emits layout declaration before constructor execution, and calls the new non-collecting runtime entry point with layout masks.
Runtime proof modes and layout installation crates/perry-runtime/src/gc/layout.rs	Runtime layout installation validates existing objects and skips allocator-slot validation for fresh allocations. Contradictory stores downgrade the layout conservatively.
Codegen and GC validation coverage crates/perry-codegen/tests/typed_shape_declared_at_allocation.rs, crates/perry-runtime/src/gc/tests/layout_trace/*, changelog.d/7532-typed-shape-declared-at-allocation.md	Tests cover declaration ordering, eligibility exclusions, fresh-slot handling, mask errors, contradictory stores, and conservative tracing. The changelog records the behavior and benchmark results.

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

Sequence Diagram(s)

sequenceDiagram
  participant NewLowering
  participant TypedShapeInit
  participant RuntimeLayout
  participant Constructor
  NewLowering->>TypedShapeInit: analyze constructor field assignments
  NewLowering->>RuntimeLayout: declare typed shape after allocation
  RuntimeLayout-->>NewLowering: install raw-f64 layout
  NewLowering->>Constructor: execute constructor stores
  Constructor->>RuntimeLayout: downgrade layout if a contradictory store occurs

Loading

Possibly related issues

• perf(GC): make per-object layout O(1)-loadable — kill per-operation thread-local layout tracking (umbrella: method_calls/array-downgrade/object-property) #5094 — Both changes modify typed-shape layout tracking in gc/layout.rs, although this PR adds allocation-time declaration rather than header-based state.
• perf(gc): layout side tables are 34% of object construction — the construction/death half of #5094 (allocation is 7.7%) #7510 — Both changes concern typed-shape initialization and runtime bookkeeping in gc/layout.rs.

Possibly related PRs

• PerryTS/perry#6930 — Directly related to typed-shape layout initialization during new lowering.
• PerryTS/perry#7486 — Introduced the constructor prologue assignment analysis reused for allocation-time eligibility.
• PerryTS/perry#7501 — Adds a related allocation-time GC typed-layout declaration path for arrays.

Suggested reviewers: thehypnoo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes declaring class typed-shape layouts at allocation instead of after construction.
Description check	✅ Passed	The description provides detailed context, implementation changes, related issues, benchmarks, and extensive test results, despite not following the template headings exactly.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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/7510-construction-layout

---

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: 1

🧹 Nitpick comments (3)

crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs (1)

107-131: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Optional: the raw child_header pointer is held across a store and a trace.

child_header is a plain Rust local. Rust stack locals are not conservatively scanned, so this pointer is neither a root nor a pin. The read at line 126 is only valid because js_object_set_field and the manual mark/trace helpers do not evacuate in this harness. If the downgrade path later gains an allocating step, this test reads freed memory instead of failing cleanly.

Re-deriving the header from child after the store would keep the test robust without changing what it proves.

🛡️ Proposed adjustment

     let child = crate::string::js_string_from_bytes(b"contradiction".as_ptr(), 13);
-    let child_header = unsafe { header_from_user_ptr(child as *mut u8) };
     crate::object::js_object_set_field(obj, 0, crate::value::JSValue::string_ptr(child));
+    let child_header = unsafe { header_from_user_ptr(child as *mut u8) };

🤖 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/layout_trace/declared_at_allocation.rs`
around lines 107 - 131, Remove the long-lived child_header local from the test
and derive the child’s header from child immediately before the final
GC_FLAG_MARKED assertion. Keep the existing store, marking, and tracing sequence
unchanged, so the assertion reads the post-trace header without retaining a
potentially stale pointer across operations.

crates/perry-codegen/tests/typed_shape_declared_at_allocation.rs (2)

107-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix the broken sentence in this doc comment.

The sentence has an unmatched ( and no predicate after "lowers to".

📝 Proposed wording

-/// `this.<name> = <param id>` in the shape user source lowers to
-/// (`PutValueSet`, not the synthesized `PropertySet` — see `#7512`).
+/// `this.<name> = <param id>` in the shape user source lowers to
+/// `PutValueSet`, not the synthesized `PropertySet` — see `#7512`.

🤖 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-codegen/tests/typed_shape_declared_at_allocation.rs` around
lines 107 - 108, The doc comment describing `this.<name> = <param id>` has an
unmatched parenthesis and an incomplete “lowers to” phrase. Rewrite the sentence
so it clearly states that the syntax lowers to `PutValueSet`, not the
synthesized `PropertySet`, with balanced punctuation.

---

246-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding in-crate coverage for the eligibility predicate.

class_layout_declarable_at_allocation is pub(crate), so its only coverage is this integration suite plus full IR compilation. A unit test module inside crates/perry-codegen/src/typed_shape.rs would pin the predicate directly and would run under the ordinary cargo test job. Keep the IR-level tests for the emission and ordering claims.

Based on the coding guideline "Do not rely exclusively on integration tests under crates/*/tests/*.rs for PR coverage ... place acceptance coverage in tests visible to cargo-test where practical".

🤖 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-codegen/tests/typed_shape_declared_at_allocation.rs` around
lines 246 - 254, Add an in-crate unit-test module in typed_shape.rs covering the
class_layout_declarable_at_allocation predicate directly, including its
eligibility behavior. Keep
declarable_class_declares_its_layout_at_the_allocation_site and the existing
IR-level tests unchanged for emission and ordering coverage.

Source: Coding guidelines

🤖 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/layout.rs`:
- Around line 946-948: In crates/perry-runtime/src/gc/layout.rs lines 946-948,
update js_gc_declare_typed_shape_layout to reject a non-empty pointer_mask when
proof is TypedShapeProof::FreshlyAllocated, and narrow the documentation to
cover only the POINTER_FREE case. In
crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs lines
168-192, add a neighboring test using non-overlapping raw 0b01 and pointer 0b10
masks, asserting the object does not become INTACT.

---

Nitpick comments:
In `@crates/perry-codegen/tests/typed_shape_declared_at_allocation.rs`:
- Around line 107-108: The doc comment describing `this.<name> = <param id>` has
an unmatched parenthesis and an incomplete “lowers to” phrase. Rewrite the
sentence so it clearly states that the syntax lowers to `PutValueSet`, not the
synthesized `PropertySet`, with balanced punctuation.
- Around line 246-254: Add an in-crate unit-test module in typed_shape.rs
covering the class_layout_declarable_at_allocation predicate directly, including
its eligibility behavior. Keep
declarable_class_declares_its_layout_at_the_allocation_site and the existing
IR-level tests unchanged for emission and ordering coverage.

In `@crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs`:
- Around line 107-131: Remove the long-lived child_header local from the test
and derive the child’s header from child immediately before the final
GC_FLAG_MARKED assertion. Keep the existing store, marking, and tracing sequence
unchanged, so the assertion reads the post-trace header without retaining a
potentially stale pointer across operations.

🪄 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: 28068c61-6c0c-42da-a6c5-7ca8aae5cb43

📥 Commits

Reviewing files that changed from the base of the PR and between fc5e56e and 231d4d4.

📒 Files selected for processing (13)

• changelog.d/7532-typed-shape-declared-at-allocation.md
• crates/perry-codegen/src/gc_call_effects.rs
• crates/perry-codegen/src/lower_call/field_init.rs
• crates/perry-codegen/src/lower_call/mod.rs
• crates/perry-codegen/src/lower_call/new.rs
• crates/perry-codegen/src/lower_call/typed_shape_init.rs
• crates/perry-codegen/src/root_reload.rs
• crates/perry-codegen/src/runtime_decls/arrays.rs
• crates/perry-codegen/src/typed_shape.rs
• crates/perry-codegen/tests/typed_shape_declared_at_allocation.rs
• crates/perry-runtime/src/gc/layout.rs
• crates/perry-runtime/src/gc/tests/layout_trace.rs
• crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs

[coderabbitai]

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

The declaring entry point does not enforce the empty-pointer-mask precondition, and no test pins it. Codegen guarantees an empty pointer mask by rejecting every pointer-bearing field, but js_gc_declare_typed_shape_layout is an extern "C" symbol that accepts any non-overlapping mask and installs SIDE_MASK over slots that still hold the allocator's fill.

• crates/perry-runtime/src/gc/layout.rs#L946-L948: reject a non-empty pointer_mask when proof == TypedShapeProof::FreshlyAllocated, and narrow this doc so it claims only the POINTER_FREE case.
• crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs#L168-L192: add a test next to test_declaring_install_rejects_overlapping_masks that passes a non-overlapping, non-empty pointer mask (for example raw 0b01, pointer 0b10) and asserts the object does not become INTACT.

📍 Affects 2 files

• crates/perry-runtime/src/gc/layout.rs#L946-L948 (this comment)
• crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs#L168-L192

🤖 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/layout.rs` around lines 946 - 948, In
crates/perry-runtime/src/gc/layout.rs lines 946-948, update
js_gc_declare_typed_shape_layout to reject a non-empty pointer_mask when proof
is TypedShapeProof::FreshlyAllocated, and narrow the documentation to cover only
the POINTER_FREE case. In
crates/perry-runtime/src/gc/tests/layout_trace/declared_at_allocation.rs lines
168-192, add a neighboring test using non-overlapping raw 0b01 and pointer 0b10
masks, asserting the object does not become INTACT.