Merge train 197: descriptor-bearing receivers keep the store fast paths (v0.5.1575) - #10297
Conversation
One `Object.defineProperty` sent every later store on that receiver down the full OrdinarySet walk: nine store fast paths test `OBJ_FLAG_HAS_DESCRIPTORS` for the whole object, and each install minted a process-unique shape generation that made the receiver's lineage private. zod v4 does exactly this — `$constructor` defines `_zod` before installing ~60 methods by assignment — so schema construction paid 5-17x the instructions and ~6x the memory of the same build without that one descriptor (#10287). Four changes, each conservative in the uncertain direction: - The `[[Set]]` fast path, the transition-cache lane, the existing-own-data overwrite and the store tail vet the descriptor flag per KEY through the #6759 Phase C2 summary (`own_descriptors_skip_key`), the same refinement `plain_data_write_may_intercept` already applied. An index-shaped key stays on the slow walk: a boxed String wrapper synthesizes non-writable index attributes the summary never records. - A class-less receiver whose [[Prototype]] was recorded explicitly (`new F()`, `Object.create`, `setPrototypeOf`) is walked per key (`plain_custom_prototype_may_intercept`) instead of being rejected wholesale. Proxies, handle prototypes, non-object prototypes, typed arrays, exotic expando hosts, class objects, native-module receivers, frozen prototypes and chains deeper than 8 keep the slow walk, and a class instance on the chain is also vetted against its class accessor chain. - `Object.defineProperty` adding a NEW key reuses the learned shape transition instead of minting a private keys array, and teaches the edge when it misses. A cached target is adopted only when its live inline-slot bound fits this receiver: two receivers can share a predecessor (notably the keyless birth shape) while holding different physical capacities, and publishing slots past the allocation would hand the collector memory the object does not own. - A DATA-descriptor install mints its semantic generation from (predecessor facts, key, attributes), so receivers repeating one install share the successor. Accessors, batches (freeze/seal), clears and prototype changes keep unique generations: their identities differ per receiver. Emitted code is untouched. Its write PIC, transition probe and read IC all test the object-wide flag (0x800 is in WRITE_PIC_BLOCKING_FLAGS), so a descriptor-bearing receiver still reaches the runtime, which re-vets every store per key. 5,000 receivers x 60 method stores, macOS arm64: | receiver | before | after | |---|---|---| | plain object | 18.65 B instr / 260 MB | 1.24 B / 44 MB | | `new F()` (zod's shape) | 13.89 B / 247 MB | 3.73 B / 42 MB | | class instance | 17.75 B / 258 MB | 4.39 B / 44 MB | The same fixtures without the descriptor are unchanged (1.04/2.52/2.50 B). 300 zod 4.1.8 `z.object` schemas: 23.25 B -> 21.94 B instructions (-5.6%) and 336 -> 354 MB peak (+5%); that workload's receivers do not converge on shared shapes, and the memory difference is not yet attributed.
… transition A descriptor install minted a process-unique semantic generation, so every receiver got a private successor shape and fell off the shared transition chain. The memo table added to share those generations was capacity-bound: it cleared wholesale at 8192 live entries, and constructing 300 real zod schemas cleared it seven times, re-minting ~57k generations that had already been agreed on. The generation is now a pure function of (predecessor shape, key, attributes), so an agreement reached once holds for the life of the process. Descriptor removals share their successor the same way. The per-object descriptor summary is also consulted exactly. It is 64 bits wide, so roughly one key in 64 collides with a real descriptor key; the collided store took the slow path, which appends to a private keys array and takes the receiver off the shared chain for the rest of its life. Both probe sites now confirm a set bit against the descriptor tables, and an object carrying descriptors for a single key (zod's `_zod`) answers from an exact hash with no table probe at all. Real zod, 300 z.object schemas: 21.879 -> 12.731 billion instructions (-41.8%). The 2000-receiver fixture: 3.887 -> 1.021 billion (-73.7%); at 80 properties 7.677 -> 2.233 billion (-70.9%), the quadratic growth removed. The identical fixture without a descriptor is unchanged, as is the Node 26.8.1 differential.
… probe
`own_descriptor_may_cover_key` kept an `if covered {}` where the removed
#10287 gate telemetry used to report. It changes nothing at runtime; it is
dead code in a hot per-key probe.
The new transition-cache lane read the interned key's header into a raw pointer at four sites, which took the module from 5 raw-handle sites to 9 and failed the ratchet's per-module ceiling. Each site passes the pointer straight to `transition_cache_lookup` or `transition_cache_insert`, both of which index a fixed-size table and neither of which allocates or reaches a collection point, and none of them reuses the pointer afterwards — so the scoped `with_const_ptr` form is what the ratchet asks for here and is semantically identical.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe runtime now tracks descriptor transitions by key, reuses eligible shape and keys-array transitions, and checks descriptors and custom prototypes per written key. New integration tests cover descriptor stores, prototype interception, proxy behavior, frozen prototypes, and property ordering. ChangesDescriptor Store Fast Paths
Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant JavaScript
participant ordinary_set_with_receiver
participant descriptor_state
participant ShapeCache
JavaScript->>ordinary_set_with_receiver: write property
ordinary_set_with_receiver->>descriptor_state: check descriptor and prototype for key
descriptor_state-->>ordinary_set_with_receiver: allow fast path or require slow walk
ordinary_set_with_receiver->>ShapeCache: use keyed shape transition
ShapeCache-->>ordinary_set_with_receiver: return successor shape
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
This train lands #10292 as v0.5.1575. A receiver carrying a property descriptor keeps the object-store fast paths, and receivers repeating the same descriptor install now share one shape instead of each minting a private lineage.
The source PR's three commits retain their authorship. Two train repairs, both for defects this train's gates caught:
if covered {}no-op inown_descriptor_may_cover_key, a hot per-key probe. Dropped.keys_array.rsfrom 5 raw-handle sites to 9 and failing the ratchet's per-module ceiling — the source run's ownlintreports the sameRaw-handle debt ratchetfailure. Each site passes the pointer straight totransition_cache_lookuportransition_cache_insert, which index a fixed-size table and neither allocate nor reach a collection point, and none reuses the pointer afterwards, so all four now use the scopedwith_const_ptrform the ratchet asks for. Total debt is back to main's 906 with no ceiling raised.Closes #10287
Validated head:
2ab1adb216ea0ce101bc5062dbd2385bd6ff2450, on main 46084f2. Source CI run 34971655584 is fully attributed against main:cargo-testfails only main's knownnative_stack::tests::stack_top_respects_custom_thread_stack_sizes(3,933 passed), the GC-stress chain and its matrix pass, gap shards 4–6 carry only main's three known regressions, andcheck/warningsfail the same steps as the baseline. Itslintadds the raw-handle ratchet failure repaired here.Local full lint passes 80/83 with main's three known failures; the
-D warningsdiagnostics are byte-identical to the landed-main build and all live inglobal_this_webassembly.rs/ic_slow.rs, neither touched here. Local suites: 1,543 codegen, 3,950 runtime (only main's release-only debug-assertion failure), 139 standard-library and 1,139 CLI tests pass. All 12 integration suites pass, 36 tests, including the PR's Node-pinned descriptor tests — sibling receivers sharing a shape, a diverging sibling keeping its own descriptor state, a custom prototype still intercepting the keys it owns, non-writable rejection, seal, and delete/re-add ordering.Targeted gap filters cover 54 fixtures across
descriptor,defineproperty,getterandshape; each filter asserts it selects fixtures. Every executed fixture passes except two whose oracle cannot run them, both attributed by compiling them with the main and train compilers and finding byte-identical output:test_three_like_native_class_descriptors(Node fails with ERR_MODULE_NOT_FOUND on an extensionless helper import; both builds printOK) andtest_issue_7981_thread_shape_stamp_parent(a perry-only fixture — Node cannot resolveperry/thread; both builds agree byte-for-byte, including the cross-thread class-inheritance chains, which is also evidence that shapes still round-trip acrossspawn/parallelMapunder the new generation scheme). No snapshot changes.Performance
Four fixtures, matched five-package release builds, eleven interleaved rounds, medians; every fixture's output matches Node 26.5.1 on both arms. The host was quiet (load 7–17) for this run.
Object.definePropertythen 40 assignments (zod's shape)defineLazyshape)new F()receiver + descriptor (the custom-prototype walk)Memory falls by roughly three quarters on every path this touches. The one cost is +1.0% instructions and +0.05 MiB on the no-descriptor control, consistent with the +0.4% the PR reports for its own control; it is stated rather than rounded away, and it is not claimed to be noise.
The final five-package build's artifact hashes are exactly those used for every probe and measurement above.
Before merging, the pushed head and unchanged main are checked again. After merging, the rewritten commits and source patches are checked for preserved authorship and the main tree must match the validated train exactly.
Summary by CodeRabbit
Performance
definePropertyoperations by retaining fast paths when unrelated properties have descriptors.Documentation
Chores