Skip to content

perf(runtime): property attributes live with the keys (descriptor arrays); no per-object attribute tables - #11411

Merged
proggeramlug merged 2 commits into
mainfrom
attrs-with-keys
Sep 26, 2026
Merged

proggeramlug merged 2 commits into
mainfrom
attrs-with-keys

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

What

Charter step 3: property attributes live with the keys, like V8's descriptor arrays.

  • Storage. A keys array that has any non-default attribute points to a parallel attributes array in its first slot. This reuses perf(regex): RegExp.prototype.test costs ~1.5 µs per call under Perex — 44x slower than the old engine even with the regex hoisted #10166's GC-traced named-properties mechanism. Keys arrays with no attributes are unchanged, so plain objects pay nothing.
  • Canonical key lists. The trie edge is now (key, attrs). A key with default attrs keeps its old identity, so existing layouts don't move.
    • Adding a key together with its attributes appends in place, with no copy. This covers exports getters, literal get/set, and defineProperty of a new key.
    • Changing an existing key's attributes (defineProperty, freeze, seal) rebuilds the list from that key onward, as V8 copies its descriptor array. Freeze does this once.
  • Shape summary byte. Each shape keeps a summary byte, so the chain store check reads one byte per prototype hop.
  • What this removes. Ordinary objects no longer use the separate per-object attribute tables, their refcounts, or the Bloom bits. Every lookup is answered from the object's own keys.
  • Lookup routing. One header read picks the lookup route: the keys, the meta summary, or the tables (descriptor_state/filter.rs).
  • IC primes. They are per key: a read IC declines only an accessor key, and a write IC declines only a key that isn't plain writable. The emitted write-PIC mask goes from 0x1987 to 0x1180.

Numbers

Dedicated Linux host, both arms built there, 5 interleaved rounds. Outputs are identical.

main branch change
Zod instructions 1.676 G 1.676 G −0.05%
Zod peak RSS 82.7–83.1 MB 78.7–79.1 MB −4.0 MB
tsc transpileModule instructions 94.06 G 94.45 G +0.4% (inside the ±2.7% noise floor)
tsc peak RSS 335–337 MB 328–329 MB −7.8 MB

Census (tsc / Zod):

tsc Zod
store checks answered by the summary byte 178,153 10,408
key-entry lookups 130,349 621
attribute edits 7,467 4,347
list rebuilds 654 330

Verification

  • Node-comparison fixtures: 9 files identical to node, including test_gap_attrs_in_shape.ts (62 rows, strict), _sloppy.cts and the new test_gap_attrs_with_keys.ts.
  • Runtime suite: 4623/0. 20× on each arm: 0 failures on this branch; main failed 2/20 on timing tests.
  • Codegen suite: 1727/0.
  • Gap suite, compared by id against main:
    • three tests flip to passing (6287_timer_batch_order, attrs_with_keys, common_ffi_handle_ids_distinct);
    • node_redis_from_source goes from compile_fail to diff. It now builds; the output diff is not yet attributed.
  • GC root-dominance: 0 violations, 40/40 seeded violations caught, native corpus clean.
  • cargo fmt is clean. run_lint_gates passes except the two host-only failures.

Follow-up: charter step 3's accessor stage. Class get/set become real accessor properties on the class prototype, and it deletes #11348's class_accessor_cache.rs.

Summary by CodeRabbit

  • Bug Fixes
    • Improved property-attribute handling across ordinary objects, including reads, writes, enumeration, accessors, deletion, and integrity operations such as freezing and sealing.
    • Updated property caching so unrelated accessors no longer prevent caching data-property reads, while writes respect each property’s writability.
    • Preserved property attributes through object shape changes and garbage collection.
  • Tests
    • Added regression coverage for property attributes, accessors, integrity operations, and assignment behavior.

Ralph Küpper added 2 commits September 26, 2026 09:59
A key list that carries any non-default attribute owns a parallel
attributes array (one entry per key position, with cumulative summary
and Bloom words), attached through the named-properties reserve slot the
collector already traces and growth already carries. Attribute-free key
lists are unchanged. The canonical trie's edge is (key, entry): a default
entry hashes as before, a key that arrives with its attributes appends in
place, and a change to an existing key rebuilds the list from that key.
The shape record carries the attribute summary byte (identity), which the
class-chain store check reads first.

An ordinary object's attributes leave the property_descriptors table,
its owner index and its meta Bloom bits; the hashed attribute generation
is deleted. Deletes, squeezes and the dictionary latch carry entries;
a dictionary receiver edits its private list in place. The read IC
declines only an accessor key and the write ICs only a key that is not
plain writable data (emitted write PIC mask 0x1987 -> 0x1180). Builtin
installs, fast-arm accessor defines and arguments objects claim their
keys with their attributes. Default-off PERRY_ATTR_DIAG census behind
the attr-census feature.
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: e4ca9f5e-da04-4a85-96c0-5be48ed0fa1a

📥 Commits

