From dca63b1e794171f39727bc8376c5f4c4e9525a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 16 Sep 2026 05:27:34 +0200 Subject: [PATCH] perf(object): vet the store-plan cache per key, not per receiver The store-plan cache refused any receiver carrying OBJ_FLAG_HAS_DESCRIPTORS. Every zod schema object carries `_zod`, so none ever held a plan and each of its stores re-ran the whole interception vet. The flag was there for a real reason - a plan hit skips the own-accessor short-circuit - but that is a per-KEY fact, and `desc_gate_ok` on the same path already proves the key uncovered. Vetting per key is worth -7.3% on a 300-schema zod workload and -19.7% on the 2,000-receiver fixture. The new native test warms the plan for a class across 300 receivers, then proves an own accessor on a different key still dispatches its setter and a non-writable data descriptor is still respected; it fails if a plan hit ever skips that dispatch. --- changelog.d/10287-store-plan-per-key.md | 16 +++++++ .../src/object/field_set_by_name/tail.rs | 28 +++++++++--- .../tests/descriptor_store_fast_paths.rs | 43 +++++++++++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 changelog.d/10287-store-plan-per-key.md diff --git a/changelog.d/10287-store-plan-per-key.md b/changelog.d/10287-store-plan-per-key.md new file mode 100644 index 0000000000..aa746331f2 --- /dev/null +++ b/changelog.d/10287-store-plan-per-key.md @@ -0,0 +1,16 @@ +The store-plan cache is vetted per key rather than per receiver. It exists so a +property store need not re-run the interception vet — the prototype-chain walk, +the class-registry lookups and the `Object.prototype` probe — but it refused any +receiver carrying a property descriptor at all. zod installs `_zod` on every +schema object, so none of them ever held a plan and every one of their stores +paid the full vet again. + +Own descriptors disqualified the receiver for a real reason: an own accessor +must dispatch through a short-circuit that a plan hit skips. That is a per-key +fact, and the same path already proves the key uncovered, so the plan is now +denied only for the keys a descriptor can actually cover. + +Constructing 300 real zod v4 `z.object` schemas drops a further 7.3%, and a +fixture building 2,000 receivers that define one non-enumerable property and +then assign 40 properties drops 19.7% (1.02 to 0.82 billion instructions). The +same fixture without a descriptor is unchanged. diff --git a/crates/perry-runtime/src/object/field_set_by_name/tail.rs b/crates/perry-runtime/src/object/field_set_by_name/tail.rs index d58f513025..a929895ab1 100644 --- a/crates/perry-runtime/src/object/field_set_by_name/tail.rs +++ b/crates/perry-runtime/src/object/field_set_by_name/tail.rs @@ -270,11 +270,18 @@ pub(crate) fn set_field_by_name_object_tail( // diverges from its class chain (per-instance `setPrototypeOf` // override, null-proto) never records or honors a plan. // Flags that make an object ineligible for class-keyed plans: a - // diverging chain (per-instance proto override / null proto) or own - // descriptors (an own accessor must dispatch through the short-circuit - // below, which a plan hit skips). - const PLAN_BLOCKING_FLAGS: u16 = - crate::gc::OBJ_FLAG_NULL_PROTO | crate::gc::OBJ_FLAG_HAS_DESCRIPTORS; + // diverging chain (per-instance proto override / null proto). + // + // Own descriptors used to be a wholesale disqualifier here for a real + // reason — an own accessor must dispatch through the short-circuit + // below, which a plan hit skips. But that is a per-KEY fact, not a + // per-receiver one (#10287). zod puts `_zod` on every schema object, + // so the object-level flag denied a plan to every one of them and made + // each store re-run the whole interception vet: the chain walk, the + // class-registry lookups and the `Object.prototype` probe. The plan is + // now denied only for the keys an own descriptor can actually cover, + // which `own_descriptors_skip_key` decides exactly. + const PLAN_BLOCKING_FLAGS: u16 = crate::gc::OBJ_FLAG_NULL_PROTO; let obj_class_id = (*obj).class_id; // #6595: class objects are excluded by their authoritative ShapeId // kind — their @@ -286,6 +293,14 @@ pub(crate) fn set_field_by_name_object_tail( && obj_class_id != NATIVE_MODULE_CLASS_ID && crate::object::object_is_regular(obj) && (*gc_header)._reserved & PLAN_BLOCKING_FLAGS == 0 + // Per-key, not per-receiver: a descriptor on some OTHER key cannot + // intercept this one, and a plan hit skips the own-accessor + // short-circuit below, so the key must be provably uncovered. + && ((*gc_header)._reserved & crate::gc::OBJ_FLAG_HAS_DESCRIPTORS == 0 + || crate::object::own_descriptors_skip_key( + obj as usize, + f64::from_bits(JSValue::string_ptr(key as *mut _).bits()), + )) && !super::prototype_chain::object_has_prototype_divergence(obj as usize); let plan_fast = plan_eligible && super::prop_plan::store_plan_check(obj_class_id, interned_key as usize); @@ -525,6 +540,9 @@ pub(crate) fn set_field_by_name_object_tail( && obj_class_id != NATIVE_MODULE_CLASS_ID && crate::object::object_is_regular(obj) && obj_flags & PLAN_BLOCKING_FLAGS == 0 + // `desc_gate_ok` above already proved this key is uncovered on + // this receiver, which is the per-key half of the old flag. + && desc_gate_ok && !super::prototype_chain::object_has_prototype_divergence(obj as usize); if !plan_fast && record_plan_eligible { super::prop_plan::store_plan_record(obj_class_id, interned_key as usize); diff --git a/crates/perry/tests/descriptor_store_fast_paths.rs b/crates/perry/tests/descriptor_store_fast_paths.rs index b1823277e0..067dcfe75d 100644 --- a/crates/perry/tests/descriptor_store_fast_paths.rs +++ b/crates/perry/tests/descriptor_store_fast_paths.rs @@ -217,3 +217,46 @@ console.log(`${b.k2} ${JSON.stringify(Object.keys(b))}`); undefined 1\n42 [\"k1\",\"k2\",\"k3\"]\n" ); } + +/// The store-plan cache is vetted per KEY rather than per receiver (#10287), +/// so a receiver carrying a descriptor can hold a plan for its other keys. +/// A plan hit skips the own-accessor short-circuit, which is exactly what must +/// NOT happen for a key the receiver does own an accessor on — so warm the +/// plan for this class on many receivers first, then prove the accessor still +/// dispatches and a non-writable data descriptor is still respected. +/// Expectations verified against Node 26 first. +#[test] +fn a_warmed_store_plan_still_dispatches_an_own_accessor() { + let dir = tempfile::tempdir().unwrap(); + let out = run( + dir.path(), + r#" +class C {} +const make = (tag) => { + const o = new C(); + Object.defineProperty(o, "_zod", { value: tag, enumerable: false }); + const seen = []; + Object.defineProperty(o, "acc", { + set(v) { seen.push(v); }, get() { return seen.length; }, configurable: true, + }); + o.__seen = seen; + return o; +}; +for (let i = 0; i < 300; i++) { const w = make(i); w.plain = i; } +const a = make("a"); +a.plain = 1; +a.acc = "x"; a.acc = "y"; +console.log(JSON.stringify(a.__seen) + " " + a.acc + " " + + typeof Object.getOwnPropertyDescriptor(a, "acc").set); +console.log(a.plain + " " + a._zod + " " + JSON.stringify(Object.keys(a))); +const b = make("b"); +Object.defineProperty(b, "ro", { value: 1, writable: false, configurable: true }); +b.ro = 99; +console.log(String(b.ro)); +"#, + ); + assert_eq!( + out, + "[\"x\",\"y\"] 2 function\n1 a [\"__seen\",\"plain\"]\n1\n" + ); +}