perf(runtime): O(1) wide property insertion on class instances#5528
Merged
Conversation
Writing many fresh own properties to a class instance
(`class C {}; const o = new C(); for (i) o["k"+i] = i`) was O(n²),
while the identical build on a plain `{}` is O(1). Two independent
quadratic factors, both in the hot dynamic-write / interception path,
are fixed here:
LAYER 1 — extend the `[[Set]]` fast path to class instances.
`ordinary_set_with_receiver` previously gated its direct own-data store
on `class_id == 0`, forcing every class-instance write onto the full
interception walk. It now consults
`class_instance_set_may_intercept(obj, class_id, key)`, which is precise
per key: a class getter/setter anywhere in the `extends` chain
(`class_chain_has_instance_accessor`), an address-keyed accessor /
non-writable descriptor on any class prototype, or an intercepting
`Object.prototype` entry all still take the slow `[[Set]]` walk; an
absent key cannot be intercepted, so the fast store stays correct.
LAYER 2 — make the actual insertion O(1):
* The dynamic-write sidecar key index is keyed on the (stable) OBJECT
pointer, but the inline-slot append path registered the new entry
under the keys-array pointer. The obj-keyed lookup therefore never
hit and rebuilt the full O(key_count) index on every write — an
object that stays on the inline-slot path (a class instance whose
pre-sized inline capacity keeps appends below the overflow
threshold) degraded to O(n²). Register under the object address to
match the lookup and the overflow path.
* `Object.getPrototypeOf` on a declared-class instance resolved the
`[[Prototype]]` via a `constructor`-field probe, which does a LINEAR
scan over the instance's own keys before missing. The per-set
interception check re-runs that walk every iteration, so the scan
grew by one each time. Resolve the prototype directly from the class
id (the same declared-class prototype object the class-id table
already returns, just hoisted ahead of the linear probe) — gated on
a real declared class id so synthetic-ctor instances and plain
objects keep their existing `constructor`-based resolution.
Interception semantics are unchanged (inherited setters, getter-only
accessors, method shadowing, non-writable inherited data, and a fresh
own data prop all behave as before). A wide class-instance build now
scales linearly: a 20k build drops from ~16s to ~30ms, on par with the
plain-object build.
Adds an e2e regression test (`issue_class_instance_wide_set`) asserting
the wide build completes and reads back while an inherited setter still
intercepts.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a class-instance-aware fast path in ChangesClass Instance Set Fast Path with Prototype Interception
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/perry-runtime/src/object/object_ops.rs (1)
2789-2816: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMirror this fast path in the raw-pointer branch.
This handles declared-class instances only when
top16 == 0x7FFD. The latertop16 == 0branch still reachesconstructor_dynamic_prototype(obj)before the class-id lookup, so module-level raw-pointer class instances can keep the linear own-key scan this PR is removing.⚡ Proposed follow-up in the raw-pointer branch before `constructor_dynamic_prototype(obj)`
+ if (*gc).obj_type == crate::gc::GC_TYPE_OBJECT + && (*obj).class_id != 0 + && !is_anon_shape_class_id((*obj).class_id) + { + if let Some(proto) = + super::class_registry::class_decl_prototype_value_for_instance_class( + (*obj).class_id, + ) + { + return proto; + } + } if let Some(proto) = constructor_dynamic_prototype(obj) { return proto; }🤖 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/object/object_ops.rs` around lines 2789 - 2816, The fast path optimization for declared-class instances shown in the diff is only applied when top16 == 0x7FFD, but the raw-pointer branch (top16 == 0) still calls constructor_dynamic_prototype(obj) which performs a linear scan over own keys before the class-id lookup. To fix this, mirror the same fast-path logic in the raw-pointer branch by adding a check for declared-class instances before the constructor_dynamic_prototype(obj) call, using the same class_decl_prototype_value_for_instance_class function to detect and return the prototype directly when applicable, ensuring consistent O(1) performance across both branches.
🤖 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/object/mod.rs`:
- Around line 800-807: The prototype chain validation in the proto pointer check
is incorrectly treating all non-POINTER_TAG values as indicating the end of the
chain. Before returning false when the bits >> 48 check fails, you need to
distinguish between raw heap-object pointers (top16 == 0) and small handle
references. According to the coding guidelines, small pointer detection should
check if the value is less than 0x100000 to identify handle references. Modify
the logic around the bits >> 48 comparison to first check if the pointer is a
valid small handle before concluding there is no heap prototype, ensuring that
raw proto values and small handles are properly classified before deciding the
prototype chain has ended.
---
Nitpick comments:
In `@crates/perry-runtime/src/object/object_ops.rs`:
- Around line 2789-2816: The fast path optimization for declared-class instances
shown in the diff is only applied when top16 == 0x7FFD, but the raw-pointer
branch (top16 == 0) still calls constructor_dynamic_prototype(obj) which
performs a linear scan over own keys before the class-id lookup. To fix this,
mirror the same fast-path logic in the raw-pointer branch by adding a check for
declared-class instances before the constructor_dynamic_prototype(obj) call,
using the same class_decl_prototype_value_for_instance_class function to detect
and return the prototype directly when applicable, ensuring consistent O(1)
performance across both branches.
🪄 Autofix (Beta)
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: 0060f17c-dd00-449b-ba2e-bb7cb458ada7
📒 Files selected for processing (7)
crates/perry-runtime/src/object/class_registry.rscrates/perry-runtime/src/object/field_set_by_name.rscrates/perry-runtime/src/object/mod.rscrates/perry-runtime/src/object/object_ops.rscrates/perry-runtime/src/object/reflect_support.rscrates/perry-runtime/src/proxy.rscrates/perry/tests/issue_class_instance_wide_set.rs
…5528) Resolves the cargo-fmt lint failure (proxy.rs one-line let-chain) and incorporates CodeRabbit review feedback on the new wide-set fast path: `class_instance_set_may_intercept` now classifies the prototype value before dereferencing it, rather than treating every non-POINTER_TAG bit pattern as "end of chain". A small-handle payload (e.g. a Proxy prototype set via Object.setPrototypeOf) is now treated conservatively (may intercept) instead of being read as an ObjectHeader, and raw top16==0 heap pointers (module-level object literals) are followed instead of mistaken for a null prototype. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Writing many fresh own properties to a class instance was O(n²), while the identical build on a plain
{}is O(1):A 20,000-property class-instance build took ~16s; the plain-object equivalent is ~30ms. Any startup path that builds a wide object on a class instance hung.
There were two independent quadratic factors, both in the hot dynamic-write / interception path.
Layer 1 — extend the
[[Set]]fast path to class instancesordinary_set_with_receivergated its direct own-data store onclass_id == 0, forcing every class-instance write onto the full interception walk. It now consultsclass_instance_set_may_intercept(obj, class_id, key), which is precise per key: a class getter/setter anywhere in theextendschain (class_chain_has_instance_accessor), an address-keyed accessor / non-writable descriptor on any class prototype, or an interceptingObject.prototypeentry all still take the slow walk. An absent key cannot be intercepted, so the fast store stays correct.Layer 2 — make the actual insertion O(1)
Object.getPrototypeOfon a declared-class instance resolved[[Prototype]]via aconstructor-field probe that does a linear scan over the instance's own keys before missing. The per-set interception check re-runs that walk every iteration, so it grew by one each time. Resolve the prototype directly from the class id (the same declared-class prototype object the class-id table already returns) — gated on a real declared class id, so synthetic-ctor instances and plain objects keep the existing resolution.Correctness
Interception semantics are unchanged — inherited setters, getter-only accessors, method shadowing, non-writable inherited data, and a fresh own-data property all behave as before (covered by the new test).
Object.getPrototypeOfreturns the identical chain. Fullperry-runtimelib suite passes single-threaded (1067/1067).Result
20k class-instance build: ~16s → ~30ms, scaling linearly (on par with plain objects).
Test
crates/perry/tests/issue_class_instance_wide_set.rs— a 20k fresh-key build on a class instance completes and reads back, while an inherited setter still intercepts.Summary by CodeRabbit