Reviewing files that changed from the base of the PR and between 3463ca3 and ca007c6.

📒 Files selected for processing (65)
  • changelog.d/11411-attrs-with-keys.md
  • crates/perry-codegen/src/expr/proxy_reflect.rs
  • crates/perry-runtime/Cargo.toml
  • crates/perry-runtime/src/array/alloc.rs
  • crates/perry-runtime/src/array/mod.rs
  • crates/perry-runtime/src/array/named_props.rs
  • crates/perry-runtime/src/async_hooks.rs
  • crates/perry-runtime/src/gc/tests/dead_owner_side_tables.rs
  • crates/perry-runtime/src/gc/tests/keys_attrs.rs
  • crates/perry-runtime/src/gc/tests/mod.rs
  • crates/perry-runtime/src/gc/tests/young_log_tests.rs
  • crates/perry-runtime/src/json/stringify_shape_template_tests.rs
  • crates/perry-runtime/src/object/arguments.rs
  • crates/perry-runtime/src/object/assert.rs
  • crates/perry-runtime/src/object/attr_census.rs
  • crates/perry-runtime/src/object/canonical_keys.rs
  • crates/perry-runtime/src/object/canonical_keys_backing_tests.rs
  • crates/perry-runtime/src/object/canonical_keys_tests.rs
  • crates/perry-runtime/src/object/cell_meta.rs
  • crates/perry-runtime/src/object/class_registry/state.rs
  • crates/perry-runtime/src/object/delete_rest.rs
  • crates/perry-runtime/src/object/descriptor_state.rs
  • crates/perry-runtime/src/object/descriptor_state/filter.rs
  • crates/perry-runtime/src/object/descriptor_state/gc_scan.rs
  • crates/perry-runtime/src/object/descriptor_state/owner_lifecycle.rs
  • crates/perry-runtime/src/object/descriptor_state/tests.rs
  • crates/perry-runtime/src/object/dictionary.rs
  • crates/perry-runtime/src/object/field_get_set/ic_miss.rs
  • crates/perry-runtime/src/object/field_set_by_name/tail.rs
  • crates/perry-runtime/src/object/global_this/generator.rs
  • crates/perry-runtime/src/object/global_this/install_static.rs
  • crates/perry-runtime/src/object/global_this/math_temporal.rs
  • crates/perry-runtime/src/object/global_this/populate.rs
  • crates/perry-runtime/src/object/global_this/proto_methods.rs
  • crates/perry-runtime/src/object/global_this/typed_array.rs
  • crates/perry-runtime/src/object/global_this_webassembly.rs
  • crates/perry-runtime/src/object/key_attrs.rs
  • crates/perry-runtime/src/object/key_attrs_tests.rs
  • crates/perry-runtime/src/object/mod.rs
  • crates/perry-runtime/src/object/native_module.rs
  • crates/perry-runtime/src/object/native_module/callable_exports.rs
  • crates/perry-runtime/src/object/object_ops.rs
  • crates/perry-runtime/src/object/object_ops/define_get_accessor.rs
  • crates/perry-runtime/src/object/object_ops/keys_array.rs
  • crates/perry-runtime/src/object/reserved_floor.rs
  • crates/perry-runtime/src/object/shapes.rs
  • crates/perry-runtime/src/object/shapes_slot_list.rs
  • crates/perry-runtime/src/object/shapes_store.rs
  • crates/perry-runtime/src/object/shapes_tests.rs
  • crates/perry-runtime/src/object/string_wrapper.rs
  • crates/perry-runtime/src/object/temporal_proto.rs
  • crates/perry-runtime/src/object/websocket_global.rs
  • crates/perry-runtime/src/perf_hooks/prototypes.rs
  • crates/perry-runtime/src/promise/then_probe.rs
  • crates/perry-runtime/src/proxy.rs
  • crates/perry-runtime/src/proxy/put_value.rs
  • crates/perry-runtime/src/proxy/put_value/packed_set.rs
  • crates/perry-runtime/src/proxy/put_value/packed_set_tests.rs
  • crates/perry-runtime/src/timer/handle_object.rs
  • crates/perry-runtime/src/web_storage.rs
  • scripts/raw_handle_debt_baseline.txt
  • scripts/raw_handle_debt_files.txt
  • test-files/test_gap_attrs_in_shape.ts
  • test-files/test_gap_attrs_in_shape_sloppy.cts
  • test-files/test_gap_attrs_with_keys.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

The runtime now stores ordinary-object property attributes alongside key lists and includes attribute summaries in shape identity. Descriptor operations and inline-cache checks use per-key attributes. The change also adds regression coverage and an opt-in attribute census.

Changes

Key-Backed Property Attributes

