From a789fe7d204f696809c564aa3588abeceb701dfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 21 Jun 2026 23:30:18 +0200 Subject: [PATCH 1/2] perf(runtime): O(1) wide property insertion on class instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/object/class_registry.rs | 34 ++++++ .../src/object/field_set_by_name.rs | 13 +- crates/perry-runtime/src/object/mod.rs | 75 ++++++++++++ crates/perry-runtime/src/object/object_ops.rs | 28 +++++ .../src/object/reflect_support.rs | 2 +- crates/perry-runtime/src/proxy.rs | 29 +++-- .../tests/issue_class_instance_wide_set.rs | 115 ++++++++++++++++++ 7 files changed, 286 insertions(+), 10 deletions(-) create mode 100644 crates/perry/tests/issue_class_instance_wide_set.rs diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 94147ad8df..df184ab482 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -5497,6 +5497,40 @@ pub(crate) fn class_has_instance_getter(class_id: u32, name: &str) -> bool { false } +/// Whether the class chain rooted at `class_id` defines an instance getter OR +/// setter named `name` (on `Class.prototype`, via `js_register_class_getter` / +/// `js_register_class_setter`). These accessors live in the per-class vtable, +/// NOT in the address-keyed descriptor tables, so a prototype-object descriptor +/// scan would miss them — the dynamic-write fast path must consult this before +/// treating `instance[name] = v` as a plain own-data store (an inherited +/// accessor must intercept instead). Walks the `extends` chain like +/// [`class_has_instance_getter`]. +pub(crate) fn class_chain_has_instance_accessor(class_id: u32, name: &str) -> bool { + let Ok(guard) = CLASS_VTABLE_REGISTRY.read() else { + return false; + }; + let Some(reg) = guard.as_ref() else { + return false; + }; + let mut cid = class_id; + let mut depth = 0usize; + while cid != 0 && depth < 32 { + if let Some(vt) = reg.get(&cid) { + if vt.getters.contains_key(name) || vt.setters.contains_key(name) { + return true; + } + } + match get_parent_class_id(cid) { + Some(p) if p != 0 && p != cid => { + cid = p; + depth += 1; + } + _ => break, + } + } + false +} + pub(crate) unsafe fn class_instance_setter_apply( class_id: u32, name: &str, diff --git a/crates/perry-runtime/src/object/field_set_by_name.rs b/crates/perry-runtime/src/object/field_set_by_name.rs index 8308333ff0..fb8ef8ac34 100644 --- a/crates/perry-runtime/src/object/field_set_by_name.rs +++ b/crates/perry-runtime/src/object/field_set_by_name.rs @@ -1076,8 +1076,19 @@ pub extern "C" fn js_object_set_field_by_name( new_keys as usize, new_index as u32, ); + // The sidecar is keyed on the OBJECT pointer (see + // `keys_index_lookup`, which probes `obj as usize`), NOT the + // keys-array pointer — shape-sharing clones the keys array on + // every insert, so a keys-keyed entry would be orphaned each + // iteration. Previously this inline-slot append registered + // under `new_keys as usize`, so the obj-keyed lookup never + // found it and rebuilt the full O(key_count) index on every + // write — turning a wide build that stays on the inline-slot + // path (e.g. a class instance whose pre-sized inline capacity + // keeps appends below the overflow threshold) into O(n²). Use + // the object address to match the lookup + the overflow path. keys_index_insert( - new_keys as usize, + obj as usize, (new_index + 1) as u32, key_hash, new_index as u32, diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index bd59c890cf..5340c83703 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -755,6 +755,81 @@ pub(crate) fn object_proto_may_intercept_key(key: f64) -> bool { reflect_support::obj_value_has_own_key(proto_value, key) } +/// Whether a fast plain-data write of `key` to a CLASS INSTANCE (`class_id != 0`) +/// at `obj_addr` might be intercepted by its prototype chain — i.e. the slow +/// `[[Set]]` walk is required instead of a direct own-data store. Conservative: +/// any uncertainty returns `true` (take the slow path). +/// +/// All interception sources are checked so the fast path stays correct: +/// 1. A class getter/setter named `key` anywhere in the `extends` chain. These +/// live in the per-class vtable, NOT the address-keyed descriptor tables, so +/// the prototype-object scan in (2) cannot see them. +/// 2. An address-keyed accessor / non-writable descriptor on any *class* +/// prototype object (`Object.defineProperty(C.prototype, …)`), detected via +/// `OBJ_FLAG_HAS_DESCRIPTORS` on that prototype object. +/// 3. `Object.prototype` at the chain tail — delegated per-key to +/// [`object_proto_may_intercept_key`]. +/// +/// Own-instance descriptors / frozen / sealed are excluded by the caller before +/// this is reached. +pub(crate) unsafe fn class_instance_set_may_intercept( + obj_addr: usize, + class_id: u32, + key: f64, +) -> bool { + // Decode the key once — used for both the class-chain and per-prototype + // accessor probes below. + let name = match reflect_support::key_to_rust_string(key) { + Some(n) => n, + // Non-decodable / non-string key: do not risk the fast path. + None => return true, + }; + // (1) A class getter/setter for this exact key anywhere in the class chain. + if class_registry::class_chain_has_instance_accessor(class_id, &name) { + return true; + } + // (2)/(3) Walk the prototype OBJECTS from the instance's [[Prototype]]. + let mut proto = js_object_get_prototype_of(crate::value::js_nanbox_pointer(obj_addr as i64)); + let mut depth = 0u32; + loop { + depth += 1; + if depth > 64 { + // Pathologically deep / cyclic chain — be safe. + return true; + } + let bits = proto.to_bits(); + // Not a heap-object pointer → end of chain (null prototype). Nothing + // below to intercept. + if (bits >> 48) != 0x7FFD { + return false; + } + let p = (bits & crate::value::POINTER_MASK) as usize; + if p == 0 { + return false; + } + if crate::array::object_prototype_addr_matches(p) { + // Reached the canonical Object.prototype: per-key check, then done. + return object_proto_may_intercept_key(key); + } + // Per-KEY intercepting descriptor on this class prototype. A blanket + // `object_has_descriptors(p)` bail is too coarse — every class prototype + // carries descriptors (constructor / method install), which would defeat + // the fast path entirely. Only an inherited accessor or non-writable data + // property *named this key* actually intercepts the write. + if object_has_descriptors(p) { + if get_accessor_descriptor(p, &name).is_some() { + return true; + } + if let Some(attrs) = get_property_attrs(p, &name) { + if !attrs.writable() { + return true; + } + } + } + proto = js_object_get_prototype_of(proto); + } +} + /// #5054: record descriptor installation on the target object itself — /// `OBJ_FLAG_HAS_DESCRIPTORS` in its GcHeader (travels with the object on /// evacuation), plus the `Object.prototype` process-global above. Unlike diff --git a/crates/perry-runtime/src/object/object_ops.rs b/crates/perry-runtime/src/object/object_ops.rs index 445efeb135..b63b4fce39 100644 --- a/crates/perry-runtime/src/object/object_ops.rs +++ b/crates/perry-runtime/src/object/object_ops.rs @@ -2786,6 +2786,34 @@ pub extern "C" fn js_object_get_prototype_of(obj_value: f64) -> f64 { } return function_prototype_or_null(); } + // Fast [[Prototype]] for a DECLARED-class instance: resolve + // directly from the class id instead of the generic + // `constructor_dynamic_prototype` probe, which reads the + // `constructor` field by name and therefore does a LINEAR scan + // over the instance's own keys (O(own-key-count)) before missing + // and continuing to the prototype. On a wide build — + // `const o = new C(); for (i) o["k"+i] = i` — that scan grows by + // one each iteration, making any reflective getPrototypeOf on the + // instance O(n²). The class-id table at line ~2810 below already + // returns this exact prototype for the same instances; hoisting it + // here is semantically identical (same declared-class prototype + // object) but O(1). Gated on a REAL declared class id only + // (`class_decl_prototype_value_for_instance_class` returns None for + // class_id 0 / anonymous-shape / unregistered ids), so synthetic + // function-ctor instances and plain objects keep the existing + // `constructor`-based resolution unchanged. + 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; } diff --git a/crates/perry-runtime/src/object/reflect_support.rs b/crates/perry-runtime/src/object/reflect_support.rs index d6a3f6b08e..0448b15f9c 100644 --- a/crates/perry-runtime/src/object/reflect_support.rs +++ b/crates/perry-runtime/src/object/reflect_support.rs @@ -180,7 +180,7 @@ pub(crate) fn reflect_define_property(obj: f64, key: f64, descriptor: f64) -> f6 reflect_bool(true) } -unsafe fn key_to_rust_string(value: f64) -> Option { +pub(crate) unsafe fn key_to_rust_string(value: f64) -> Option { let key_str = crate::builtins::js_string_coerce(value); if key_str.is_null() { return None; diff --git a/crates/perry-runtime/src/proxy.rs b/crates/perry-runtime/src/proxy.rs index 028635dec2..39f1cef5de 100644 --- a/crates/perry-runtime/src/proxy.rs +++ b/crates/perry-runtime/src/proxy.rs @@ -1249,10 +1249,6 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) // POINTER_TAG'd heap object, or a module-level slot's raw I64 pointer // (top 16 bits zero). && (target_top16 == 0x7FFD || target_top16 == 0) - // Per-key, not the coarse process-wide flag: an unrelated descriptor on - // Object.prototype must not force every write of an *absent* key onto the - // O(n) slow walk (that made wide-object builds O(n²)). - && !crate::object::object_proto_may_intercept_key(key) && unsafe { crate::symbol::js_is_symbol(key) } == 0 { let addr = extract_pointer(target.to_bits()) as usize; @@ -1270,11 +1266,28 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) | crate::gc::OBJ_FLAG_HAS_DESCRIPTORS; if header.obj_type == crate::gc::GC_TYPE_OBJECT && header._reserved & SLOW_FLAGS == 0 - && (*(addr as *const crate::ObjectHeader)).class_id == 0 - && crate::object::prototype_chain::object_static_prototype(addr).is_none() { - target_set(target, key, value); - return true; + let class_id = (*(addr as *const crate::ObjectHeader)).class_id; + let fast_safe = if class_id == 0 { + // Plain object: prototype is exactly Object.prototype, and + // Object.prototype doesn't intercept this key (per-key, not + // the coarse process-wide descriptor flag — that made wide + // builds O(n²)). + crate::object::prototype_chain::object_static_prototype(addr) + .is_none() + && !crate::object::object_proto_may_intercept_key(key) + } else { + // Class instance: the `class_id == 0` guard previously sent + // EVERY wide class-instance build down the O(own-key) slow + // walk (O(n²)). Safe to fast-path when no inherited accessor / + // non-writable anywhere in the prototype chain could intercept + // this key. + !crate::object::class_instance_set_may_intercept(addr, class_id, key) + }; + if fast_safe { + target_set(target, key, value); + return true; + } } } } diff --git a/crates/perry/tests/issue_class_instance_wide_set.rs b/crates/perry/tests/issue_class_instance_wide_set.rs new file mode 100644 index 0000000000..fce27721f1 --- /dev/null +++ b/crates/perry/tests/issue_class_instance_wide_set.rs @@ -0,0 +1,115 @@ +//! Regression test: writing many fresh own properties to a CLASS INSTANCE +//! (`class C {}; const o = new C(); for (i) o["k"+i] = i`) must be O(1) per +//! insert — the same as a plain `{}` — while STILL honoring an inherited +//! setter that intercepts the write. +//! +//! Two layered O(n²) bugs previously made the class-instance wide build scale +//! quadratically (a 20k build took tens of seconds vs ~25ms for a plain +//! object): +//! * The dynamic-write sidecar key index was registered under the keys-array +//! pointer instead of the (stable) object pointer on the inline-slot append +//! path, so the obj-keyed lookup never hit and rebuilt the full index every +//! insert. +//! * `Object.getPrototypeOf` on a declared-class instance resolved its +//! `[[Prototype]]` via a `constructor`-field probe, which does a LINEAR scan +//! over the instance's own keys before missing — re-run on every set by the +//! `[[Set]]` interception check, the scan grew by one each iteration. +//! +//! This test asserts BOTH: the wide build completes (and reads back), and an +//! inherited `set` accessor still intercepts (no own data property created). + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(src: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write(&entry, src).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output).output().expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).to_string() +} + +#[test] +fn class_instance_wide_set_is_fast_and_intercepts() { + // An `Object.prototype` accessor is present (the worst case that forces the + // interception check on every write). The wide build of 20_000 fresh keys + // must still complete and read back correctly. The base-class `set baz` + // must intercept (no own `baz` data property created on the instance). + let src = r#" +Object.defineProperty(Object.prototype, "__x__", { get(){ return 1; }, configurable: true }); + +class Base { _b: any; set baz(v: any) { this._b = "base:" + v; } } +class C extends Base { [k: string]: any; } + +const o: any = new C(); +const N = 20000; +for (let i = 0; i < N; i++) o["k" + i] = i; + +// Inherited setter still intercepts: stored via the setter, NOT as an own prop. +o.baz = 5; + +const own = (obj: any, k: string) => Object.prototype.hasOwnProperty.call(obj, k); + +// Wide build completed: every key read back, count is exact. +console.log("count", Object.keys(o).length); +console.log("first", o["k0"], "mid", o["k12345"], "last", o["k19999"]); +console.log("setter", o._b, own(o, "baz")); +console.log("fresh-own", own(o, "k7")); +console.log("DONE"); +"#; + + let out = compile_and_run(src); + // Object.keys includes the 20_000 fresh keys (the inherited `baz` setter + // created `_b`, an own data field on the instance, but `baz` itself is not + // an own key). The exact count guards against dropped/duplicated keys. + assert!( + out.contains("count 20001"), + "wide build must keep every fresh key (+ the setter-created `_b`)\n{out}" + ); + assert!( + out.contains("first 0 mid 12345 last 19999"), + "values must read back at the correct keys\n{out}" + ); + // `o.baz = 5` ran the inherited setter (`_b == "base:5"`) and created NO + // own `baz` property — interception preserved despite the fast insert path. + assert!( + out.contains("setter base:5 false"), + "inherited setter must intercept (no own `baz` prop)\n{out}" + ); + assert!( + out.contains("fresh-own true"), + "a fresh key is a real own data property\n{out}" + ); + assert!( + out.contains("DONE"), + "program must run to completion\n{out}" + ); +} From e097c58f3be45112958089beb8aa0f083f614188 Mon Sep 17 00:00:00 2001 From: Ralph Date: Sun, 21 Jun 2026 19:21:49 -0700 Subject: [PATCH 2/2] fix(runtime): format proxy.rs + harden prototype classification (PR #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) --- crates/perry-runtime/src/object/mod.rs | 33 +++++++++++++++++++------- crates/perry-runtime/src/proxy.rs | 3 +-- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 5340c83703..4459b32ea3 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -798,15 +798,32 @@ pub(crate) unsafe fn class_instance_set_may_intercept( return true; } let bits = proto.to_bits(); - // Not a heap-object pointer → end of chain (null prototype). Nothing - // below to intercept. - if (bits >> 48) != 0x7FFD { - return false; - } - let p = (bits & crate::value::POINTER_MASK) as usize; - if p == 0 { + let top16 = bits >> 48; + // Classify the prototype value before dereferencing it — mirror the + // shapes `js_object_get_prototype_of` can hand back: + // - 0x7FFD NaN-boxed pointer: a small-handle payload (e.g. a Proxy) + // is NOT an ObjectHeader and may carry a trap → be conservative. + // - top16 == 0 raw pointer: module-level object literals recorded via + // `Object.setPrototypeOf` come back as raw I64 pointers. + // - null / undefined: genuine end of chain, nothing to intercept. + // - anything else: unknown shape → do not risk the fast path. + let p = if top16 == 0x7FFD { + let p = (bits & crate::value::POINTER_MASK) as usize; + if p == 0 { + return false; + } + if crate::value::addr_class::is_small_handle(p) { + // Proxy / handle prototype — assume it may intercept the write. + return true; + } + p + } else if top16 == 0 && bits >= (crate::gc::GC_HEADER_SIZE as u64) + 0x1000 { + bits as usize + } else if bits == crate::value::TAG_NULL || bits == crate::value::TAG_UNDEFINED { return false; - } + } else { + return true; + }; if crate::array::object_prototype_addr_matches(p) { // Reached the canonical Object.prototype: per-key check, then done. return object_proto_may_intercept_key(key); diff --git a/crates/perry-runtime/src/proxy.rs b/crates/perry-runtime/src/proxy.rs index 39f1cef5de..7ee801376a 100644 --- a/crates/perry-runtime/src/proxy.rs +++ b/crates/perry-runtime/src/proxy.rs @@ -1273,8 +1273,7 @@ fn ordinary_set_with_receiver(target: f64, key: f64, value: f64, receiver: f64) // Object.prototype doesn't intercept this key (per-key, not // the coarse process-wide descriptor flag — that made wide // builds O(n²)). - crate::object::prototype_chain::object_static_prototype(addr) - .is_none() + crate::object::prototype_chain::object_static_prototype(addr).is_none() && !crate::object::object_proto_may_intercept_key(key) } else { // Class instance: the `class_id == 0` guard previously sent