Layer / File(s) Summary
Store attributes with key lists
crates/perry-runtime/src/object/key_attrs.rs, crates/perry-runtime/src/object/canonical_keys.rs, crates/perry-runtime/src/object/{arguments.rs,delete_rest.rs,dictionary.rs,field_set_by_name/tail.rs}, crates/perry-runtime/src/array/*, crates/perry-runtime/src/object/object_ops/*, crates/perry-runtime/src/gc/tests/*
Key lists now carry per-key attribute entries and summaries. Canonicalization, append, deletion, dictionary handling, and moving-GC coverage preserve those entries.
Route descriptor operations through key attributes
crates/perry-runtime/src/object/descriptor_state*, crates/perry-runtime/src/object/cell_meta.rs, crates/perry-runtime/src/object/mod.rs, crates/perry-runtime/src/object/{global_this/*,global_this_webassembly.rs,native_module*,*proto.rs}, crates/perry-runtime/src/{async_hooks.rs,perf_hooks/prototypes.rs,timer/handle_object.rs,web_storage.rs}
Eligible ordinary objects read and edit data-property attributes through key metadata. Accessor descriptors remain in descriptor tables. Built-in property installers use a combined definition helper.
Include attributes in shapes and cache guards
crates/perry-runtime/src/object/{shapes.rs,shapes_store.rs,shapes_slot_list.rs,reserved_floor.rs}, crates/perry-runtime/src/proxy/put_value*, crates/perry-codegen/src/expr/proxy_reflect.rs, crates/perry-runtime/src/promise/then_probe.rs
Shape records include attribute summaries. Read and write cache priming checks the relevant key attributes, and write-PIC blocking flags no longer include integrity and descriptor bits.
Validate attribute-aware reads, writes, and layouts
test-files/test_gap_attrs*, crates/perry-runtime/src/object/*tests.rs, crates/perry-runtime/src/proxy/put_value/packed_set_tests.rs, crates/perry-runtime/src/json/stringify_shape_template_tests.rs
Regression tests cover shared and dictionary layouts, descriptor changes, accessors, integrity operations, inline-cache priming, and key-list attributes.
Add attribute census and supporting updates
crates/perry-runtime/src/object/attr_census.rs, crates/perry-runtime/Cargo.toml, changelog.d/11411-attrs-with-keys.md, scripts/raw_handle_debt_*
Adds the opt-in attr-census feature and census output. The changelog and raw-handle debt baseline files are updated.

Priority: ➖ Normal

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

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant PropertyDefinition
  participant DescriptorState
  participant KeyAttributes
  participant ShapeRecord
  participant WriteInlineCache
  PropertyDefinition->>DescriptorState: apply attribute edit
  DescriptorState->>KeyAttributes: update the key entry
  KeyAttributes->>ShapeRecord: publish attribute summary
  WriteInlineCache->>ShapeRecord: inspect receiver shape
  WriteInlineCache->>KeyAttributes: check target key writability
Loading

Merge Risk: ⚪ Minimal · up to ca007

This change moves property attributes onto key lists and makes cache eligibility depend on each key's attributes. The integrity, deletion, and cache-guard paths that were examined behave correctly, and no concrete merge-blocking issue remains.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to ca007

The change substantially alters core runtime object metadata and cached property-write behavior. The implementation includes safeguards for descriptor enforcement and garbage collection, but an optional diagnostic feature can write through the runtime process’s filesystem authority when enabled; ownership of that enablement in deployed environments remains unverified.

Retained concerns

  • Medium · security · inferred: The new opt-in attribute census can route PERRY_ATTR_DIAG to an arbitrary filesystem path and write with the runtime process’s authority; the deployment boundary governing who can set the variable or enable the feature is not established.
Security review details

Security Blast Radius

  • inferred — If a lower-trust actor can control PERRY_ATTR_DIAG in a feature-enabled runtime, the affected scope is the filesystem authority of that runtime process rather than only the census subsystem.

Security Findings and Attack Paths

  • inferred — The potential path is environment configuration to Sink::File to temporary-file write and rename. Attacker control of the configuration is unproven, so this is an unresolved trust-boundary concern rather than a verified exploit path.

Trust Boundaries and Controls

  • observed — The feature gate and default-off configuration are controls that constrain availability of attribute census, but no supplied deployment evidence assigns ownership or validation of PERRY_ATTR_DIAG.
  • observed — For key-backed properties, descriptor enforcement remains key-specific: descriptor lookup returns stored entry attributes and packed write caching refuses keys that are not plain writable data.

Resilience and Maintainability Implications

  • observed — The census does not add a separate diagnostic writer; it uses the existing sink abstraction, and write failures fall back to stderr.

Hardening Proposals

  • proposed — Establish and document the deployment owner and trust boundary for enabling attr-census and setting PERRY_ATTR_DIAG; if lower-trust configuration can reach it, constrain diagnostic file destinations or disable file output for those deployments.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: moving property attributes into key-associated descriptor arrays and removing per-object attribute tables.
Description check ✅ Passed The description is detailed and on topic. It covers the storage model, canonical keys, shape summaries, lookup routing, inline-cache changes, benchmarks, and verification results. It does not use the …
Docstring Coverage ✅ Passed Docstring coverage is 85.20% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 223 functions across 50 files. (15 skipped:…
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
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

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